On Tue, 17 Mar 2020 14:19:45 GMT, Frederic Thevenet <github.com+7450507+ftheve...@openjdk.org> wrote:
>> Now that the tiling is done in the `QuantumRenderer` level, I'll bring back >> [JDK-8189082](https://bugs.openjdk.java.net/browse/JDK-8189082). Can't this >> tiling be used to fix that? > >> >> >> Now that the tiling is done in the `QuantumRenderer` level, I'll bring back >> [JDK-8189082](https://bugs.openjdk.java.net/browse/JDK-8189082). Can't this >> tiling be used to fix that? > > It won't help with thing in their current state, as the `RenderToImage` > method that is modified in this PR is currently > called only by the snapshot feature in `Scene` But of course I suspect the > same technique could also be used to solve > [JDK-8189082](https://bugs.openjdk.java.net/browse/JDK-8189082) and now is > indeed a good time to investigate it > further. I'll have a look to see if it is possible to factorize the tiling > implementation for both classes of issues, > in which case it would make sense to do prior to merging this PR. If it is > different enough that the implementation > cannot be shared but only the general idea, then I suggest we address it in a > different PR. What do you all think ? At first glance, the NPE in [JDK-8189082](https://bugs.openjdk.java.net/browse/JDK-8189082) occurs in the Prism layer, which is one level _below_ Quantum where the tiling is currently implemented, so I'm not sure tit is reachable from there; if we want the code to be shared, it looks like it would need to be moved even further down (maybe in the `ResourceFactory`?) Also, while reusing code is generally the way to go, in such lower layers, very closely intertwined with the actual rendering, I'm afraid that insisting on having a "one-size-fits-all" implementation might get in the way of necessary case-by-case optimizations, so I'd like to have someone with a deeper knowledge of the code base to weight in before starting work in that direction. Maybe @kevinrushforth could advise? ------------- PR: https://git.openjdk.java.net/jfx/pull/112
