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

[Jim Graham]

Thanks!

		...jim

On 3/10/16 10:19 PM, Jayathirth D V wrote:
> Hi Jim,
>
> I have replaced wildcard imports with specific import and shared webrev to Prasanta before pushing. Please find the webrev that is pushed for reference.
>
> http://cr.openjdk.java.net/~jdv/8139183/webrev.07/
>
> Thanks,
> Jay
> -----Original Message-----
> From: Jim Graham
> Sent: Friday, March 11, 2016 2:38 AM
> To: prasanta sadhukhan; 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
>
> Yes, that is the only issue I see now as well...
>
> 			...jim
>
> On 3/9/16 10:39 PM, prasanta sadhukhan wrote:
>> 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/codeconv
>>>> en
>>>> 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
>>>>>
>>