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

[Jim Graham]

I think it makes sense to catch it at a higher level, but also to throw 
some type of exception from the X11 code as you do now because 
regardless of our higher level policy, the X11 implementation function 
can never succeed there...

So, my preference would be to keep the existing pieces of this fix that 
you already have and to add an explicit check and throw IAE to 
SunVolatileImage for completeness and guaranteed consistency...

			...jim

On 12/2/15 11:18 PM, prasanta sadhukhan wrote:
> 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
>>>
>