Hi Jim, I have updated the code to remove indentation problem and added VolatileImage also for testing.
Please find the updated webrev for review : http://cr.openjdk.java.net/~jdv/8139183/webrev.03/ Thanks, Jay -----Original Message----- From: Jim Graham Sent: Friday, March 04, 2016 5:21 AM To: Jayathirth D V; Sergey Bylokhov Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel 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@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@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 >>
