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

[Kevin Rushforth]

On Sun, 26 Jan 2020 11:58:27 GMT, Frederic Thevenet <github.com+7450507+fthevenet at openjdk.org> 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?
>> 
>> Simply by watching the value of  `tile.getPixelReader().getPixelFormat()` and `wimg.getPixelWriter().getPixelFormat()` before doing the copy at https://github.com/openjdk/jfx/blob/4bc4417356ebd639567d315257a6bbe11344d9c2/modules/javafx.graphics/src/main/java/javafx/scene/Scene.java#L1315
>>> 
>>> > Unfortunately it seems the only way to choose the pixel format for a `WritableImage` is to initialize it with a `PixelBuffer`, but then one can no longer use a `PixelWriter` to update it...
>>> 
>>> You can update it with `PixelBuffer#updateBuffer`. I think that you will want to pass the stitched tile as the dirty region.
>> 
>> Thanks. I'll look into it.
> 
>> 
>> 
>> > 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 result, but you can try to replace the loops with this:
>> 
>> ```
>> IntStream.range(0, h).parallel().forEach(y -> {    
>>     IntStream.range(0, w).parallel().forEach(x -> {
>>         int pixel = srcarr[srcoff++];              
>>         dstarr[dstoff++] = (byte) (pixel      );   
>>         dstarr[dstoff++] = (byte) (pixel >>  8);   
>>         dstarr[dstoff++] = (byte) (pixel >> 16);   
>>         dstarr[dstoff++] = (byte) (pixel >> 24);   
>>     });                                            
>>     srcoff += srcscanints;                         
>>     dstoff += dstscanbytes;                        
>> });                                                
>> ```
> 
> I don't think this works as it is, as all threads race to increment `srcoff` and `dstoff`.

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 performance regressions for those cases that will now be tiled, but formerly weren't, I no longer think this is a good candidate for openjfx14. This PR should be retargeted to the `master` branch for openjfx15.

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

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