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

Jim Graham james.graham at oracle.com
Tue Oct 28 19:00:32 UTC 2014


He'd have to grab the data buffer on a BufferedImage as well to prevent 
acceleration.

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. 
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...

			...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