[OpenJDK 2D-Dev] Please review patch for 7105461
Jim Graham
james.graham at oracle.com
Fri Sep 21 20:40:21 UTC 2012
Hi Clemens,
Is the clip guaranteed to be rectangular here? Or is the clipping code
only meant to optimize the region of interest being processed?
In either case, I think the intersectsQuickCheckXYXY method might work
better since it does the entire test in one go and it if this code has
to deal with complex clipping then I think it would be a better choice
of test in the first place (it won't be fooled by "the value is inside
the bounds of the clip, but misses out on the specific sub-rects that
are contained in it" and it will be faster than scanning a bunch of
sub-rects only to approve the application of a basic bounds-intersection
operation).
(A similar caveat may apply to drawLine unless we can guarantee that the
comp clip is rectangular...)
Finally, why is the awtLock() moved inside the try/finally? If it fails
then you probably don't want to do an Unlock() do you? Looking through
the rest of the file, half of the methods call it inside the try-block
and half call it outside - we should be consistent there (and according
to the doc comments in SunToolkit, outside is probably the right choice)...
...jim
On 9/21/2012 10:04 AM, Clemens Eisserer wrote:
> Hi Jim,
>
> Sorry it took so long - could you please have a look at
> http://cr.openjdk.java.net/~ceisserer/7105461/webrev.02
> It does now protect against integer overflow, using the Region.clipAdd.
>
> Thanks, Clemens
>
> 2012/4/17 Jim Graham<james.graham at oracle.com>:
>> This code doesn't protect against integer overflow. We don't protect
>> against it in many places, but it couldn't hurt to get in the habit. I
>> think the Region code has some methods that do safe addition of integers
>> with simple limit clipping that would work fine for rectangle dimensions.
>>
>> (JDK8 will be introducing new methods in Math for unsigned results and exact
>> non-overflowing integer results as well, but that would complicate any
>> backports to JDK7...)
>>
>> ...jim
>>
>>
>> On 4/13/12 9:46 AM, Clemens Eisserer wrote:
>>>
>>> Hi,
>>>
>>> Please take a look at the patch for bug 7105461, located at
>>> http://cr.openjdk.java.net/~ceisserer/7105461/webrev.00/
>>>
>>> The problem was caused by Swing calling drawLine/fillRect with
>>> coordinates outside the valid X11 coordinate space.
>>> I took the same approach of the original X11 pipeline to simply clamp
>>> the corrdinates to the min/max allowed value although its not enterly
>>> correct - as it doesn't adjust width/height in case it clamps x/y -
>>> triggered for exmaple by the following call:
>>> g.fillRect(-32868, 0, 32968, 10);
>>>
>>> Should I take care of this special case, or is it ok to handle it the
>>> same way the X11 pipeline does?
>>>
>>> Thanks, Clemens
More information about the 2d-dev
mailing list