RFR: JDK-8306990: The guarantees given by Region's floor and ceiling functions should work for larger values [v2]

John Hendrikx jhendrikx at openjdk.org
Tue May 2 15:43:27 UTC 2023


On Tue, 2 May 2023 15:38:06 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> > Okay, I will investigate, I have an idea what this can be, just very surprised that the rounding fixes would cause any issues in this code.
> 
> Thank you. I wonder if other places might also exhibit a similar failure mode. `scaledCeil` is called by `snapSizeX/snapSizeY` as well as `layoutInArea()` which are ... everywhere.

The problem is that the code you linked (L517 in HBox) is doing a few things that are incorrect (and which I fixed in #1111).  First, it has this line:

    while (Math.abs(available) > 1 && adjustingNumber > 0) {

Here it assumes that pixels have size == 1; the correct check should be `Math.abs(available) > 1 / renderScale`.

Secondly, it uses `snapPortionX`, which are package private methods (luckily) which have an interesting side effect.  They don't just round "down" (floor for positive, ceiling for negative) but they also have a minimum value (1 or -1) -- this is incorrect for two reasons: a round down function can also round to 0 (which I corrected), and secondly, using the constant 1 is incorrect because that should be `1 / renderScale`.

>From what it looks like, these `snapPortion` functions are only used by HBox/VBox, and I think it would be best to correct the callers here to not rely on the behavior that the smallest values that can be returned are 1 or -1 (also because those are simply incorrect).

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1118#issuecomment-1531696750


More information about the openjfx-dev mailing list