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

2020-01-28 Thread Kevin Rushforth
On Tue, 28 Jan 2020 16:00:35 GMT, Frederic Thevenet wrote: >> I have tested using a recipient WritableImage with an IntARGB pixel format >> (so that is aligned with that of the tile), that I construct by supplying a >> PixelBuffer, and as expected that performance of the tiled code is much

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

2020-01-28 Thread Frederic Thevenet
On Mon, 27 Jan 2020 18:45:17 GMT, Frederic Thevenet wrote: >> I thought of one possibility that might be worth looking into for a short >> term fix (i.e., could still go into openjfx14). Rather than relying on >> `PrismSettings.maxTextureSize` you could instead try to render the entire >>

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

2020-01-27 Thread Frederic Thevenet
On Mon, 27 Jan 2020 15:30:27 GMT, Kevin Rushforth wrote: >> I would be very cautious of using multi-threading here. In any case, I think >> that the issues around absolute performance could be handled separately. >> >> Having said that, given my review comments, along with the concerns over

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

2020-01-27 Thread Kevin Rushforth
On Mon, 27 Jan 2020 13:47:43 GMT, Kevin Rushforth wrote: >>> >>> >>> > profiling a run of the benchmark shows that a lot of time is spent into >>> > `IntTo4ByteSameConverter::doConvert` >>> >>> This is a bit naive, but what if you parallelize the code there? I didn't >>> test that this

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

2020-01-27 Thread Kevin Rushforth
On Sun, 26 Jan 2020 11:58:27 GMT, Frederic Thevenet wrote: >>> >>> >>> > the `WriteableImage` used to collate the tiles and the tiles returned >>> > from the `RTTexture` have different pixel formats (`IntARGB` for the tile >>> > and `byteBGRA` for the `WriteableImage`). >>> >>> Where did

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

2020-01-26 Thread Frederic Thevenet
On Sun, 26 Jan 2020 11:40:06 GMT, Frederic Thevenet wrote: >>> the `WriteableImage` used to collate the tiles and the tiles returned from >>> the `RTTexture` have different pixel formats (`IntARGB` for the tile and >>> `byteBGRA` for the `WriteableImage`). >> >> Where did you see these? >>

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

2020-01-26 Thread Frederic Thevenet
On Sat, 25 Jan 2020 20:24:54 GMT, Nir Lisker wrote: >>> profiling a run of the benchmark shows that a lot of time is spent into >>> `IntTo4ByteSameConverter::doConvert` >> >> This is a bit naive, but what if you parallelize the code there? I didn't >> test that this produces the correct

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

2020-01-25 Thread Nir Lisker
On Sat, 25 Jan 2020 19:55:23 GMT, Nir Lisker wrote: >> With regard as to why the tiling version is significantly slower, though, I >> do have a pretty good idea; as Kevin hinted, the pixel copy into a temporary >> buffer before copying into the final image is where most the extra time is >>

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

2020-01-25 Thread Nir Lisker
On Fri, 24 Jan 2020 17:16:13 GMT, Frederic Thevenet wrote: >> I don't, to be honest. >> The results for some dimensions (not always the same) can vary pretty >> widely from one run to another, despite all my effort to repeat results and >> remove outliers. >> Out of curiosity, I also tried

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

2020-01-24 Thread Frederic Thevenet
On Fri, 24 Jan 2020 16:55:47 GMT, Frederic Thevenet wrote: >>> Here are the results when running JavaFX 14-ea+7. >>> The columns of the table correspond the width of the target snapshot, while >>> the rows correspond to the height and the content of the cells is the >>> average time* spent

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

2020-01-24 Thread Frederic Thevenet
On Fri, 24 Jan 2020 16:34:29 GMT, Nir Lisker wrote: >> And here are the results with the change in this PR, on the same machine >> under Windows 10: >> >> || 1024 |2048 |3072 |4096 |5120 |6144 |7168 |8192 | >> |---|---|---|---|---|---|---|---|---| >> | 1024 | 6.957774 | 10.461498 |

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

2020-01-24 Thread Nir Lisker
On Fri, 24 Jan 2020 16:26:38 GMT, Frederic Thevenet wrote: >> Here are the results when running JavaFX 14-ea+7. >> The columns of the table correspond the width of the target snapshot, while >> the rows correspond to the height and the content of the cells is the >> average time* spent (in

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

2020-01-24 Thread Frederic Thevenet
On Fri, 24 Jan 2020 16:03:39 GMT, Frederic Thevenet wrote: >> I've put together a small benchmark to measure the time it takes to >> snapshots into images of sizes varying from 1024*1024 to 8192*8192, by >> increment of 1024 in each dimension, which you can find here: >>

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

2020-01-24 Thread Frederic Thevenet
On Fri, 24 Jan 2020 15:55:50 GMT, Frederic Thevenet wrote: >> I haven't done any testing yet, but I have two comments on the patch: >> >> 1. Using the clamped texture size as the upper limit is the right thing to >> do, but `Prism.maxTexture` isn't guaranteed to be that size. The >>

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

2020-01-24 Thread Frederic Thevenet
On Tue, 21 Jan 2020 21:53:29 GMT, Kevin Rushforth wrote: >>> >>> >>> 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,

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

2020-01-21 Thread Kevin Rushforth
On Mon, 20 Jan 2020 11:24:05 GMT, Frederic Thevenet wrote: >> 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

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

2020-01-20 Thread Frederic Thevenet
On Mon, 20 Jan 2020 05:06:50 GMT, Ambarish Rapte 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

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

2020-01-19 Thread Ambarish Rapte
On Mon, 20 Jan 2020 05:07:11 GMT, Frederic Thevenet wrote: >> This PR aims to address the following issue: JDK-8088198 Exception thrown >> from snapshot if dimensions are larger than max texture size >> >> In order to do that, it simply captures snapshots in multiple tiles of >>

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

2020-01-17 Thread Frederic Thevenet
> This PR aims to address the following issue: JDK-8088198 Exception thrown > from snapshot if dimensions are larger than max texture size > > In order to do that, it simply captures snapshots in multiple tiles of > maxTextureSize^2 dimensions (or less, as needed), and then recomposes all the