On Fri, 28 Jun 2024 10:25:58 GMT, eduardsdv <d...@openjdk.org> wrote:
>> This is an alternative solution to the PR: >> https://github.com/openjdk/jfx/pull/1310. >> >> This solution is based on the invariant that if a node is marked as dirty, >> all ancestors must also be marked as dirty and that if an ancestor is marked >> as clean, all descendants must also be marked as clean. >> Therefore I removed the ``clearDirtyTree()`` method and put its content to >> the ``clearDirty()`` method. >> >> Furthermore, since dirty flag is only used when rendering by >> ``ViewPainter``, it should also be deleted by ``ViewPainter`` only. >> This guarantees: >> 1. that all dirty flags are removed after rendering, and >> 2. that no dirty flags are removed when a node is rendered, e.g. by creating >> a snapshot or printing. >> Therefore I removed all calls of the methods ``clearDirty()`` and >> ``clearDirtyTree()`` from all other classes except the ``ViewerPainter``. >> >> The new version of the ``clearDirty()`` method together with calling it from >> the ``ViewerPainter`` needs to visit far fewer nodes compared to the version >> prior this PR. >> >> The supplied test checks that the nodes are updated even if they are >> partially covered, which led to the error in the version before the PR. The >> test can be started with ``gradlew -PFULL_TEST=true :systemTests:test >> --tests NGNodeDirtyFlagTest`` > > eduardsdv 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 five additional commits since > the last revision: > > - Merge remote-tracking branch 'refs/remotes/origin/master' into > bugfix/JDK-8322619-render-dirty-flag > - JDK-8322619: Fix waiting for the stage > - JDK-8322619: Improve output message in test > - JDK-8322619: Avoid using of Thread.sleep(..) > - JDK-8322619: Combine clearDirtyTree() and clearDirty() methods. Few minor comments on the tests, apart from the failure that I see on Mac. Otherwise I could not find any issues. tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 49: > 47: static MyApp myApp; > 48: > 49: private CountDownLatch latch = new CountDownLatch(1); minor: latch is unused variable. Should be removed. tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 68: > 66: root = new StackPane(); > 67: primaryStage.setScene(new Scene(root, 500, 400)); > 68: Please add `primaryStage.setAlwaysOnTop(true);` call here, without it the test can fail fail intermittently if the test window gets behind any other window. tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 121: > 119: checkLineColor(root, lineColor.get()); > 120: } > 121: minor: can remove the empty line. ------------- PR Review: https://git.openjdk.org/jfx/pull/1451#pullrequestreview-2152023072 PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1661400361 PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1664293406 PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1664293879
