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). Thanks a lot for documenting this! I think you highlighted many good points, and I agree with all of them. I've added a small discussion around the re-snapping (which I think doesn't solve all issue without an adjustment to our `snapSize` logic) but is about as accurate as it can be with the current state. That in itself shouldn't stop us from adding these docs, so will approve once the other small points have been discussed/resolved. modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java line 348: > 346: * <li>Snapping too early can lose precision, while snapping too > late can introduce errors. > 347: * <li>Snapping several children independently can avoid clipping, > but at the same time risk exceeding > 348: * the content size defined by the region. There should be no risk, as the compute results should take snapping into account; this can basically only occur if the calculation used by compute methods is different from the one used during layoutChildren. modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java line 414: > 412: * fixed allocated space, the algorithm must consider all > children together and coordinate rounding children > 413: * up or down so that their sum does not exceed the allocated > space. > 414: * <li><b>Do not repeatedly ceil the same semantic size.</b><br> I think this applies to all of them (you specifically mentioned `ceil`), but it is especially dangerous with the ones that use `ceil` (although I can think we can mitigate this with a small adjustment to the `ceil` code used if we're willing to be only accurate to say 1 millionth of a pixel). modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java line 428: > 426: * pieces of content are two allocations, while two intermediate > terms describing one piece of content are > 427: * one allocation. > 428: * <li><b>Re-snap after calculations, using the meaning of the > result.</b><br> I think we may need to check if this actually helps (it sometimes definitely will), even though I've saying this as well there is still a problem: The `snapSize` operation first applies its own floating point calculation (multiplying by render scale which can already introduce a tiny error) before calling `Math.ceil`. So even a correctly snapped end result (say 0.66...667) multiplied by the renderscale 1.5 can become 1.00...001 which is then ceil'd to 2. This happens because the result of a child's `compute`, even if snapped, gets used by the parent that treats it as content (using `snapSizeX/Y` ceiling). This is why I'm now of the opinion that we should subtract a constant value before ceiling, so the operation becomes: ceil((v * renderScale) - epsilon) / renderScale Where the epsilon is set to 1 millionth of a logical pixel (1e-6). The reasoning to use 1 millionth is: - too large a value may become noticable (ie. 1/10th of a pixel may introduce slight blurriness) - too small a value may not absorb floating point errors that have been multiplied (a spacing * number of children), are using values of fairly high magnitudes (a 100000 pixel screen or group of screens) or are using a fairly high renderscale - 1 millionth of a pixel is still unobservable and it is fair to say that a value within 1 millionth of a pixel can be considered to be *that* pixel Why not `ulp`? - It doesn't help correct the result when the calculations have accumulated more than 1 floating point error - It works poorly when the value to snap value is `Double.MAX_VALUE` (or something equally large) which is used through-out FX (an `ulp` at that magnitude is like 1e292 pixels) -- `ceiling` a `Double.MAX_VALUE` should yield `Double.MAX_VALUE` not `Double.MAX_VALUE - 1e292`). So resnapping may only be needed after significant number of calculations have been done with values that were snapped originally; tiny floating point errors should be absorbed by the parent's snapping (after `ceil` has been fixed) or by the rendering hardware (usually only accurate up to `float` precision). modules/javafx.graphics/src/main/java/javafx/scene/layout/package-info.java line 463: > 461: * // Depending on this region's fitting and overflow policy, > snap this region's > 462: * // raw position and raw allocated size to lay out its > children. > 463: * } This is a bit unclear to me, the snippet does not include any snapping? ------------- PR Review: https://git.openjdk.org/jfx/pull/2260#pullrequestreview-4945276341 PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3790825983 PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3790834531 PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3790898503 PR Review Comment: https://git.openjdk.org/jfx/pull/2260#discussion_r3790909343
