[OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

[Jim Graham]

Hi Jay,

The new code looks good except for one minor indentation problem on the 
argument list of the method after you added the private keyword.  Either 
back out the keyword or re-indent the argument list continuation line to 
match either the "(" of the method or the standard indent.

And, VolatileImage.getSnapshot() should help with testing VolatileImage 
results...

			...jim

On 3/3/2016 6:13 AM, Jayathirth D V wrote:
> Hi,
>
> Thanks for pointing to logical mistake I made Jim. I have made changes to use bg color transparency to determine what type of buffered image should be used in makeBufferedImage().
>
> Also based on Sergey's suggestions included other source types like ARGB_PRE in test case and made makeBufferedImage() API private since it is only used in DrawImage.java. But I was not able to include VolatileImage as source type as I didn't find a way to automate the test to verify alpha value after drawImage(), since there is no API to get pixel value from VolatileImage. While debugging I was able to make sure that in case VolatileImage also proper type is selected before makeBufferedImage() is called.
>
> Please review updated webrev:
> http://cr.openjdk.java.net/~jdv/8139183/webrev.02/
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Jim Graham
> Sent: Thursday, March 03, 2016 5:19 AM
> To: Jayathirth D V; 2d-dev at openjdk.java.net; Philip Race; Prasanta Sadhukhan
> Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel
>
> The logic here has mixed up the opacities and what needs to be done about them.
>
> If the source image is opaque, then this is not a BG operation at all because the bg color would not show through an opaque image, so checking the srcData is both wrong and should be a NOP here.  If we get into that block with a srcData that is opaque then something has gone wrong somewhere else.  In particular, isBgOp should have returned false in that case.
>
> The transparency that matters is when the bg color has transparency and that is not what is being tested here.  The test for xRGB and ARGB should be using the bg color...
>
> 			...jim
>
> On 3/2/16 5:17 AM, Jayathirth D V wrote:
>> Hi,
>>
>> I have updated the changes to select proper Buffer Image type based on
>> source transparency and not just using ARGB directly.
>>
>> Please find the updated webrev for review:
>>
>> http://cr.openjdk.java.net/~jdv/8139183/webrev.01/
>>
>> Thanks,
>>
>> Jay
>>
>> *From:* Jayathirth D V
>> *Sent:* Wednesday, March 02, 2016 5:02 PM
>> *To:* 2d-dev at openjdk.java.net; Philip Race; Prasanta Sadhukhan
>> *Subject:* Review Request for JDK-8139183 : drawImage misses
>> background's alpha channel
>>
>> Hi,
>>
>> _Please review the following fix in JDK9:_
>>
>> __
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8139183
>>
>> Webrev : http://cr.openjdk.java.net/~jdv/8139183/webrev.00/
>>
>> Issue : When we scale any buffered image using drawImage() API which
>> takes scale coordinates we are losing alpha channel in background color.
>>
>> Root cause : We are creating opaque temporary image when we have
>> background color and scale is happening in renderImageXform() API of
>> DrawImage.java. By making it opaque we are losing translucency.
>>
>> Solution : Instead of creating opaque RGB temporary image use ARGB
>> temporary image to maintain translucency of image.
>>
>> Thanks,
>>
>> Jay
>>