[OpenJDK 2D-Dev] [9] RFR: JDK-8140530, , Creating a VolatileImage with size 0, 0 results in no longer working g2d.drawStri

[prasanta sadhukhan]

Hi Jim,

On 12/3/2015 12:38 AM, Jim Graham wrote:
> Hi Prasanta,
>
> (On a practical note, in the HTML version of your message, the text 
> said "webrev.01", but the link in the href pointed to "webrev.00" so I 
> sat there wondering why the changes you noted weren't there until I 
> realized that I was still looking at webrev.00 and had to manually 
> enter webrev.01 in the browser to see the new code...)
>
> Have you run your new test on all platforms to make sure that it 
> succeeds (by throwing IAE) on all currently supported/tested platorms?
>
IAE was already been thrown for windows and mac. It was not working for 
linux only.
> It seems, from the comment, that one issue is that X11 has a special 
> need in that if we make it through to the native code with 0,0 
> arguments and attempt to create a pixmap of 0,0 then we get an X11 
> error so I'm OK with the native code having its own check for 
> protection against the X11 error. But, for consistency, shouldn't the 
> 0,0 be detected and and IAE thrown at a much higher level shared by 
> all platforms so that no platform can accidentally allow 0,0?  
> Otherwise we have to make sure that each and every current platform 
> and each and every future platform port contains these checks to 
> satisfy the new behavior expectation.
>
> Apparently, somewhere above the native method there is a check that 
> converts OOME to IAE - is that in shared code or in the X11-specific 
> code?
It is not a direct conversion from OOME to IAE. Basically, the Java will 
try to create a accelerated surface and if it cannot, it creates a 
backup BufferedImage via createCompatibleImage() which calls 
createCompatibleWritableRaster() and that function has a check for 0 
width,height which throws IAE.

Code wise flow:
VolatileSurfaceManager.initialize() will check for accelerated surface 
via initAcceleratedSurface() which goes to different pipeline for 
different platforms.

In windows, D3DVolatileSurfaceManager.java#initAcceleratedSurface() will 
call D3DSurfaceData.createData() which calls initSurface() which 
initializes surface by calling "native" initSurfaceNow() which returns 
false for 0 width.height. D3DSurfaceData throws InvalidPipeException to 
D3DVolatileSurfaceManager#initAcceleratedSurface()  which marks 
accelerated surface as null in that case so Java will fall back to 
BufferedImage as explained above.

In mac, CGLVolatileSurfaceManager.jaav#initAcceleratedSurface() will get 
OOM from CGlSurfaceData.createData() which calls native initFBObject() 
which returns false

In linux, native was not throwing any issue even though it does not 
utilise 0 width,height so Java still assumes it is working with 
accelerated surface so will not try to create backup BufferedImage 
surface so cannot throw IAE.
My fix will let native throw OOM to 
XRVolatileSurfaceManager.java#initAcceleratedSurface() so that it knows 
it fails to create accelerated surface and has to fall back to 
BufferedImage surface and then the IAE will be thrown from
createCompatibleWritableRaster()


If you think it's ok, we can catch 0 width,height in SunVolatileImage 
constructor before it calls VolatileSurfacemaanger. initialize() so that 
it gets trapped  at higher level? Please advise.

Regards
Prasanta

>
>             ...jim
>
> On 11/30/15 9:58 PM, prasanta sadhukhan wrote:
>> Modified the testcase to "fail" if IAE is not thrown
>> http://cr.openjdk.java.net/~psadhukhan/8140530/webrev.01
>>
>> Regards
>> Prasanta
>> On 11/30/2015 2:13 PM, prasanta sadhukhan wrote:
>>> Hi All,
>>>
>>> Please review a fix for jdk9
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8140530
>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8140530/webrev.00/
>>>
>>> The issue was creating a volatileImage with 0 width, height does not
>>> result in IllegalArgumentException.
>>> But, when we try to create a non-volatile Image via
>>> GraphicsConfiguration.createCompatibleImage(0,0) or a
>>> BufferedImage(0,0,imagetype) it results in IAE.
>>> So, to maintain consistency across all image w.r.t 0 width,height,
>>> createVolatileImage() should also throw IAE.
>>>
>>> In windows, creating a volatileImage with 0 width,height resulted in
>>> IAE but in linux it does not.
>>>
>>> This is because XCreatePixmap() generate BadValue unless width,height
>>> is nonzero but the error handler does not catch it.
>>> https://tronche.com/gui/x/xlib/pixmap-and-cursor/XCreatePixmap.html
>>> [The width and height arguments must be nonzero, or a *BadValue* error
>>> results.]
>>>
>>> I have added a check to prevent zero width,height to be used for
>>> XCreatePixmap() and also throw OOME so to ask Java to throw IAE.
>>>
>>> Regards
>>> Prasanta
>>