On Fri, 22 Mar 2024 22:29:32 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> In https://github.com/openjdk/jfx/pull/1405, I identified some shortcomings >> of the stub font implementation. As I don't want to clutter the PR with >> that, I decided to cherrypick the improvements I did to a new ticket and PR. >> >> The current implementation has the following shortcomings: >> - It does not reliable detect the System Font, as a consequence, tests in >> TableColumnHeaderTest.java are failing on my local machine >> - Another consequence of this is, that the font size is always estimated >> with 0, as it is not detected >> - One shortcoming currently is, that the stub font siie estimate is not >> considering bold fonts. That would improve writing tests for some scenarios, >> e.g. for TableColumnHeader, where we would expect that the size of the >> header is bigger since it is bold >> >> Some tests were failing for the following reasons: >> - `AreaChartTest.java` - `expected -30.0, was -30.00000000004` - I added >> ceiling to the data. >> - `StackedBarChartTest.java` - since we now calculate correctly, the path >> changed >> - A test tried to load `Helvetica`, which is not supported in the stub font >> loader. I changed it >> - The default System font is considered a `Regular` one (style) >> >> I wrote tests and documented the stub behaviour. >> I did some minor changes here: >> - System font is now detected, also in bold and italic >> - A bold font will be calculated with a little bit more width (1px). >> Checkout the test as well for that >> >> Note: This only changes test setup, no 'production' code. > > modules/javafx.controls/src/test/java/test/javafx/scene/chart/AreaChartTest.java > line 538: > >> 536: .map(lineTo -> new Point2D( >> 537: >> Math.ceil(xAxis.getValueForDisplay(lineTo.getX()).doubleValue()), >> 538: >> Math.ceil(yAxis.getValueForDisplay(lineTo.getY()).doubleValue())) > > would it make more sense to either do a Math.round(), or better yet - write a > utility that computes array equality of Point2D's with some non-zero > tolerance? > > Do we have more tests like this that might warrant a new utility? AFAIK, we do not have other tests with that problem. I tried to keep the diff small, but nothing against writing a better method to compare the points here with a delta. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1422#discussion_r1538332039