adam-urbanczyk
commented
7 months ago I agree, approximate equality check sounds like a good solution.
Closed jmwright closed 5 months ago
adam-urbanczyk
commented
7 months ago I agree, approximate equality check sounds like a good solution.
jmwright
commented
7 months ago 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).
lorenzncode
commented
7 months ago I suppose it could be related to font differences between platforms as well,
we don't seem to always be getting the correct result back across systems.
Do you get consistent results if you specify fontPath explicitly for these alignment tests?
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).
michaelgale
commented
7 months ago 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.
jmwright
commented
6 months ago @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 halign="center", valign="center" does not actually center the resulting text, or at least centers it differently depending on the OS. That may be unavoidable, but maybe a note should be added in the docs.
jmwright
commented
6 months ago @michaelgale @lorenzncode See #1577 for a fix for the test failures across platforms.
adam-urbanczyk
commented
6 months ago @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?
jmwright
commented
6 months ago @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.
lorenzncode
commented
6 months ago fontPath is used to specify a font file. That's what I meant - to verify that specifying a font file explicitly as done in some of the other existing tests resolves the issue.
https://github.com/CadQuery/cadquery/blob/552203713b2c5415b20109202442f366d33d082f/tests/test_cadquery.py#L3846
jmwright
commented
6 months ago @lorenzncode I had forgotten we included the OpenSans font file in the testdata directory, so I switched the test to use that.
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.
lorenzncode
commented
6 months ago 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.
@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 errorOutput message:
Font_FontMgr, warning: unable to find font 'badfont' [regular]; 'DejaVu Sans' [aspects: regular,bold,italic,bold-italic] [paths: /usr/share/fonts/dejavu-sans-fonts/DejaVuSans.ttf;/usr/share/fonts/dejavu-sans-fonts/DejaVuSans-Bold.ttf;/usr/share/fonts/dejavu-sans-fonts/DejaVuSans-Oblique.ttf;/usr/share/fonts/dejavu-sans-fonts/DejaVuSans-BoldOblique.ttf] is used instead@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.
lorenzncode
commented
6 months ago @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.
jmwright
commented
6 months ago @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
testTextAlignmentthat 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.ymlfrom latest git masterOutput of
conda listfrom your active Conda environment: