[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 

(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)...


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