<AWT Dev> [8] Review request for 7153339: InternalError when drawLine with Xor and Antialiasing

[Oleg Pekhovskiy]

Hi Jim,

first of all I should say, that I prepared that fix for 7u as the most 
safe, with minimum changes.
I agree that getReplacement() should return a valid sufrace or null, but 
it doesn't happen during that switch.

That CR was assigned to me because my previous changes for:
7112642 Incorrect checking for graphics rendering object
7121482 some sun/java2d and sun/awt tests failed with InvalidPipeException
discovered the problem with getReplacement() and were somehow related.

But maybe it would be better to create a separate CR for 
getReplacement() issue and assign it to Java2d Team?
I also could add a comment for "!surfaceData.isValid()" reffering to 
CR7153339.

What do you think?

Thanks,
Oleg

24.08.2012 4:04, Jim Graham wrote:
> Hi Oleg,
>
> getReplacement should not be returning an invalid surface.  If the 
> current D3DSD is invalid, then it should return a valid replacement. 
> The only time it should return null is when the window is gone.  It 
> sounds like the window isn't gone here, it has just switched over to a 
> non-D3D type of SurfaceData and return that...
>
>             ...jim
>
> On 8/23/2012 4:37 PM, Oleg Pekhovskiy wrote:
>> Hi Phil, Jim,
>>
>> thank you for pointing out the testing work that should be performed.
>> I tested my fix with the following regression tests:
>> test/java/awt/Graphics
>> test/java/awt/Graphics2D
>> test/java/awt/GraphicsDevice
>> test/java/awt/GraphicsEnvironment
>> test/sun/java2d
>>
>> Plus I tested performance differences on:
>> demo/jfc/Java2D/Java2Demo.jar
>>
>> Testing was done on Windows 7 & Ubuntu 12.04 LTS.
>> No differences were found.
>>
>> I also asked Yuri Nesterenko to test all that stuff on Mac.
>>
>> Thanks,
>> Oleg
>>
>>
>> 11.08.2012 3:10, Phil Race wrote:
>>> Oleg,
>>> This looks OK to me but since this is a shared code change I have
>>> to ask what testing you've done ?
>>>
>>> Also why not provide a regression test ? The provided test was
>>> interactive but I think it can be automated.
>>>
>>> You need another review and I'd like Jim to take a look.
>>>
>>> -phil.
>>>
>>> On 8/10/2012 2:26 PM, Oleg Pekhovskiy wrote:
>>>> Hi,
>>>>
>>>> Please review the fix for CR:
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7153339
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~bagiras/8/7153339.1/
>>>>
>>>> Comments:
>>>> XOR is not supported for D3D (see comments inside
>>>> D3DSurfaceData.validatePipe()) and
>>>> software rendering is used invalidating current D3DSurfaceData.
>>>> So we have situation when component's peer has invalid SurfaceData
>>>> and it's retrieved in
>>>> SunGraphics2D.revalidateAll() through SurfaceData.getReplacement()
>>>> without any check.
>>>> That's why I added validity check there.
>>>>
>>>> Thanks,
>>>> Oleg
>>>>
>>>> <http://cr.openjdk.java.net/%7Ebagiras/8/7153339.1/>
>>>
>>
>>