On Mon, 21 Sep 2026 15:13:11 GMT, Nir Lisker <[email protected]> wrote:
>> Update for the 3D lighting test tool as described in the JBS issue. >> >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Fix typo tests/performance/3DLighting/src/main/java/app/Benchmark.java line 65: > 63: var stopGraphic = createGraphic("⏹"); > 64: stopGraphic.setFill(Color.RED); > 65: stopGraphic.setFont(Font.font(20)); this sets graphic size to 20, but L182 it's 40 - that explains why they look different. use one size for all perhaps? tests/performance/3DLighting/src/main/java/app/Benchmark.java line 252: > 250: double fps = elapsedFrames / elapsedSeconds; > 251: instantFps.set(fps); > 252: System.out.println("\ninstant fps: " + fps); these stats are printed to stdout - would it make more sense to shows them in let's say a status bar at the bottom? tests/performance/3DLighting/src/main/java/app/CaptureUtils.java line 52: > 50: } > 51: > 52: private static final Path DIRECTORY = Path.of("screenshots"); This creates a directory in the project tree. I would suggest either to move the directory outside (user home?), or making sure it's `.gitignore`'d this file is probably a bad place to declare it - maybe in LightingApplication itself? also, make sure to explain in the javadoc there about files/dirs it creates. tests/performance/3DLighting/src/main/java/app/CaptureUtils.java line 68: > 66: throw new IOException("No writer found for " + > formatName); > 67: } > 68: > Desktop.getDesktop().open(DIRECTORY.toAbsolutePath().toFile()); seems backwards - first it writes the screenshot, then opens a file chooser, why? also, if I cancel the file chooser I expect nothing to be written out. I think it either needs to write silently (or maybe with a message saying "saved in XXX"), or follow the standard procedure to let the user select the folder to write the screenshot or cancel. and remember that choice (ideally, between the sessions). ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r4064676086 PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r4064565147 PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r4064385953 PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r4064429980
