On Mon, 21 Sep 2026 18:15:01 GMT, Nir Lisker <[email protected]> wrote:
>> 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?
>
> They are already shown in the GUI at the Performance pane (that's what
> `instantFps.set(fps);` does).
>
> The printing is done to keep the previous results in a log-like fashion. They
> are also legacy (Kevin wrote the fps counter, as well as the original quad).
ah, ok, so make it look like a log: timestamp, value1, value2; one line per
event?
>> 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).
>
> That line opens the folder where the screenshot it after it was taken, it's
> not a file chooser. It's for convenience. The screenshots you take in this
> app are for demonstration of something, so you always want to do something
> with it. It makes it easier if you don't need to dig around for the path.
oh right, it is just a small Finder shaped like a chooser. still, a but
unexpected (opens each time I make a screenshot), but it's a test program so
probably ok.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r4065092259
PR Review Comment: https://git.openjdk.org/jfx/pull/1387#discussion_r4065084171
