[OpenJDK 2D-Dev] [9] Review request for 8160124 SunGraphics2D.hitClip() can give wrong result for floating point scale
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Tue Aug 2 17:36:05 UTC 2016
On 27.07.16 1:56, Jim Graham wrote:
> Rectangle2D rClip = (Rectangle2D) usrClip;
> int x0 = (int) Math.ceil(rClip.getMinX() - 0.5);
> int y0 = (int) Math.ceil(rClip.getMinY() - 0.5);
> int x1 = (int) Math.ceil(rClip.getMaxX() - 0.5);
> int x1 = (int) Math.ceil(rClip.getMaxY() - 0.5);
> clipRegion = devClip.getIntersectionXYXY(x0, y0, x1, y1);
>
> Some work and testing should be done to see what happens when any of the
> min/max/xy coordinates overflow an integer, but that would be the basic
> gist of what we should be doing in that case in validateCompClip()...
Should we really take care about negative minXY/maxXY here? I assume
that negative coordinates will be skipped anyway when we interact them
with devClip. So it seems we can simply cast double to int here?
> ...jim
>
> On 07/26/2016 02:50 PM, Sergey Bylokhov wrote:
>> On 13.07.16 5:42, Jim Graham wrote:
>>> What does the compClip end up being in that case?
>>
>> BufferedImage bi = new
>> BufferedImage(10,10,BufferedImage.TYPE_INT_ARGB);
>> SunGraphics2D graphics = (SunGraphics2D) bi.createGraphics();
>> graphics.scale(1.5, 1.5);
>> graphics.setClip(1, 1, 0, 0);
>> System.out.println("clip = " + graphics.getClip().getBounds2D());
>> System.out.println("clip.isEmpty = " +
>> graphics.getClip().getBounds2D().isEmpty());
>> boolean hit = graphics.hitClip(1, 1, 1, 1);
>> System.out.println("hit = " + hit);
>> System.out.println("compClip = " + graphics.getCompClip());
>> System.out.println("compClip.isEmpty = " +
>> graphics.getCompClip().isEmpty());
>>
>> The output:
>> clip = java.awt.geom.Rectangle2D$Double[x=1.0,y=1.0,w=0.0,h=0.0]
>> clip.isEmpty = true
>> hit = true
>> compClip = Region[[1, 1 => 2, 2]]
>> compClip.isEmpty = false
>>
>> So the graphics.getClip() returns empty clip, but compClip is not empty.
>>
>> It seems this occurs when we intersect usrClip
>> =[x=1.5,y=1.5,w=0.0,h=0.0] and devClip=Region[[0, 0 => 10, 10]] because
>> result is clipRegion=Region[[1, 1 => 2, 2]]
>>
>> The code in question is:
>> protected void validateCompClip() {
>> .....
>> clipRegion = devClip.getIntersection(usrClip.getBounds());
>> .....
>> }
>> The usrClip.getBounds() method will change [x=1.5,y=1.5,w=0.0,h=0.0] to
>> [x=1,y=1,w=1,h=1], so the empty Rectangle2D became non-empty.
>>
>>
>>>
>>> ...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.
>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
--
Best regards, Sergey.
More information about the 2d-dev
mailing list