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

[Jim Graham]

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();

The answer from getClip in that case should be scaled down by 5x.  Going 
through device space is the most straightforward way to manage any 
potential differences in transform, but more direct results are possible 
if you want to get into (storing and) comparing the before and after 
results.  The drawbacks are the storage for the original transform and 
complicated code to compute the difference in the transforms.  I suppose 
you could keep it as an optimization and simply disable the optimization 
if the transform changes at all, but that means you then still have to 
have the backup plan.  Also, it becomes pretty much unmanageable if they 
combine a few different shapes in a few different coordinate systems 
(using clip, not setClip) - at that point you pretty much must go 
through the common denominator of device space anyway so just doing that 
in all cases is the path of least resistance by a wide margin.

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.

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).  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?

			...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
>>>>>>>
>>>>>
>>>>>
>>
>>
>
>