RFR: 8238954: Improve performance of tiled snapshot rendering
github.com+7450507+fthevenet at openjdk.java.net
Fri Feb 28 18:08:41 UTC 2020
On Wed, 12 Feb 2020 14:57:33 GMT, Frederic Thevenet <github.com+7450507+fthevenet at openjdk.org> wrote:
>> Issue JDK-8088198, where an exception would be thrown when trying to capture a snapshot whose final dimensions would be larger than the running platform's maximum supported texture size, was addressed in openjfx14.
>> The fix, based around the idea of capturing as many tiles of the maximum possible size and re-compositing the final snapshot out of these, is currently only attempted after the original, non-tiled, strategy has already failed.
>> This was decided to avoid any risk of regressions, either in terms of performances and correctness, while still offering some relief to the original issue.
>> This follow-on issue aims to propose a fix to the original issue, that is able to correctly decide on the best snapshot strategy (tiled or not) to adopt before applying it and ensure best performances possible when tiling is necessary while still introducing no regressions compared to the original solution.
> I've marked this PR as a WIP for the time being because I've only tested it on the d3d rendering pipeline so far, so I think it is too early to call for a formal review yet.
> Still, I welcome feedback if someone wants to have a look at it already.
> In a nutshell, this is what this PR does:
> - Reverts changes from 8088198
> - Implements tiling within `QuantumToolkit::renderToImage` instead
> - Gets the maxTextureSize directly from the `ResourceFactory` instance instead of relying on `PrismSettings.maxTextureSize` (which may be wrong in the event the maxTexture size supported by the hardware is less than the clamped value set in PrismSettings)
> - Tries to align the PixelFomat for the target `WritableImage` with that of the `RTTexture` when possible, since converting IntARGB to ByteBGRA (or the other way round) is a major cost factor when copying large amounts of pixels.
> - Avoids as much as possible having to resize the tile in between subsequent calls to `RTTexture::readPixels`, as it is another major cost factor, under the d3d rendering pipeline at least. (Native method `D3DResourceFactory_nReadPixels` attempts to reuse the same surface in between calls but is forced to create a new one if dimensions are not exactly the same, which is quite costly).
> - [x] Make sure chosen PixelFormat is optimum when using es2 pipeline.
> - [ ] Consider using subtexture pixel read with es2 (SW and d3d don't support it) as a better alternative to relying on same size tiles to avoid surface thrashing.
I finally got a chance to do some more extensive testing when running this patch with the es2 pipeline on Linux.
It works as expected, and from what I saw, using a IntARGB pixelBuffer when no WritableImage is provided avoids swapping around pixel components, like under d3d.
I've also verified than the patch's behaviour is correct when a writable image is provided as a parameter to Node.snapshot, whether the underlying image is actually a PlatformImage or a wrapper for a PixelBuffer, which corrects another limitation of the previous implementation.
More information about the openjfx-dev