RFR: 8327179: Update the 3D lighting application

[Ambarish Rapte]

On Sun, 3 Mar 2024 22:29:02 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

> Update for the 3D lighting test tool as described in the JBS issue.

providing few more comments

tests/performance/3DLighting/src/main/java/lighting3D/Benchmark.java line 49:

> 47:         stopGraphic.setBoundsType(TextBoundsType.VISUAL);
> 48:         stopGraphic.setFill(Color.RED);
> 49:         stopGraphic.setFont(Font.font(20));

Play and Stop button could be same size. It would be good to look at. Currently the Stop button seems lesser than half size of play button.

tests/performance/3DLighting/src/main/java/lighting3D/Benchmark.java line 81:

> 79: 
> 80:         var sphere = new Button("Sphere");
> 81:         sphere.setOnAction(e -> switchTo(Models.createSphere(SPHERE_RADIUS, (int) subdivisionSlider.getValue())));

In `Models.createSphere()` method second argument is used for z distance, and not for subdivisions.. Increasing the slider value reduces the size of Sphere and vanishes at 510.

tests/performance/3DLighting/src/main/java/lighting3D/Benchmark.java line 179:

> 177:             totalElapsedFrames = 0;
> 178:             System.out.println();
> 179:             System.out.println(" --------------------- ");

May be print a message in addition to this.

tests/performance/3DLighting/src/main/java/lighting3D/CaptureUtils.java line 29:

> 27:     }
> 28: 
> 29:     private static final Path FOLDER = Path.of("screenshots");

suggestion: FOLDER -> SCREENSHOT_DIR or DIRECTORY
Using word directory would align with Java io/nio naming.

tests/performance/3DLighting/src/main/java/lighting3D/LightingApplication.java line 62:

> 60:         Node lightsControls = environment.createLightsControls();
> 61: 
> 62:         var controls = new VBox(perfControls, modelsControls, backgroundControls, screenshotControls,

Recommend to provide some spacing. 5 looked good.
-> var controls = new VBox(**5**, perfControls, modelsControls, backgroundControls, screenshotControls,

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

Changes requested by arapte (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1387#pullrequestreview-1917586203
PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r1513164981
PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r1513182318
PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r1513870834
PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r1513876778
PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r1514693071