Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8238954: Improve performance of tiled snapshot rendering #112

Closed
wants to merge 20 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b914a48
Revert "8088198: Exception thrown from snapshot if dimensions are lar…
fthevenet Jan 31, 2020
e13b5a0
Use tiling in QuantumToolkit::renderToImage when requested image is l…
fthevenet Feb 5, 2020
9ec2d25
Added comments to computeOptimumTileSize. Also, Ensure isDivExact is …
fthevenet Mar 6, 2020
9c5b0d4
Avoid useless width and height calculation
fthevenet Mar 9, 2020
50ac8c2
Fixed code style and typo following review.
fthevenet Mar 14, 2020
7f5f289
Revert changes in import statements
fthevenet Mar 17, 2020
8c92ad4
Removed fully qualified name when declaring RTTexture
fthevenet Jun 12, 2020
15b261e
Fixed typo in comment
fthevenet Jun 12, 2020
1fb04e5
Use boolean[] instead of AtomicBoolean to effec pass-by-reference
fthevenet Jun 12, 2020
0642096
Revert changes to import statements
fthevenet Jun 12, 2020
8f0c2d2
Changed tile walking logic
fthevenet Jun 12, 2020
2951d53
Added tiled snapshots tests
fthevenet Jun 16, 2020
8274631
Removed trailing whitespaces
fthevenet Jun 17, 2020
4752d83
Changed wrong value for image size (for these tests we *don't* want i…
fthevenet Jun 17, 2020
a01aaa2
Crawling pixels in writableImage with parallel stream is a bad idea.
fthevenet Jun 19, 2020
efccc4d
Fill test image with a bilinear gradient instead of random noise.
fthevenet Jun 29, 2020
c0f7d14
Prevent attempt to render tiles with a 0 sized dimension.
fthevenet Jun 30, 2020
d808307
- Removed unused imports in Snapshot2Test
fthevenet Jul 1, 2020
7568d9c
Using for loops instead of while
fthevenet Jul 1, 2020
58cc200
Mark variables as final
fthevenet Jul 1, 2020
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -1520,29 +1520,17 @@ private void renderWholeImage(int x, int y, int w, int h, ResourceFactory rf, Qu
rt.unlock();
}

private int computeOptimumTileSize(int size, int maxSize) {
return computeOptimumTileSize(size, maxSize, null);
}

private int computeOptimumTileSize(int size, int maxSize, boolean[] isDivExact) {
// This method attempts to find the smallest exact divider for the provided `size`
// while the result of the division is less than `maxSize`.
// It tests all potential dividers from 2 to 6 and returns the result of the division
// if all conditions can be satisfied or, failing that, `maxSize`.
// If non-null, the value for `isDivExact` is set so as to reflect whether or not
// an exact divider could be found.
for (int n = 2; n <= 6; n++) {
private int computeTileSize(int size, int maxSize) {
// If 'size' divided by either 2 or 3 produce an exact result
// and is lesser that the specified maxSize, then use this value
// as the tile size, as this makes the tiling process more efficient.
for (int n = 1; n <= 3; n++) {
int optimumSize = size / n;
if (optimumSize <= maxSize && optimumSize * n == size) {
if (isDivExact != null && isDivExact.length > 0) {
isDivExact[0] = true;
}
return optimumSize;
}
}
if (isDivExact != null && isDivExact.length > 0) {
isDivExact[0]= false;
}
return maxSize;
}

@@ -1579,49 +1567,67 @@ public void run() {
pImage.setImage(com.sun.prism.Image.fromIntArgbPreData(IntBuffer.allocate(w * h), w, h));
}
// Find out if it is possible to divide up the image in tiles of the same size
This conversation was marked as resolved by fthevenet

This comment has been minimized.

@kevinrushforth

kevinrushforth Jun 30, 2020
Member

Very minor: this comment seems like a hold-over from when the method was computing an optimum size. Now the caller doesn't know about "tiles of the same size".

int tileWidth = computeOptimumTileSize(w, maxTextureSize);
var exactHeightDivFound = new boolean[]{false};
int tileHeight = computeOptimumTileSize(h, maxTextureSize, exactHeightDivFound);
int tileWidth = computeTileSize(w, maxTextureSize);
int tileHeight = computeTileSize(h, maxTextureSize);
IntBuffer buffer = IntBuffer.allocate(tileWidth * tileHeight);
// In order to minimize the number of time we have to resize the underlying
// surface for capturing a tile, choose a dimension that has an exact divider
// (if any) to be processed in the inner most loop.
// E.g. looping on width then height in the example bellow requires four
// surface resizing, whereas the opposite requires only two:
//
// for (w;;) for (h;;)
// for(h;;) for(w;;)
// ----------------- -----------------
// | | | | | |
// | 1 | 3 | | 1 | 2 |
// h | | | h | | |
// ----------------- -----------------
// | 2 | 4 | | 3 | 4 |
// ----------------- -----------------
// w w



if (exactHeightDivFound[0]) {
for (int xOffset = 0; xOffset < w; xOffset += tileWidth) {
tileWidth = Math.min(tileWidth, w - xOffset);
for (int yOffset = 0; yOffset < h; yOffset += tileHeight) {
tileHeight = Math.min(tileHeight, h - yOffset);
renderTile(x, xOffset, y, yOffset, tileWidth, tileHeight,
buffer, rf, tileRttCache, pImage);
}
}
} else {
for (int yOffset = 0; yOffset < h; yOffset += tileHeight) {
tileHeight = Math.min(tileHeight, h - yOffset);
for (int xOffset = 0; xOffset < w; xOffset += tileWidth) {
tileWidth = Math.min(tileWidth, w - xOffset);
renderTile(x, xOffset, y, yOffset, tileWidth, tileHeight,
buffer, rf, tileRttCache, pImage);
}

// 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 the bottom row,
// C is the corner:
// +-----------+-----------+ . +-------+
// | | | . | |
// | M | M | . | R |
// | | | . | |
// +-----------+-----------+ . +-------+
// | | | . | |
// | M | M | . | R |
// | | | . | |
// +-----------+-----------+ . +-------+
// . . . .
// +-----------+-----------+ . +-------+
// | B | B | . | C |
// +-----------+-----------+ . +-------+

// Walk through all same-size "M" tiles
int xOffset = 0;
int yOffset = 0;
var mTileWidth = tileWidth;
var mTileHeight = tileHeight;
while (mTileWidth == tileWidth && xOffset < w) {
yOffset = 0;
mTileHeight = tileHeight;
while (mTileHeight == tileHeight && yOffset < h) {
renderTile(x, xOffset, y, yOffset, mTileWidth, mTileHeight,
buffer, rf, tileRttCache, pImage);
yOffset += tileHeight;
mTileHeight = Math.min(tileHeight, h - yOffset);
}
xOffset += tileWidth;
mTileWidth = Math.min(tileWidth, w - xOffset);
}
} else {
// Walk through remaining same-width "B" tiles, if any
int bOffset = 0;
int bTileHeight = tileHeight;
while (bTileHeight == tileHeight && bOffset < h) {
This conversation was marked as resolved by fthevenet

This comment has been minimized.

@kevinrushforth

kevinrushforth Jun 29, 2020
Member

It looks like the "B" and the "R" loops are reversed. This isn't causing any actual problems, but is confusing since it doesn't match the comment or the diagram above. The comment for this block says "B" tiles, but actually, it is the "R" tiles in the diagram that this is looping over. At the end of the main loop, mTileWIdth and mTileHeight will be set to the size of the corner tile. Given this, the tiles of mTileWidth X tileHeight will be the right hand column.

Once you fix this, you will need to surround this while loop in a check for if (mTileWidth > 0)

This comment has been minimized.

@fthevenet

fthevenet Jun 30, 2020
Author Member

You're right: the comments and variable names are swapped. Will fix it (and obviously add the extra condition that prevents attempts to render 0 width/height tiles).
Thanks.

renderTile(x, xOffset, y, bOffset, mTileWidth, bTileHeight, buffer, rf, tileRttCache, pImage);
bOffset += tileHeight;
bTileHeight = Math.min(tileHeight, h - bOffset);
}
// Walk through remaining same-height "R" tiles, if any
int rOffset = 0;
int rTileWidth = tileWidth;
while (rTileWidth == tileWidth && rOffset < w) {
This conversation was marked as resolved by fthevenet

This comment has been minimized.

@kevinrushforth

kevinrushforth Jun 29, 2020
Member

Similarly, you will need to surround this while loop in a check for if (mTileHeight > 0)

renderTile(x, rOffset, y, yOffset, rTileWidth, mTileHeight, buffer, rf, tileRttCache, pImage);
rOffset += tileWidth;
rTileWidth = Math.min(tileWidth, w - rOffset);
}
// Render corner "C" tile if needed
if (bOffset > 0 && rOffset > 0) {
This conversation was marked as resolved by fthevenet

This comment has been minimized.

@kevinrushforth

kevinrushforth Jun 29, 2020
Member

I might recommend to also add a check for mTileWidth > 0 && mTileHeight > 0

renderTile(x, rOffset, y, bOffset, rTileWidth, bTileHeight, buffer, rf, tileRttCache, pImage);
}
}
else {
// The requested size for the screenshot fits max texture size,
This conversation was marked as resolved by fthevenet

This comment has been minimized.

@arapte

arapte Jun 30, 2020
Member

Very minor: screenshot should be changed to snapshot.

// so we can directly render it in the target image.
renderWholeImage(x, y, w, h, rf, pImage);
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.