[OpenJDK 2D-Dev] [9] Review Request: 8062164 Incorrect color conversion, when bicubic interpolation is used

[Phil Race]

On 10/28/2014 5:29 AM, Sergey Bylokhov wrote:
> Hi, Jim.
> Thanks for review!
>
> On 27.10.2014 21:12, Jim Graham wrote:
>> In the test case you call new BufferedImage() with a Transparency 
>> constant which is essentially a random number to determine the type 
>> of BufferedImage since it is expecting its own values.  You get lucky 
>> with the value of the integer as it turns out, but the test case 
>> really should be using the constants from BufferedImage in its own 
>> constructor instead (too bad we didn't have typed enums back when 
>> this was all happening).
>
> BufferedImage was incorrectly used, instead of CompatibleImage:
> http://cr.openjdk.java.net/~serb/8062164/webrev.02

That explains why you went to all the trouble to grab the data buffer.

Looks good.

-phil.

>>
>> Also, a test case with an infinite loop should either have a timeout, 
>> or a limit on "retries" in the loop - just in case we ever run the 
>> test in an environment where VIs fail for some reason...
>
> I prefer to use default timeout, which is 2 minutes in jtreg. This 
> timeout can be tweaked for slow machines for all tests at once.
>>
>>             ...jim
>>
>> On 10/27/14 8:57 AM, Sergey Bylokhov wrote:
>>> Hello.
>>> Please review the fix for jdk 9.
>>> Type of the temporary buffer in DrawImage.renderImageXform() is
>>> incorrect. Note that we blit this buffer to the destination, using next
>>> maskblit/blit:
>>>
>>> line 463: maskblit = MaskBlit.getFromCache(SurfaceType.IntArgbPre,
>>>                                               sg.imageComp,
>>>                                               dstType);
>>> line 489: blit = Blit.getFromCache(SurfaceType.IntArgbPre,
>>>                                       sg.imageComp,
>>>                                       dstType);
>>>
>>> Type of source surface is IntArgbPre in both cases, but the type of
>>> tmpBuffer is IntArgb.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8062164
>>> Webrev can be found at: 
>>> http://cr.openjdk.java.net/~serb/8062164/webrev.01
>>>
>
>