RFR: 8311527: Region.snapInnerSpace*()

[Kevin Rushforth]

On Sat, 29 Jul 2023 13:57:41 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, returns a value floored
>> 1807:      * to the nearest pixel in the horizontal direction, else returns the
>> 1808:      * same value, for any non-negative value.  For negative values returns 0.0.
> 
> I see this in the two new snap methods:
> 
>> For negative values returns 0.0.
> 
> Clamping to zero is not something any of the other snap methods do. It seems unexpected for a general purpose method such as this. If your use case really does require clamping to 0 (does it?), that should either be done by the caller, or it will need a name that implies clamping (e.g., has "clamp" or "positive" or similar in the name).
> 
> So the first question to answer is whether this really needs to clamp to zero?
> 
> Possibly worth noting is that this new method is similar to the existing private method `snapPortion`, but with a clamp to 0. I don't know if that might help inform the name (since it is non-public, it might not).

Weird. When I submitted my review, GitHub posted this old pending comment on an outdated revision. Please ignore.

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

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