[OpenJDK 2D-Dev] [8] Request for review: 8004859 Graphics.getClipBounds/getClip return difference nonequivalent bounds, depending from transform.

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Wed Jun 26 12:07:01 UTC 2013


Hello.
No volunteers to review the fix?
Thanks.

On 10.06.2013 16:49, Sergey Bylokhov wrote:
> Hello.
> Additional note is that this fix is targeted to 7u40.
>
> On 06.06.2013 22:38, Sergey Bylokhov wrote:
>> HI, Jim.
>> Can you review the updated version of the fix:
>> http://cr.openjdk.java.net/~serb/8004859/webrev.03
>> I decided to implement an option, where transformed graphics 
>> (identity, translated, scaled) returns equivalent clip. For this in 
>> the transformShape() we preserve orientation of Rectangle, and in the 
>> getClipBounds() we do not lose this information because we use 
>> getBounds2D() instead of getBounds(). Note that I have simplified 
>> getClipBounds() from the previous version:
>> http://cr.openjdk.java.net/~serb/8004859/webrev.02/src/share/classes/sun/java2d/SunGraphics2D.java.sdiff.html 
>>
>> Also there is no additional check for empty rectangles.
>>> The original code in getClipBounds would end up returning a "new
>>> Rectangle()" if the clip was an empty rectangle due to the way that
>>> "Rectangle2D/Path2D.getBounds()" works.  You now use
>>> setFrame(getBounds2D()) which will attempt to preserve the dimensions
>>> of empty clips.  So, the "preserve the size of an empty clip" support
>>> is new. 
>> Yes it is new in case of Rectangle2D, but now it works in the same 
>> way as for Rectangle.
>>
>> On 23.04.2013 1:11, Jim Graham wrote:
>>> I think if empty (un)transforms to something else that is also 
>>> classified as empty naturally without having to special case it or 
>>> test it then returning the "transformed empty thing" is a fine 
>>> return value.
>>>
>>> But, if it comes down to "Eeek, it was empty so I need to calculate 
>>> a specifically similar empty thing to return" then I feel that the 
>>> test should just result in "return new Rectangle()".  No use pulling 
>>> hair out to make a "more useful non-answer" unless it falls out of 
>>> the calculations for free - as in "unless it avoids even having to 
>>> check for the exceptional condition in the first place".  In the 
>>> cases I reviewed I seem to recall that we were doing the tests 
>>> anyway (or we were doing equally complex tests in order to normalize 
>>> the answer even if we never explicitly tested for "isEmpty()") so it 
>>> would make the code much less problematic to just weed the empty 
>>> answers out early.
>>>
>>> As far as "are any Developers expecting a usefully non-degenerate 
>>> empty answer" then I don't think so...
>>>
>>>             ...jim
>>>
>>> On 4/18/13 6:24 AM, Sergey Bylokhov wrote:
>>>> Hello Jim.
>>>>
>>>> On 1/17/13 4:56 AM, Jim Graham wrote:
>>>>> The original code in getClipBounds would end up returning a "new
>>>>> Rectangle()" if the clip was an empty rectangle due to the way that
>>>>> "Rectangle2D/Path2D.getBounds()" works.  You now use
>>>>> setFrame(getBounds2D()) which will attempt to preserve the dimensions
>>>>> of empty clips.  So, the "preserve the size of an empty clip" support
>>>>> is new.
>>>>>
>>>>> I then mentioned that we don't need to go out of our way to preserve
>>>>> the dimensions of an empty clip, but you seem to be saying that we
>>>>> don't want to change this behavior - but your new code represents the
>>>>> break with existing behaviors, no?
>>>> I returned to this problem and studied it a little more deeply.
>>>> Description.
>>>> Assume we set clip=Rectangle[100, 100,-100,-100] for sg2d with a
>>>> different transform.
>>>>   1 identity/translate (default mode for non-retina): getClip () will
>>>> return Rectangle[100, 100,-100,-100].
>>>>   2 scale (default mode for retina): getClip() will return
>>>> Rectangle[0,0,100,100] <- bug and it should be fixed.
>>>>   3 generic: getClip will return Rectangle[0,0,0,0], because we 
>>>> convert
>>>> Rectangle to the Shape through RectIterator, which ignores the 
>>>> negative
>>>> width and height. Note that x and y were not preserved only if w and
>>>> h<0, but if w = h =0, then x and y will be preserved.
>>>>
>>>> In an original fix I made case 2 like case 1, so from the point of 
>>>> view
>>>> of users there was no difference in case of retina display.
>>>> But now I have doubts and I think that all cases shall work equally 
>>>> like
>>>> in case 3.
>>>> What do you think?
>>>>
>>>>>
>>>>>             ...jim
>>>>
>>>>
>>>
>>
>>
>
>


-- 
Best regards, Sergey.




More information about the 2d-dev mailing list