-
Notifications
You must be signed in to change notification settings - Fork 290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Text Alignment Test Failure #1569
Comments
I agree, approximate equality check sounds like a good solution. |
There may be a bigger issue here. When I fixed the first part of the test the next part of it failed with an even larger error. The alignment settings are passed to OpenCASCADE and we don't seem to always be getting the correct result back across systems. The following code should horizontally and vertically center the letter "I". import cadquery as cq
centers = cq.Workplane().text("I", 10, 0, halign="center", valign="center", font="Sans")
show_object(centers) However, this is the result I get in CQ-editor (conda master install). |
Do you get consistent results if you specify I do not find a bigger issue with text alignment as far as the CQ results appear to agree with other tools such as blender (comparing CQ valign="center" with blender Vertical=Middle). |
This could very well be a difference of font metrics issue between platforms. The resulting font asset file that the OS returns for "Sans" could be the same, similar or different among platforms. The only definitive way of testing is to bundle a known TrueType font file with the CQ test suite and explicitly reference its path. |
@lorenzncode Specifying the font path does not fix the issue. This may be related to #187 . In that issue the same suggestion is made as the one from @michaelgale about bundling a font file for tests so that the tests cannot fail across platforms. It still seems like there is a bigger issue though when using |
@michaelgale @lorenzncode See #1577 for a fix for the test failures across platforms. |
@jmwright what is the conclusion actually? You wrote above that specifying the font path does not solve the issue, but your PR does specify a path. Is the PR using a different font than one causing the issue? |
@adam-urbanczyk Specifying the path to the font that CadQuery should have already been using based on the font name did not fix the issue. I was assuming that's what @lorenzncode wanted me to test. In this PR I used a known font that is already embedded in the repo so that we can avoid this test failure in the future, no matter which OS or distro they are run on. We can discuss whether there is a bigger issue with font alignment in a separate issue. |
cadquery/tests/test_cadquery.py Line 3846 in 5522037
|
@lorenzncode I had forgotten we included the OpenSans font file in the I think the root of the problem is that my system defaults to the Ubuntu Mono Regular font, which causes the bounding box asserts to fail by the values seen above. I haven't seen a warning when the font name and/or font path don't exist on the system, so it's fairly easy to accidentally cause a fall-back to the profile/system default font and not know it. This is mainly an issue when using font names or relative paths to font files. I got bitten by this multiple times while debugging. |
@jmwright This may be another reason to provide an interface to OCCT messages. I had explored that some in #1525. Testing with that branch: import cadquery as cq
from cadquery.occ_impl.message import Message, Level
Message.set_trace_level(Level.info)
report = cq.message.Message.add_report()
r = cq.Workplane().text("my text", 12, 0.1, font="badfont")
alerts = report.GetAlerts(cq.message.Level.info.value)
# optionally handle alerts, raise error Output message:
|
@lorenzncode I see the value in that. Can we create a logging level setting in CQ so that users can turn it up after importing the cadquery package? We would probably have to implement the logging over time as we implement/fix features, but a global setting would be a place to start. |
@jmwright Yes, I would like an option to set the OCCT message trace level where the source of the messages is OCCT. It might also be useful to interface with the OCCT Message_Report in some cases. |
@lorenzncode Ok. I think we should make that a separate issue rather than trying to implement it in #1577 |
Description
I can open a PR to fix this, but I wanted to open an issue for discussion first.
Running pytest shows an error in
testTextAlignment
that seems to be rounding error.To Reproduce
On an AMD 64, Debian 12 machine (could also be present on others):
The model looks like it has the correct bounding box ymin in CQ-editor, but is just slightly off when the bounding box is calculated.
Backtrace
Suggested Solution
Changing the assert to an "almost equal" variant would solve this problem. If the error was larger I would think that something is wrong, but it seems like rounding error based on the platform/distro since the test currently passes with the Ubuntu 22.04 runner on AppVeyor. I suppose it could be related to font differences between platforms as well, but that's a total guess.
Environment
OS:
mamba env create -f environment.yml
from latest git masterOutput of
conda list
from your active Conda environment:The text was updated successfully, but these errors were encountered: