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

[Sergey Bylokhov]

Hi, Jim.
The new version:
http://cr.openjdk.java.net/~serb/8062164/webrev.03
On 28.10.2014 22:00, Jim Graham wrote:
> I wasn't aware that jtreg had a default timeout, but 2 minutes is 
> rather long.  It's one thing if you are waiting for a slow operation, 
> but in this case I'm concerned about running on a machine that somehow 
> faults its VolatileImages frequently, or somehow they just fail on 
> creation.
I was under impression that in this case we should use VI implementation 
based on bufimg?
> That would mean that any test that uses this pattern takes 2 minutes 
> to complete wherein it has looped several million times in a loop that 
> is really only there for theoretical matters and should probably fail 
> if it can't complete in a couple of iterations...
It is too optimistic. On my system it will be hundreds of iterations.
>
>             ...jim
>
> On 10/28/14 10:28 AM, Phil Race wrote:
>> 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
>>>>>
>>>
>>>
>>


-- 
Best regards, Sergey.