On Tue, 30 Jun 2020 21:22:14 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> Frederic Thevenet has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Prevent attempt to render tiles with a 0 sized dimension.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/QuantumToolkit.java
>  line 1638:
> 
>> 1637:                         renderWholeImage(x, y, w, h, rf, pImage);
>> 1638:                     }
>> 1639:                     params.platformImage = pImage;
> 
> I tried to write this code using for loop to make it little easy for reader 
> to understand. This is just a suggestion, I
> leave it to you to whether replace it or not,
> int mTileWidth = computeTileSize(w, maxTextureSize);
> int mTileHeight = computeTileSize(h, maxTextureSize);
> IntBuffer buffer = IntBuffer.allocate(mTileWidth * mTileHeight);
> 
> int mTileXOffset = 0;
> int mTileYOffset = 0;
> for (mTileXOffset = 0; (mTileXOffset + mTileWidth) <= w; mTileXOffset += 
> mTileWidth) {
>       for (mTileYOffset = 0; (mTileYOffset + mTileHeight) <= h; mTileYOffset 
> += mTileHeight) {
>               renderTile(x, mTileXOffset, y, mTileYOffset, mTileWidth, 
> mTileHeight,
>                               buffer, rf, tileRttCache, pImage);
>       }
> }
> 
> int rTileXOffset = mTileXOffset;
> int rTileWidth = w - rTileXOffset;
> if (rTileWidth > 0) {
>       for (int rTileYOffset = 0; (rTileYOffset + mTileHeight) <= h; 
> rTileYOffset += mTileHeight) {
>               renderTile(x, rTileXOffset, y, rTileYOffset, rTileWidth, 
> mTileHeight,
>                               buffer, rf, tileRttCache, pImage);
>       }
> }
> 
> int bTileYOffset = mTileYOffset;
> int bTileHeight = h - bTileYOffset;
> if (bTileHeight > 0) {
>       for (int bTileXOffset = 0; (bTileXOffset + mTileWidth) <= w; 
> bTileXOffset += mTileWidth) {
>               renderTile(x, bTileXOffset, y, bTileYOffset, mTileWidth, 
> bTileHeight,
>                               buffer, rf, tileRttCache, pImage);
>       }
> }
> 
> if (rTileWidth > 0 &&  bTileHeight > 0) {
>       renderTile(x, rTileXOffset, y, bTileYOffset, rTileWidth, bTileHeight,
>                       buffer, rf, tileRttCache, pImage);
> }

Thanks for you suggestion.

The original code is still too fresh in my mind for me to really be able to say 
which version is easier to understand,
so I'll let others with a fresh pair of eyes be the judge of that. However I 
think your code is arguably more concise -
and overall more elegant - so I'd be happy to swap my version for yours, 
provided it is considered at least as readable
as the original. WDYT?

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

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

Reply via email to