RFR: 8322619: Parts of SG no longer update during rendering - overlapping - culling - dirty [v7]

[Kevin Rushforth]

On Fri, 28 Jun 2024 10:34:58 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> In some situations, a part of the SG is no longer rendered.
>> I created a test program that showcases this problem.
>> 
>> Explanation:
>> 
>> This can happen, when a part of the SG, is covered by another Node.
>> In this part, one node is totally covered, and the other node is visible.
>> 
>> When the totally covered Node is changed, then it is marked dirty and it's parent, recursively until an already dirty node is found.
>> Due to the Culling, this totally covered Node is not rendered - with the effect that the tree is never marked as Clean.
>> 
>> In this state, a Node is Dirty but not It's parent. Based on my CodeReview, this is an invalid state which should never happen.
>> 
>> In this invalid state, when the other Node is changed, which is visible, then the dirty state is no longer propagated upwards - because the recursive "NGNode.markTreeDirty" algorithm encounters a dirty node early.
>> 
>> This has the effect, that any SG changes in the visible Node are no longer rendered. Sometimes the situation repairs itself.
>> 
>> Useful parameters for further investigations:
>> -Djavafx.pulseLogger=true
>> -Dprism.printrendergraph=true
>> -Djavafx.pulseLogger.threshold=0
>> 
>> PR:
>> This PR ensures the dirty flag is set to false of the tree when the culling is used.
>> It doesn't seem to break any existing tests - but I'm not sure whether this is the right way to fix it.
>> It would be great to have some feedback on this solution - maybe guiding me to a better solution.
>> 
>> I could write a test, that just does the same thing as the test application, but checks every frame that these nodes are not dirty - but maybe there is a better way to test this.
>
> Florian Kirmaier has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8322544-render-culling
>  - JDK-8322619: Adjust test: remove Thread.sleep(), runAndWait()
>  - JDK-8322619: Add test
>  - Revert "JDK-8322619: Clear dirty flag on the node and all its children if they are skipped due to visible==false or opacity==0"
>    
>    This reverts commit 5f9f1574515c078c1fd0dccd476325090a0b284d.
>  - JDK-8322619: Clear dirty flag on the node and all its children if they are skipped due to visible==false or opacity==0
>  - reverted accidental change in the .idea folder
>  - JDK-8322619 Fix for rendering bug, related to overlap - culling - dirtynodes

This PR should be closed now that PR #1451 is integrated.

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

Changes requested by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1310#pullrequestreview-2166623300