On Fri, 29 Jan 2021 00:05:57 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

> As indicated in the JBS bug, using a `PixelReader` to read a scaled image in 
> HiDPI mode, for example an `@2x` image, to read more than one pixel will read 
> data from the wrong location in the image, usually leading to an IOOBE.
> 
> The bug is in the `getPixelAccessor` method of Prism Image. The method 
> correctly creates a new `Accessor` if one hasn't already been created, but 
> then it unconditionally wraps the current `Accessor` in a 
> `ScaledPixelAccessor` if the scale is > 1. So the second time this method is 
> called, it created another `ScaledPixelAccessor` that wraps the first 
> `ScaledPixelAccessor`, meaning that the indexes into the pixel array are 
> multiplied by 4. This continues for each new call to this method.
> 
> The fix is to only wrap an `Accessor` in a `ScaledPixelAccessor` the first 
> time, when it is created.
> 
> I added a test, which is run for both scaled and unscaled images, to ensure 
> that we get the right value when reading a pixel, and that reading it a 
> second time returns the same result. Without the fix, the new test will fail 
> with IOOBE when the scale factor is two. Related to this, I initially 
> commented out the wrapping in a  `ScaledPixelAccessor` entirely, just to see 
> what would happen, and none of the existing tests caught it. The new test 
> will catch this.

modules/javafx.graphics/src/main/java/com/sun/prism/Image.java line 654:

> 652:             }
> 653:             if (pixelScale != 1.0f) {
> 654:                 pixelaccessor = new ScaledAccessor<>(pixelaccessor, 
> pixelScale);

is that really correct? I think you should not overwrite save the scaled 
pixelAccessor in the instance variable - if I read that correct with this 
change the following happens: if I call getPixelAccessor() 3 times I get a 
ScaledAccessor wrapping a ScaleAccessor wrapping a ScaledAccessor, wrapping one 
of ByteAccess, ByteRgbAccess, IntAccess

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

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

Reply via email to