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

[Kevin Rushforth]

On Wed, 29 Jan 2020 13:59:12 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> The pull request has been updated with 1 additional commit.
> 
> I need to re-review the updated PR.

The code changes look good to me. As long as you are willing to address the follow-on issues for openjfx15, I'll approve this PR for openjfx14, once I run my last test.

I did a fair bit of testing of the functionality, which reinforces the decision to only try tiling as a fallback when an exception occurs, since there could otherwise be functional regressions in addition to performance regressions.

With the latest update to the PR, any case that works today will continue to behave exactly as it does today. The worst that will happen is that the tiling might produce very slightly different results for interpolated colors, or possibly fail with a different exception (e.g., if a 4K x 4K texture is still too big for some hypothetical device, or if the user passes in a `WritableImage` created with a `PixelBuffer` (see below)).

I added a verbose println to snapshot to validate my testing:

1. Snapshot with image size = 5k x 5k (should still fit on Windows D3D with no tiling)
2. Snapshot with image size = 20k x 3k (should tile in X)
3. Snapshot with image size = 3K x 20k (should tile in Y)
4. Snapshot with image size = 20K x 20k (should tile in both X and Y)

RESULT: All of the above work as expected

5. Modify the jfx runtime to force tiling, set the tile size to 128 bytes, and run various tests (e.g., a stress test of tiling)
RESULT: Everything looks visually correct, but four of the tests in `test.robot.test3d.Snapshot3DTest` fail with a color mismatch [can be addressed in a follow-up bug]

6. Pass in a `WritableImage` created with a `PixelBuffer` object (NIO buffer)
RESULT: It works without tiling and throws an exception with tiling [can be addressed in a follow-up bug]

7. Pass in a larger than needed `WritableImage` (e.g., using a viewport that is large enough to cause tiling, but smaller than the passed in image) into snapshot when using tiling. Is the entire image cleared?
RESULT: I still need to run this test, but even if it doesn't work correctly when tiling, it wouldn't be a regression, and could be addressed in a follow-up bug.

Issues to file for follow-on bug(s):
1. Improve performance of tiled snapshot rendering
    * As part of this, we need to get max texture size from the toolkit
    * Consider moving the tiled rendering into `QuantumRenderer` thread
    * Reuse a single temporary buffer rather than creating a new one for each tile
2. Passing in a `WritableImage` created with a `PixelBuffer` object (NIO buffer) fails if tiling is needed. I'll file this one.
3. Pixel accuracy issues with tiled image; this can be seen in `Snapshot3DTest` by instrumenting the code to force tiling with a tile size of 128. It looks OK visually, but fails the color comparison test. I'll file this one.
4. Presuming that the above test 7 produces incorrect results, file a follow-up bug to fix it.

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

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