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

[prasanta sadhukhan]

ok. Thanks Jim .
Please review the modified webrev
http://cr.openjdk.java.net/~psadhukhan/8140530/webrev.02/

Regards
Prasanta
On 12/4/2015 6:45 AM, Jim Graham wrote:
> 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
>>>>
>>