RFR: 8218745: TableView: visual glitch at borders on horizontal scrolling

Marius Hanl mhanl at openjdk.org
Thu May 23 21:56:14 UTC 2024


Alternative PR to https://github.com/openjdk/jfx/pull/1330 which does not modify the layout of `VirtualFlow`.

This PR fixes the glitching by removing the code in `NGNode.renderRectClip`, which made many calculations leading to floating point errors.
Interestingly I found out, that `getClippedBounds(..)` is already returning the correct bounds that just need to be intersected with the clip of the `Graphics` object.

So the following code is effectively doing the same:

Old:

BaseBounds newClip = clipNode.getShape().getBounds();
if (!clipNode.getTransform().isIdentity()) {
    newClip = clipNode.getTransform().transform(newClip, newClip);
}
final BaseTransform curXform = g.getTransformNoClone();
final Rectangle curClip = g.getClipRectNoClone();
newClip = curXform.transform(newClip, newClip); // <- The value of newClip after the transform is what getClippedBounds(..) is returning
newClip.intersectWith(PrEffectHelper.getGraphicsClipNoClone(g));
Rectangle clipRect = new Rectangle(newClip)


New:

BaseTransform curXform = g.getTransformNoClone();
BaseBounds clipBounds = getClippedBounds(new RectBounds(), curXform);
Rectangle clipRect = new Rectangle(clipBounds);
clipRect.intersectWith(PrEffectHelper.getGraphicsClipNoClone(g));


As you can see, there are very similar, but `getClippedBounds` does a much better job in calculating the bounds.
I also wrote a tests proving the bug. I took 100% of the setup and values from a debugging session I did when reproducing this bug.

I checked several scenarios and code and could not find any regressions.
Still, since this is change affects all nodes with rectangular clips, we should be careful.
Performance wise I could not spot any difference, I do not expect any difference.
**So I would like to have at least 2 reviewers.**
Note that I will do more testing as well soon on all JavaFX applications I have access to.

---

As written in the other PR, I have some interesting findings on this particular problem.

Copy&Paste from the other PR:
--

Ok, so I found out the following:
When a Rectangle is used as clip without any effect or opacity modification, the rendering goes another (probably faster) route with rendering the clip. That's why setting the `opacity` to `0.99` fixes the issue - another route will be used for the rendering.
This happens at the low level (`NGNode`) side of JavaFX.
...
I could track it down to be a typical floating point problem
...
The bug always appears when I scroll and the clip RectBounds are something like:
`RectBounds { minX:6.999996, minY:37.0, maxX:289.00003, maxY:194.0} (w:282.00003, h:157.0)`
...
while this does not happen when the value is something like:
`RectBounds { minX:7.000004, minY:37.0, maxX:289.0, maxY:194.0} (w:282.0, h:157.0`

Even more details:
---
- As briefly explained above, due to floating point arithmetic, we may do not get the correct value, leading to an incorrect calculation where 1 pixel is missing. That is why this issue happens only on display scales other than integer values.
- And since only the `ClippedContainer` changes its `layoutX` AND the `layoutX` of its clip, the bug only appears there 
- JavaFX Nodes uses `double`, while the low level side (NG) uses `float` mostly, which seems to make things less accurate, although not 100% sure if doubles will avoid this particular problem completely, probably not.
I'm not sure why this decision was made.

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

Commit messages:
 - 8218745: TableView: visual glitch at borders on horizontal scrolling

Changes: https://git.openjdk.org/jfx/pull/1462/files
  Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1462&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8218745
  Stats: 255 lines in 11 files changed: 173 ins; 58 del; 24 mod
  Patch: https://git.openjdk.org/jfx/pull/1462.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1462/head:pull/1462

PR: https://git.openjdk.org/jfx/pull/1462


More information about the openjfx-dev mailing list