On Thu, 11 Jul 2024 17:15:14 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.
>> 
>> ---
>> 
>> 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 f...
>
> Marius Hanl has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains three 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
>  - Improve test
>  - 8218745: TableView: visual glitch at borders on horizontal scrolling

Marked as reviewed by angorya (Reviewer).

@arapte could you be a second reviewer?

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

PR Review: https://git.openjdk.org/jfx/pull/1462#pullrequestreview-2286745959
PR Comment: https://git.openjdk.org/jfx/pull/1462#issuecomment-2334452367

Reply via email to