[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