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

[prasanta sadhukhan]

Looks good to me. Only thing that can be improved is instead of wildcard 
imports, you can have specific imports in testcase.

Regards
Prasanta
On 3/10/2016 11:53 AM, Jayathirth D V wrote:
> Jim thanks for the review.
>
> Prasanta I have updated the webrev to address indentation issues. Please review:
> http://cr.openjdk.java.net/~jdv/8139183/webrev.06/
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Jim Graham
> Sent: Thursday, March 10, 2016 5:28 AM
> To: Jayathirth D V
> Cc: 2d-dev at openjdk.java.net
> Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel
>
> Also, line 59, "if{" => "if {"...
>
> 			...jim
>
> On 3/9/16 3:30 PM, Jim Graham wrote:
>> That looks good to go.
>>
>> A couple of code indentation issues in the test case:
>>
>> Per:
>> http://www.oracle.com/technetwork/java/javase/documentation/codeconven
>> tions-142311.html#454
>>
>>
>> for statements should have a space as in "for (...)" otherwise it
>> looks like a call to a function named "for()".
>>
>> do-while statements also need a space before/after the braces and
>> between while and the parentheses, as in:
>>       do {
>>       } while (...);
>>
>> I don't need to review those code style issues, but please fix them
>> before you push it...
>>
>>               ...jim
>>
>> On 3/8/16 10:44 PM, Jayathirth D V wrote:
>>> Hi Jim,
>>>
>>> I have made changes mentioned by you.
>>>
>>> Please find updated webrev:
>>> http://cr.openjdk.java.net/~jdv/8139183/webrev.05/
>>>
>>> Thanks,
>>> Jay
>>>
>>> -----Original Message-----
>>> From: Jim Graham
>>> Sent: Wednesday, March 09, 2016 3:23 AM
>>> To: Jayathirth D V
>>> Cc: 2d-dev at openjdk.java.net
>>> Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 :
>>> drawImage misses background's alpha channel
>>>
>>> Hi Jay,
>>>
>>> On 3/8/16 12:56 AM, Jayathirth D V wrote:
>>>> Hi Jim,
>>>>
>>>> Thanks for the review. I have made changes in test case based on
>>>> input provided.
>>>> 1) Added contentsLost() after we drawImage() and getSnapshot() to
>>>> verify the content and if it is lost loop again.
>>> Unfortunately, if contents are lost you need to loop back to where
>>> you create the image, which means you don't have enough code inside
>>> the while loop.  All of the items I mentioned below in my comment
>>> need to be included in that "loop back around".  To be painfully
>>> clear here, the loop should look like this:
>>>
>>>        backgroundColor = ...;  /* no need to create this inside the
>>> loop */
>>>        do {
>>>            img = ...;
>>>            Graphics2D imgGraphics = img.createGraphics();
>>>            // fill img with transparency
>>>            imgGraphics.drawImage(..., bgColor);
>>>            img.getSnapShot();
>>>            imgGraphics.dispose();
>>>        } while (img.contentsLost());
>>>
>>> The rest looked fine...
>>>
>>>              ...jim
>>>