[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
Fri Dec 21 00:42:43 UTC 2012 

Hi, Jim.
21.12.2012 3:54, Jim Graham wrote:
> Hi Sergey,
>
> The getClip methods don't claim to return the exact same shape that 
> the user handed in - it just has to cover the territory covered by the 
> clip.  x,y,-N,-M covers the same territory as x,y,0,0 so it would be 
> fine to substitute that value in its place.  It is arguable whether it 
> matters if we convert x,y,+N,-M to x,y,+N,0 or x,y,0,0 since both 
> rectangles cover no ground. Also, preserving the x,y is questionable 
> since any 0-dimensioned rectangle covers no ground and is equally 
> representative of the fact that the clip is empty.
Yes, they covers the same territory, but how we can check, that the 
these shapes equivalent? Because equals return false for them. Note that 
equals of the Area states about
> There isn't even any guarantee that we will hand them back a rectangle 
> from getClip().
One of the jck tests expect this, but I think that this is the wrong test.
> After all, if you end up in the drop through case then we hand the 
> paths to Area to do intersections and Area will do all sorts of 
> surgery on the geometry.
But I sure this area will be equal to the user's clip?
>   We could hand them back a hard-coded "I'm an empty shape" object 
> that isn't any class that they'd recognize and would return 0,0,0,0 
> for bounds and a singleton iterator that immediately claims 
> "isDone()==true".
>
> If it passes tests (meaning our clever optimizations in the 
> transform/untransform helper methods don't unravel it incorrectly) 
> then I think simply replacing any negative dimensions with zeros would 
> be the least surprising result.  I'm not sure what value there is in 
> attempting to return a rectangle that conveys the "same negative 
> dimensions"...
>
>             ...jim
>
> On 12/20/2012 3:35 PM, Sergey Bylokhov wrote:
>> 21.12.2012 3:26, Sergey Bylokhov пишет:
>>> Hi, Jim.
>>> 21.12.2012 1:42, Jim Graham wrote:
>>>> Hi Sergey,
>>>>
>>>> Avoiding the transform only works if they read it back in the same
>>>> coordinate system that they set it.  It will fail if they do:
>>>>
>>>>     setClip or clip(...);
>>>>     scale(5, 5);
>>>>     getClip();
>>>>
>>>> So, given that we have to deal with the transform/untransform issues
>>>> anyway then a user-space clip caching solution isn't going to solve
>>>> the immediate problem for all cases.
>>> Yes, right, second solution does not work in this case.
>>>>
>>>> I'd have to look closely at the code to know for sure, but I imagine
>>>> that it would be easy enough to ensure that the [un]transform methods
>>>> always turn a rectangle with 0 dimensions into another rectangle with
>>>> 0 dimensions (whether one or both are 0).
>>> Well, I expect that solution from the webrev does this, isn't it?
>>> Approach is simple if we have correct clip from the user, we always
>>> get correct clip after transformation, and vice versa.
>>>> The setFFD normalization only hurts us when we have negative
>>>> dimensions, but it should be OK with 0 dimensions, no?  So, if we
>>>> normalize negative dimensions to 0 going in then it might be safe?
>>> Do you mean just change Rectangle(0,0,-100,-100) to (100,100,0,0)?
>> I just realise that (100,100,0,0) is incorrect here. So the question
>> more general: Do you mean just change Rectangle(0,0,-100,-100) to ..
>> "what"?
>>> How we can revert back this operation?
>>>
>>>>
>>>>             ...jim
>>>>
>>>> On 12/20/2012 9:52 AM, Sergey Bylokhov wrote:
>>>>> 20.12.2012 14:12, Sergey Bylokhov пишет:
>>>>>> 20.12.2012 13:49, Jim Graham wrote:
>>>>>>> More to the point, if you substitute a 0x0 clip when an incoming 
>>>>>>> clip
>>>>>>> is an empty rectangle then it will always be empty under any 
>>>>>>> kind of
>>>>>>> transform.  This could be done by performing a "max(w,0);max(h,0);"
>>>>>>> operation on the incoming data.  Once a clip is accepted as
>>>>>>> non-empty, then I think the current code will adequately deal with
>>>>>>> flipped transforms, won't it?
>>>>>> Yes, but with one exception. What to do with 
>>>>>> getClip:untransformShape
>>>>>> methods in this case, it executes the reverse transformation.
>>>>>> The question is: if the incoming clip is an empty how to store 
>>>>>> it? If
>>>>>> we did not transform incoming clip, we should change
>>>>>> getClip/clip/untransformShape + some flag which should indicate that
>>>>>> incoming clip(not a result of transformation) was empty.
>>>>> Another solution: don't use transformShape/untransformShape in the
>>>>> set/get methods. store usrClip as is, and transform 
>>>>> usrClip->usrClipTmp
>>>>> in validateCompClip().
>>>>> In this case setters/getters will become much simpler.
>>>>>>>
>>>>>>>             ...jim
>>>>>>>
>>>>>>> On 12/19/2012 8:47 PM, Jim Graham wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> This is all a lot more complicated than I think it needs to be.
>>>>>>>> Why not
>>>>>>>> just detect incoming empty rectangles and force an empty
>>>>>>>> composite clip
>>>>>>>> in that case?  No heuristics necessary...
>>>>>>>>
>>>>>>>>              ...jim
>>>>>>>>
>>>>>>>> On 12/15/2012 9:26 AM, Sergey Bylokhov wrote:
>>>>>>>>> Hi, Jim.
>>>>>>>>> Could you please review the updated version of the fix:
>>>>>>>>> http://cr.openjdk.java.net/~serb/8004859/webrev.01
>>>>>>>>> 14.12.2012 0:40, Jim Graham wrote:
>>>>>>>>>> This fix breaks other behavior.
>>>>>>>>>>
>>>>>>>>>> You need to use setFrameFromDiagonal on the results of the
>>>>>>>>>> transform
>>>>>>>>>> because a flip or rotation can cause the transformed points 
>>>>>>>>>> to be
>>>>>>>>>> unordered and setFFD will sort them as they need to be. It is 
>>>>>>>>>> the
>>>>>>>>>> incoming rectangle that needs to be checked for being empty,
>>>>>>>>>> not the
>>>>>>>>>> results of the transform.
>>>>>>>>> In the new version I restore coordinates relation, if it were
>>>>>>>>> changed
>>>>>>>>> during transform.
>>>>>>>>> Swap needed:
>>>>>>>>>   - If the user provide incorrect rectangle and after
>>>>>>>>> transformation we
>>>>>>>>> get correct rectangle.
>>>>>>>>>   - If the user provide correct rectangle and after
>>>>>>>>> transformation we
>>>>>>>>> get incorrect rectangle.
>>>>>>>>>>
>>>>>>>>>> The case that will fail with this fix is setting the clip to a
>>>>>>>>>> valid
>>>>>>>>>> rectangle in a coordinate system that is rotated by a multiple
>>>>>>>>>> of 90
>>>>>>>>>> degrees or is flipped horizontally or vertically. Those cases 
>>>>>>>>>> will
>>>>>>>>>> result in an empty clip, but the clip was not empty coming in...
>>>>>>>>>>
>>>>>>>>>>             ...jim
>>>>>>>>>>
>>>>>>>>>> On 12/13/2012 3:08 AM, Sergey Bylokhov wrote:
>>>>>>>>>>> Hello,
>>>>>>>>>>> Please review the fix for jdk 8.
>>>>>>>>>>> Change description:
>>>>>>>>>>> 1 transformShape now symmetric to untransformShape()
>>>>>>>>>>> (setFrameFromDiagonal was removed).
>>>>>>>>>>> 2 getClipBounds now always uses getBounds2D which does not 
>>>>>>>>>>> return
>>>>>>>>>>> empty
>>>>>>>>>>> Rectangle if the userclip has negative width or height.
>>>>>>>>>>>
>>>>>>>>>>> Note that if the userclip has negative width or height, our 
>>>>>>>>>>> real
>>>>>>>>>>> graphic
>>>>>>>>>>> clip will be empty/no-area. This wasn't true before the fix
>>>>>>>>>>> for the
>>>>>>>>>>> scaled graphics.
>>>>>>>>>>>
>>>>>>>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8004859
>>>>>>>>>>> Webrev can be found at:
>>>>>>>>>>> http://cr.openjdk.java.net/~serb/8004859/webrev.00
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>
>>>
>>
>>


-- 
Best regards, Sergey.

More information about the 2d-dev mailing list