[Rev 04] RFR: 8238954: Improve performance of tiled snapshot rendering
Frederic Thevenet
github.com+7450507+fthevenet at openjdk.java.net
Wed Jun 17 11:50:59 UTC 2020
On Mon, 15 Jun 2020 15:55:25 GMT, Frederic Thevenet <github.com+7450507+fthevenet at openjdk.org> wrote:
>> Overall, this looks quite good. In particular the tiled rendering, as implemented by the `renderTile` method, should be
>> reasonably efficient.
>> My only high-level comment is that I'm somewhat skeptical of `computeOptimumTileSize` to determine the size and
>> direction of tiling. I note that in the case of an image that is tiled in both X and Y, there are at most 4 distinct
>> tile sizes if it doesn't fit evenly. In the case where only one of X or Y is tiled, there are at most 2 distinct tile
>> sizes. Here is an example: +-----------+-----------+ . +-------+ | | | . | |
>> | M | M | . | R |
>> | | | . | |
>> +-----------+-----------+ . +-------+
>> | | | . | |
>> | M | M | . | R |
>> | | | . | |
>> +-----------+-----------+ . +-------+
>> . . . .
>> +-----------+-----------+ . +-------+
>> | | | . | |
>> | M | M | . | R |
>> | | | . | |
>> +-----------+-----------+ . +-------+
>> | B | B | . | C |
>> +-----------+-----------+ . +-------+
>>
>> Where `M` represents the middle set of tiles each with a size of `tileW x tileH`. `R` is the right hand column of
>> tiles, `B` is bottom row, and `C` is corner.
>> Recognizing this, I wonder if it might be better to always use the maximum tile size, but fill all of the middle tiles
>> of that size first, and then pick up the right and/or bottom edges as needed. This will minimize thrashing (no more
>> than 3 changes of tile size), while avoiding the more complicated logic that tries to keep the tiles all the same size
>> at the cost of smaller tiles, and which has to fall back to using uneven tiles anyway. If you do it this way, there is
>> also no need to have code that switches the order of the inner loop. It will naturally handle that. Either way, I'd
>> like to see some additional system tests that cover all of the cases of X and Y fitting/not-fitting exactly (and if you
>> stick with your current approach, X or Y as the inner loop). I left a couple inline comments as well.
>
>> [...] I'd like to see some additional system tests that cover all of the cases of X and Y fitting/not-fitting exactly
>> (and if you stick with your current approach, X or Y as the inner loop).
>
> What kind of tests do you have in mind? More specifically do you mean simply adding tests that expand on the existing
> `doTestSnapshotScaleNodeDefer`and `doTestSnapshotScaleNodeImm` (which basically just prove that taking a snapshot
> returns a non-null image of the expected size)? Or do you think we need to include a test that proves the snapshot
> produced by tiling is entirely faithful to the original, pixel-wise?
I went ahead and wrote a bunch of tests that:
1. Setup a scene to display an `ImageView` of a selected dimensions chosen to trigger tiling in different ways when
taking snapshots. 2. Fill up the image with noise.
3. Take a snapshot and do a pixel-wise comparison with the original image.
I've added the new tests to the existing `Snapshot2Test.java`.
-------------
PR: https://git.openjdk.java.net/jfx/pull/112
More information about the openjfx-dev
mailing list