[Rev 03] RFR: 8088198: Exception thrown from snapshot if dimensions are larger than max texture size

[Frederic Thevenet]

On Mon, 20 Jan 2020 05:06:50 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> The pull request has been updated with 2 additional commits.
> 
> Looks good to me.
> Below is just an observation about time taken by the API,
> Platform: Windows10,  `maxTextureSize`: 4096
> For a snapshot of (4096 * n, 4096 * n): each call to `doSnapshotTile()` takes ~100 ms, and each call to `setPixels()` takes ~30 ms.
> 
> Please wait for one more approval before integrating.

> 
> 
> Looks good to me.
> Below is just an observation about time taken by the API,
> Platform: Windows10, `maxTextureSize`: 4096
> For a snapshot of (4096 * n, 4096 * n): each call to `doSnapshotTile()` takes ~100 ms, and each call to `setPixels()` takes ~30 ms.
> 
> Please wait for one more approval before integrating.

Do you have the same kind of measurements for similar uses cases without this patch? I'm expecting performances to remain the same for snapshots less than `maxTextureSize*maxTextureSize` in size, since there is no extra pixel copy in that case, and the rest of the code if globally unchanged.

Still there exists a window for performance regressions, which for instance on Windows 10 w/ the D3D rendering pipeline would be when one of the dimension of a snapshot is >4096  and <8192: in that case a snapshot would require up to 4 extra copy pixels steps with this patch.
This is due to the fact that the old code would accept to render in a texture of a size up to the non-clamped max texture size (8192 in D3D, 16384 in ES2), but because I couldn't find a clean way to access the non clamped maxTextureSize exposed by the render factory from the Scene class, I settled for Prsim.maxTextureSize, which is the clamped value (4096 by default).
I haven't dug deep enough in the code to understand precisely why the max texture size is clamped to 4096 by default, but assuming that it is for a good reason wouldn't that also apply in that case? Or is it always safe to use the non-clamped value in that particular case?

-------------

PR: https://git.openjdk.java.net/jfx/pull/68