RFR: 8311527: Region.snapInnerSpace*()

[Andy Goryachev]

On Wed, 6 Sep 2023 23:29:35 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Introduces Region.snapInnerSpaceX/Y() methods for dealing with inner space (using Math.floor), see for instance [JDK-8299753](https://bugs.openjdk.org/browse/JDK-8299753), using existing methods Region.snapPortionX/Y().
>
> modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java line 1808:
> 
>> 1806:      * If this region's snapToPixel property is true, then the value is either floored (positive values) or
>> 1807:      * ceiled (negative values) with a scale. When the absolute value of the given value
>> 1808:      * multiplied by the current scale is less than 10^15, then this method guarantees that:
> 
> 1. This doesn't match the descriptions of the other snap methods. All of them say "to the nearest pixel" without mentioning "the given scale" (which isn't accurate anyway, since the scale isn't "given" here). I recommend doing the same here. If we want to add something about the scale, then it should be done for all of the snap methods, and in a follow-on issue. If we do this, I would add it after the first sentence, and it would need to say that it is the "render" scale, explaining that the scale is used so that it will be rounded/floored/ceiled to the nearest screen pixel after taking scale into account.
> 
> 2. None of the other snap methods specify the guarantee you are making in this method, so we shouldn't add it here. This is also something we could do in a follow-on if needed.

the javadoc was lifted almost verbatim from snapPortionX/Y (I did not find the words "given scale" though).

Changing to be the same as snapSizeX/Y, although, technically speaking, that description is not correct either: the value in both sets of methods is ceiled/floored for positive values and floored/ceiled for negative values.

We might reword both cases saying something like "rounded to the nearest pixel with larger/smaller value" which will capture the behavior exactly.  What do you think?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1190#discussion_r1318847823