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

[Oleg Pekhovskiy]

Hi Phil,

I forgot to mention the test creation problem.

The manual test proposed in CR's description just
relies on specific call sequence after getReplacement() returns invalid 
surfaceData.

So the test doesn't catch InvalidPipeException when invalid surfaceData 
found,
because this exception is cought in sun.java2d.SunGraphics2D.drawLine().

But it just catches java.lang.InternalError that is thrown on catch of 
Throwable exception
(indeed InvalidPipeException) that is thrown from:
sun.java2d.SunGraphics2D.validatePipe(SunGraphics2D.java:380)
with the stack:
sun.java2d.SunGraphics2D.revalidateAll(SunGraphics2D.java:2363)
sun.java2d.SunGraphics2D.getCompClip(SunGraphics2D.java:496)
sun.java2d.pipe.LoopPipe.getStrokeSpans(LoopPipe.java:270)
sun.java2d.pipe.LoopPipe.draw(LoopPipe.java:201)
sun.java2d.pipe.PixelToShapeConverter.drawLine(PixelToShapeConverter.java:52)
sun.java2d.pipe.ValidatePipe.drawLine(ValidatePipe.java:62)
sun.java2d.SunGraphics2D.drawLine(SunGraphics2D.java:2137)

Could we rely on such indirect error in our case?
As I remember you had plans to refactor Graphics related stuff, so the 
sequence could change
making test's expectations odd.

Thanks,
Oleg

24.08.2012 3:37, 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/>
>>
>
>