On Thu, 21 Jul 2022 00:04:44 GMT, Kevin Rushforth <[email protected]> wrote:
>> Florian Kirmaier has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> JDK-8271395
>> QuantumRenderer is no longer public
>
> Inline
I've worked in the feedback, thank you @kevinrushforth
Small note, this change is not used for a while and is therefore well tested.
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/QuantumToolkit.java
> line 451:
>
>> 449: try {
>> 450: CountDownLatch latch = new CountDownLatch(1);
>> 451:
>> com.sun.javafx.tk.quantum.QuantumRenderer.getInstance().execute(() -> {
>
> Minor: this is in the same package, so you don't need the full package name.
> As long as you are making other changes, you might change this, too.
>
> A bigger question is that I think this is the first time we've used execute
> directly. I'm not sure that's a problem, but want to take a second look at
> it. The other places where we run a job on the renderer all use
> `Toolkit.getToolkit(). addRenderJob()`, which ultimately calls `submit()`.
I removed the package prefix.
I've just tested it, and the method "submit" is changed by overriding
newTaskFor in QuantumRenderer. This has the effect, that calling submit leads
to an exception. But calling execute works.
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/QuantumToolkit.java
> line 453:
>
>> 451:
>> com.sun.javafx.tk.quantum.QuantumRenderer.getInstance().execute(() -> {
>> 452: runnable.run();
>> 453: latch.countDown();
>
> If you do stick with the current approach, you would need a try / finally
> here, to avoid blocking the caller indefinitely. But in that case the
> exception isn't thrown back to the caller. I think using the existing method
> that returns a Future might be better.
I added a try-catch block, to ensure the countDown is always called.
As mentioned - calling submit instead of executing leads to an exception.
-------------
PR: https://git.openjdk.org/jfx/pull/618
