[OpenJDK 2D-Dev] [8] request for review: 7124347: [macosx] "java.lang.InternalError: not implemented yet" on call Graphics2D.drawRend

[Jim Graham]

I'm OK with the removal of the test, but I didn't look in more depth. 
Phil already approved from his end, though, so I think you can take this 
as a concurrence...

			...jim

On 11/30/12 1:37 AM, Andrew Brygin wrote:
> Hi Jim,
>
>   I have verified that we use either the imageComp field from SunGraphics2D
>   (which can not be null), or some explicit composite type in this method.
>   So, according to  your suggestion, I have removed  the check for null.
>   Please take a look to updated webrev:
>
>   http://cr.openjdk.java.net/~bae/7124347/8/webrev.01/
>
>   Phil, could you please take a look the the updated fix also?
>
> Thanks,
> Andrew
>
>
> On 30.11.2012 4:48, Jim Graham wrote:
>> Hi Andrew,
>>
>> My belief is that comp should never be null.  A simple grep should
>> confirm that belief.  While I do agree that it can be safer to check
>> for null if you aren't sure about the code base, once one piece of
>> code does it then it spreads and weakens the API contract so it would
>> be better to check and keep the API contract stronger if we can...
>>
>>             ...jim
>>
>> On 11/29/12 12:18 AM, Andrew Brygin wrote:
>>> Hello Jim,
>>>
>>>   I do not think that suggested change allows a null composite per se:
>>>   the check for null was introduced just to make sure that we can safely
>>>   call isDerivedFrom method. However, it does not change the method
>>>   behavior for the case of null composite.
>>>
>>>   If it is impossible to get null composite here for some reason
>>> (unfortunately
>>>   I can not say this for sure), then this check is not required.
>>> However, I do not
>>>   think that the extra check for null may cause any harm.
>>>
>>> Thanks,
>>> Andrew
>>>
>>>
>>> On 29.11.2012 4:07, Jim Graham wrote:
>>>> Is there a reason for allowing a null comp in the isSupported method?
>>>>
>>>>             ...jim
>>>>
>>>> On 11/26/2012 4:38 AM, Andrew Brygin wrote:
>>>>> Hello,
>>>>>
>>>>>   could you please review a fix for 7124347?
>>>>>
>>>>>   This fix does not implement getRater() in ogl surfaces.
>>>>>   Instead, it provides a blit for custom composite, which
>>>>>   prepares a snapshot of the destination surface (which
>>>>>   is one of ogl surfaces) and delegates the work to the
>>>>>   general blit, which now extracts raster from the snapshot.
>>>>>   A result of general blit's work is transferred to the
>>>>>   original destination.
>>>>>
>>>>>   Changes in OGLSurfaceDataProcy are required in order to prevent
>>>>>   getting an accelerated copy of original source image as an
>>>>>   operand of the blit. Now we first check for composite type,
>>>>>   and only then (if composite is not specified, or is a kind
>>>>>   of alpha composite) we take into account other conditions.
>>>>>
>>>>>   So this fix does not change anything for the case of alpha
>>>>> composites
>>>>>   but affects only XOR and custom composites case.
>>>>>
>>>>>   Note that the fix solves the problem only for the case of blits
>>>>>   (i.e. drawImage()). Other operations may result into the same
>>>>>   problem if custom composite is used. However, in case of XOR,
>>>>>   this change solves the problem completely, because other rendering
>>>>>   operations in the ogl pipeline are ready for XOR composite.
>>>>>
>>>>>   Supplied regression test verifies that the fix makes the problem
>>>>> gone.
>>>>>   I have verified both windows and macosx platforms.
>>>>>
>>>>>   Please take a look and share your comments.
>>>>>
>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7124347
>>>>> Webrev: http://cr.openjdk.java.net/~bae/7124347/8/webrev.00/
>>>>>
>>>>> Thanks,
>>>>> Andrew
>>>
>