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

[eduardsdv]

On Fri, 28 Jun 2024 05:17:41 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> eduardsdv has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - JDK-8322619: Fix waiting for the stage
>>  - JDK-8322619: Improve output message in test
>>  - JDK-8322619: Avoid using of Thread.sleep(..)
>
> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 68:
> 
>> 66:             primaryStage.show();
>> 67: 
>> 68:             launchLatch.countDown();
> 
> This countDown() should be called after stage is shown to ensure the stage shown before proceeding.
> `primaryStage.setOnShown(e -> Platform.runLater(launchLatch::countDown));`

Done

> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 75:
> 
>> 73:     public static void setupOnce() {
>> 74:         Util.launch(launchLatch, MyApp.class);
>> 75:         assertEquals(0, launchLatch.getCount());
> 
> Util.launch() asserts if launchLatch.countDown() does not occur in 15 seconds, so this line can be removed.

Done

> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 90:
> 
>> 88:         StackPane root = myApp.root;
>> 89: 
>> 90:         runAndWait(() -> {
> 
> All such calls can be changed to use `Util.runAndWait()`

Done

> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 103:
> 
>> 101:         });
>> 102: 
>> 103:         Thread.sleep(500);
> 
> Could use `Util.waitForIdle(Scene)` instead of sleep. It would reduce the test time and should work. We can consider a sleep if `Util.waitForIdle(Scene)` does not work. Similarly please see if other sleep calls can be removed.

Done

> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 149:
> 
>> 147:             throw new RuntimeException(e);
>> 148:         }
>> 149:     }
> 
> Please use the `Util.runAndWait()` instead of adding this method. So this method can be removed and all calls can be changed to Util.runAndWait().
> Please do remove any imports that become unused afterwards.

Done

> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java line 188:
> 
>> 186:         return result;
>> 187:     }
>> 188: }
> 
> Please add an empty line at the end.

Done

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658440382
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658440585
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658444155
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658444269
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658443932
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658443599