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

Reply via email to