On Thu, 16 Jan 2020 17:00:32 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> The pull request has been updated with 2 additional commits. > > I tested this fix against the repro code in > https://github.com/javafxports/openjdk-jfx/issues/433 (which is > [JDK-8222238](https://bugs.openjdk.java.net/browse/JDK-8222238)), but there > is still an NPE. I'm not certain that this fix is supposed to solve that bug, > but according to the comments, the root cause is the same as > [JDK-8189082](https://bugs.openjdk.java.net/browse/JDK-8189082), which is > related to this one. It's worth to take a look to see if something was missed. > > > I tested this fix against the repro code in > [javafxports/openjdk-jfx#433](https://github.com/javafxports/openjdk-jfx/issues/433) > (which is [JDK-8222238](https://bugs.openjdk.java.net/browse/JDK-8222238)), > but there is still an NPE. I'm not certain that this fix is supposed to solve > that bug, but according to the comments, the root cause is the same as > [JDK-8189082](https://bugs.openjdk.java.net/browse/JDK-8189082), which is > related to this one. It's worth to take a look to see if something was missed. Although JDK-8189082 (and potentially others) are indeed likely to share the same root cause, the fix proposed here won't have an effect on anything else than snapshots, since the tiling is done at the Scene level rather than within QuantumToolkit. I purposefully choose to take the "easy" way out to limit the potential side-effects of the change, but I agree that on the other hand supporting tiling when needed directly in QuantumToolkit::renderToImage would have the potential to solve a whole category of issues. Also, from a very pragmatic angle, because of the increased complexity and scope and the risks that come with it, I'm not very optimistic that this change could realistically make it into jfx14 and to be honest I'm quite eager to get the screenshot feature in my app working properly ASAP. I'd be willing to try and work on a more generic fix under the umbrella of JDK-8189082, that'd be targeted at 15, while still having this fix included as part 14; but that means this should (ideally) be reverted once it becomes useless. I don't know if the idea of having a "temporary fix" approach seems acceptable in that context. ------------- PR: https://git.openjdk.java.net/jfx/pull/68
