On Sat, 15 Aug 2026 18:31:23 GMT, Michael Strauß <[email protected]> wrote:

> Pixel snapping is really hard to get right (in fact, it's so hard that even 
> JavaFX itself gets it wrong in so many places).
> I've compiled a list of things that I've learned, because there isn't really 
> any good documentation as of yet.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

A very nice guide. I've left some comments on readability.

modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java 
line 409:

> 407:  *         The same rule applies to distinct margins and gaps: snap each 
> one with {@code snapSpaceX/Y}, then re-snap
> 408:  *         the final sum with {@code snapSpaceX/Y} again. For more 
> information, refer to <em>Re-snap after
> 409:  *         calculations, using the meaning of the result</em>.

This referred-to section hasn't appeared yet.
Suggestion:

 *         the final sum with {@code snapSpaceX/Y} again. For more information, 
refer to <em>Re-snap after
 *         calculations, using the meaning of the result</em> below.

modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java 
line 442:

> 440:  *
> 441:  *         // Incorrect: snapSizeX can turn floating-point noise into an 
> extra pixel
> 442:  *         double allocatedWidth = snapSizeX(firstWidth + gapWidth + 
> secondWidth);

The correct example uses `snapSpaceX` (after snapping individually), but this 
incorrect example uses `snapSizeX` without snapping individually. If the idea 
is to demonstrate re-snapping, then the same method should be used, otherwise 
it introduces another incorrectness.

modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java 
line 455:

> 453:  *         Since a region's position and allocated size are determined 
> by its parent (and the {@code isSnapToPixel}
> 454:  *         policy of its parent), it must not reposition or resize 
> itself. If the allocated width or height is not
> 455:  *         aligned, the region cannot both preserve the exact allocation 
> and make its complete bounds pixel-aligned.

"...is not aligned" with what? pixel-aligned?

modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java 
line 495:

> 493:  *             child.prefHeight(rawChildWidth),
> 494:  *             child.minHeight(rawChildWidth),
> 495:  *             child.maxHeight(rawChildWidth)));

The incorrect example combines the 2 calculations above it, but the mistake is 
only in the first one. Combining them hides the difference. I would write for 
the incorrect example:

double rawChildHeight = boundedSize(
    child.prefHeight(rawChildWidth),
    child.minHeight(rawChildWidth),
    child.maxHeight(rawChildWidth)));

for a direct comparison.

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

PR Review: https://git.openjdk.org/jfx/pull/2260#pullrequestreview-4946766595
PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3792343313
PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3792406938
PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3792514615
PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3792527806

Reply via email to