[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] Review request for 8160124 SunGraphics2D.hitClip() can give wrong result fo
From: Jim Graham <james.graham () oracle ! com>
Date: 2016-07-26 22:56:26
Message-ID: 3dca1571-a32d-3d35-5b44-51a42f1c80b5 () oracle ! com
[Download RAW message or body]
Ah, this is an issue of what an empty rectangle means.
Filling 1.5,1.5,0,0 produces no output because it has no interior.
But, stroking 1.5,1.5,0,0 can produce output because it is simply a
rectangle that spins in place.
As a result there are really 3 different containment properties for a
rectangle - one that has area, one that has only location/no area, and
one that has neither location nor area. This is discussed in the class
comments for java.awt.Rectangle. isEmpty() indicates whether any area
is covered, but the Rectangle may still accumulate into bounds
calculations (via union operations) and other similar operations will
treat its location as valid even if it encompasses no area.
The issue is what should Rectangle2D.getBounds() return? We decided to
do a min/max on the origin coordinate if the width/height are 0 because
of this "it exists even if it covers no area" issue.
But, clip areas are governed by what pixels would be filled, which is
not reflected in the way that getBounds() works.
What is going wrong in SG2D is that it is using Rectangle2D.getBounds()
to compute the containment of the clip, but that does a min/max on the
area rather than compute which pixels would be hit by a fill operation.
We should replace that calculation with a more accurate computation on
which centers of pixels are included in the Rectangle2D, something like:
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()...
...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.
>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic