On Mon, 9 Dec 2024 23:53:54 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> 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. -> DONE: Could not spot any problem or difference. >> >> --- >> >> 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 Java... > > Marius Hanl has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains eight commits: > > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8218745-tableview-glitch-clipping-fix > > # Conflicts: > # > modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGNode.java > - Rename setContent to setClipLayout > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8218745-tableview-glitch-clipping-fix > > # Conflicts: > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8218745-tableview-glitch-clipping-fix > > # Conflicts: > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java > # > modules/javafx.graphics/src/test/java/test/com/sun/javafx/sg/prism/NGNodeTest.java > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8218745-tableview-glitch-clipping-fix > > # Conflicts: > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java > # > modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8218745-tableview-glitch-clipping-fix > > # Conflicts: > # > modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGNode.java > - Improve test > - 8218745: TableView: visual glitch at borders on horizontal scrolling Marked as reviewed by angorya (Reviewer). sorry, please disregard. had the window open where the merge conflict label was still there. ------------- PR Review: https://git.openjdk.org/jfx/pull/1462#pullrequestreview-2500805851 PR Comment: https://git.openjdk.org/jfx/pull/1462#issuecomment-2540037494
