[OpenJDK 2D-Dev]  Request for review: 8004859 Graphics.getClipBounds/getClip return difference nonequivalent bounds, depending from transform.
james.graham at oracle.com
Mon Apr 22 21:11:14 UTC 2013
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...
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.
> 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?
More information about the 2d-dev