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

Jim Graham james.graham at oracle.com
Sat Nov 1 03:23:41 UTC 2014


That looks good.  Approved...

	...jim

On 10/29/2014 7:33 AM, Sergey Bylokhov wrote:
> 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
>>>>>>
>>>>
>>>>
>>>
>
>



More information about the 2d-dev mailing list