[OpenJDK 2D-Dev] [9] Review request for 8160124 SunGraphics2D.hitClip() can give wrong result for floating point scale

[Jim Graham]

What does the compClip end up being in that case?

			...jim

On 7/4/16 1:12 AM, Sergey Bylokhov wrote:
> On 01.07.16 2:49, Jim Graham wrote:
>> How is it returning true?  If the clip really is empty, then
>> intersectsQuickCheck() should never return true.  Or are you saying that
>> an empty clip shape produces a non-empty composite clip region?
>
>
> This code will test such situation:
>         BufferedImage bi = new BufferedImage(10,10,BufferedImage.TYPE_INT_ARGB);
>         Graphics2D graphics = bi.createGraphics();
>         graphics.scale(1.5, 1.5);
>         graphics.setClip(1, 1, 0, 0);
>         System.out.println("empty = " + graphics.getClip().getBounds2D().isEmpty());
>         boolean hit = graphics.hitClip(1, 1, 1, 1);
>         System.out.println("hit = " + hit);
>
> if "graphics.scale(1.5, 1.5);" will be removed then correct false will be printed. In both cases the clip will be empty
> but hitCLip will return different result in case of scaled SG2D.
>
>>
>>             ...jim
>>
>> On 06/30/2016 10:02 AM, Sergey Bylokhov wrote:
>>> It looks strange that the empty clip became "non-empty"(at least hitClip
>>> reports this) when we apply transform to it, no? I guess that at the
>>> beginning of hitClip() we should check that the clip.isEmpty(), and we
>>> should return "false" in this case(I think this is not strictly related
>>> to this bug)?
>>>
>>> On 24.06.16 1:14, Jim Graham wrote:
>>>> Think of this method as asking:
>>>>
>>>> I don't want you to waste a lot of time, but tell me if it is silly for
>>>> me to even consider rendering something with these bounds.
>>>>
>>>> And the answer is either "Oh, yeah, it is inconceivable that those
>>>> bounds would be rendered", or "Not sure, maybe, just render it and
>>>> see".  There may be some cases where it knows "I know for sure that lots
>>>> of stuff will render through the clip", but that is not where the
>>>> divining line is here in terms of when the answer becomes true - it
>>>> becomes true when there is a chance that it might render something.
>>>>
>>>> Arguably, the user-space comparison against the user-space clip that you
>>>> added here can never be accurate even if you allow for "false
>>>> positives".  The clip is rasterized and whole pixels are chosen based on
>>>> various criteria that affect clip rasterization.  Thus, while the
>>>> theoretical clip is at N.5, our rasterization choice has us render
>>>> anything that hits pixel N, even if the contribution of the rendered
>>>> primitive is only for the first half of N.  That pixel might be rendered
>>>> if anything hits any part of it, depending on what rendering operation
>>>> is being done.
>>>>
>>>> So, your "fix" actually breaks the functionality because it is quite
>>>> possible that something with a bounding box that stops before N.5 might
>>>> affect pixel N and cause it to be rendered even though your new answer
>>>> suggested that it wouldn't happen.  Your code might actually cause a
>>>> bug, not fix one.
>>>>
>>>> (There are also some potential theoretical failures related to how AA
>>>> and STROKE_CONTROL might perturb our rendering, effects which are ignore
>>>> by the current implementation of hitClip(), however I believe that all
>>>> of those effects fit within the current implementation - it's just that
>>>> I don't think anyone has ever proven this, or written an exhaustive test
>>>> suite to verify that none of our rendering hints can perturb rendering
>>>> by enough to create some surprises here...)
>>>>
>>>>             ...jim
>>>>
>>>> On 6/23/16 3:00 PM, Jim Graham wrote:
>>>>> Since "return true" would be a compliant implementation of
>>>>> Graphics.hitClip(), this is not a bug...
>>>>>
>>>>> Read the documentation, it is allowed to use fast math that can return
>>>>> true when technically the answer is false...
>>>>>
>>>>>             ...jim
>>>>>
>>>>> On 6/23/16 5:04 AM, Alexandr Scherbatiy wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> Could you review the fix:
>>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8160124
>>>>>>   webrev: http://cr.openjdk.java.net/~alexsch/8160124/webrev.00
>>>>>>
>>>>>>   Let's set the clip [x=5, y=5, width=5, height=5] to a graphics and
>>>>>> call the hitClip() with the passed rectangle [x=0,
>>>>>> y=0, width=5, height=5].
>>>>>>
>>>>>>   The result is false for the graphics with scale 1 and true if the
>>>>>> scale is floating point 1.5.
>>>>>>
>>>>>>   This is because the transformed clip which has floating point
>>>>>> bounds [7.5, 7.5, 7.5, 7.5] for the scale 1.5 has bounds
>>>>>> with rounded down upper-left  and rounded up lower-right corners [7,
>>>>>> 7, 8, 8] which now intersects with the transformed
>>>>>> rectangle [0, 0, 7.5, 7.5].
>>>>>>
>>>>>>   The proposed fix adds additional check for the user clip and the
>>>>>> user rectangle intersection if the intersection with
>>>>>> the region clip passes.
>>>>>>
>>>>>>  Thanks,
>>>>>>  Alexandr.
>>>>>>
>>>>>>
>>>
>>>
>
>