On Sat, 29 Jul 2023 13:57:41 GMT, Kevin Rushforth <k...@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
