On Fri, 17 Jan 2020 11:28:03 GMT, Frederic Thevenet <github.com+7450507+ftheve...@openjdk.org> wrote:
>> 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. Wel,l upon closer inspection it appears `QuantumToolkit::renderToImage` is only really used by `Scene::doSnapshot` anyway. Rendering of a the content of an NGSubScene (the underlying issue in JDK-8189082) is handled in a completely different code path, and I suspect this is also true for Canvas, where I hear a similar kind of issue exists. So from my (admittedly limited) understanding, fixing all problems at once would requires handling the tiling transparently within `RTTexture` (or more exactly inside all of its GraphicsPipeline specifics implementations) which seems quite a bit more complicated and risky. ------------- PR: https://git.openjdk.java.net/jfx/pull/68
