On Tue, 23 Jan 2024 11:51:52 GMT, Florian Kirmaier <fkirma...@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 incrementally with one 
> additional commit since the last revision:
> 
>   reverted accidental change in the .idea folder

I analysed the bug and can confirm the fix.
I summarized it in the following table. 
First column is the node structure, the others the steps: update nodes, render, 
update nodes, render, ...

| Nodes (L - left and R - right) / Steps                        | 1st -> update 
Circles         | 2nd -> render         | 3rd -> update Lines   | 4th -> render 
|
|-|-|-|-|-|
|HBox                                   |dirty                          
|clearDirty             |dirty                          |clearDirty             
        
|HBox >StackPane-L                      |dirty                          
|clearDirty             |dirty                          |clearDirty
|HBox >StackPane-L > Group-L                    |dirty                          
|clearDirty             |dirty                          |clearDirty
|HBox >StackPane-L > Group-L >Line-L            |                               
        |                               |dirty                          
|clearDirty
|HBox >StackPane-L > Group-L >Circle-L          |dirty                          
|clearDirty             |
|HBox >StackPane-R                      |dirty                          
|clearDirty  *1         |                               |       *3
|HBox >StackPane-R > Group-R                    |dirty                          
|dirty                  |dirty  
|HBox >StackPane-R > Group-R > Line-R           |                               
        |                               |dirty  *2                              
|dirty
|HBox >StackPane-R > Group-R > Circle-R         |dirty                          
|dirty                  |dirty                          |dirty


*1: > no culling bits for first dirty region (Circle-L) -> skip children -> 
clearDirty() is not called on children
     > no root path for the second dirty region (Circle-R) -> no rendering and 
root.clearDirtyTree() stops on StackPane-R (the children stay dirty: Group-R 
and Circle-R)
        
*2: > Line-R.markTreeDirty() skips marking of further parents because Group-R 
is already dirty -> the StackPane-R stays clean

*3: > the dirty regions are not collected because the StackPane-R is clean -> 
Line-R is not rendered and the dirty-flag is not reset

-------
The error is that ``clearDirty()`` is not called in the second step on the 
children of StackPane-R (see *1). Subsequent changes in Line-R and Circle-R are 
not propagated to the root node because Group-R is already dirty (see *2) and 
therefore StackPane-R remains clean. The last changes are therefore ignored in 
the next rendering phase (see *3).

-------
We can simply say: **the dirty flags must be deleted on all nodes after 
rendering**.

If this is true, the test can simply check this instead of screenshots or 
similar.

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

PR Comment: https://git.openjdk.org/jfx/pull/1310#issuecomment-2079676517

Reply via email to