[OpenJDK 2D-Dev] [9] Review Request: 8167310 The graphics clip is incorrectly rounded for some fractional scales

[Jim Graham]

One last thing.  In the clipRound() method you mention that NaN evaluates to 0.  I assume that is just an observation 
that we'd end up in the "(int) ceil()" case and the cast returns a 0.

In the case of a path we omit NaN coordinates, which means skipping segments in a path until they become sane again. 
That would likely turn a rectangle into a triangle if it happened in a shape, but in a Rectangle object the NaN ends up 
affecting more than one corner (minX is the coordinate of 2 corners, maxX is the coordinate for 2 corners, same for Y) 
and it degenerates further into an empty shape in all cases.  For example:

(x, y, w, h)
10, 10, NaN, 20
path is 10,10 => NaN,10 => NaN,30 => 10,30
     => 2 points eliminated
     => devolves to a vertical line
     => no pixels included

10, 10, 20, NaN
path is 10,10 => 30,10 => 30,NaN => 10,NaN
     => 2 points, devolves

NaN, 10, 20, 20
path is NaN,10 => NaN,10 => NaN,30 => NaN,30
     => no points left
     => completely degenerate

10, NaN, 20, 20
path is 10,NaN => 30,NaN => 30,NaN => 10,NaN
     => no points, degenerate

In the case of NaN in X and Y, the clipRound() method would end up returning 0 for both min and max of X or Y which 
would hopefully produce an empty region when we do the intersection.

In the case of NaN as a W or H, clipRound() would only produce a 0 for maxX or maxY, but the min would still be computed 
correctly.  I guess this could technically be considered an empty shape in practice since we tend to use Regions to only 
encompass pixels in a non-negative space, so it is fine in practice, but if we ever use translated Regions for some sort 
of use (what about shaped components which might use Region?) then 0 for maxX or maxY might be paired with a negative 
value for minX or minY and end up with a non-empty Region in the negative coordinates (and potentially translated later 
to positive coordinates and encompass some pixels on a destination drawable).

All of those would (should?) return a completely empty region if we rasterized the shape, so we should set an empty clip 
if we get a NaN in any coordinate...

			...jim

On 10/10/16 4:37 PM, Sergey Bylokhov wrote:
> On 10.10.16 23:42, Jim Graham wrote:
>> Can we also not use MAX_INT for the drawImage test case?  Either have
>> the drawImage follow the clip around or choose an appropriate non-limit
>> size to cover the worst case scale with some margin for error...
>
> Something like this?
> http://cr.openjdk.java.net/~serb/8167310/webrev.04
>
>
>> On 10/10/16 12:45 PM, Sergey Bylokhov wrote:
>>> An updated version:
>>> http://cr.openjdk.java.net/~serb/8167310/webrev.03
>>>  - STROKE_PURE is used in the test, the line width is set to 2.01.
>>> This also fixed the difference between clips(Shape vs
>>> Rectangle).
>>>  - The if statement is changed as suggested.
>>>
>>> The additional questions:
>>>  - In the current fix we change behavior of the clip. Before the fix
>>> if we set the clip to the nearest areas they can
>>> overlaps in the destination. Should we change the drawImage as well?
>>> Currently if I draw image to the nearest areas in
>>> the user space, the images can overlap in the destination(so the
>>> actual result in destination depends on what image was
>>> painted first).
>>>  - Should the clip be affected by the stroke(if it was set by the
>>> shape)? Right now if the clip was set by the shape it
>>> will produce different result than if it was set via rectangle.
>>>
>>> On 10.10.16 22:29, Jim Graham wrote:
>>>> That does sound like a problem.  Does it do the same thing with new
>>>> Path2D(Rectangle)?  The Area class does some processing on the path and
>>>> it would be nice to eliminate that as a potential source of this
>>>> problem.  I don't have a buildable JDK9 repo right now that I can fire
>>>> off some quick tests on so I'll have to look at this later...
>>>>
>>>>             ...jim
>>>>
>>>> On 10/10/16 12:04 PM, Sergey Bylokhov wrote:
>>>>> On 10.10.16 21:55, Sergey Bylokhov wrote:
>>>>>>> Will give us a pass on the test as long as we made the same
>>>>>>> mistake for
>>>>>>> both the rect clip and the shape clip.  I think you want "(rgb !=
>>>>>>> goldRGB) || (rgb != GREEN && rgb != RED)"...?
>>>>>>
>>>>>> Correct, I will update the test.
>>>>>
>>>>> surprisingly but it is produce the different results.... And I think
>>>>> that the clip which is set via Shape is shifted,
>>>>> because the first and last fillRects cover only the half of expected
>>>>> area. But in case of clip=rectangle all fillRects
>>>>> produce the same areas.
>>>>>
>>>>>
>>>
>>>
>
>