[OpenJDK 2D-Dev] Please review patch for 7105461

[Clemens Eisserer]

Hi Jim,

A new version of the patch is available at:
http://cr.openjdk.java.net/~ceisserer/7105461/webrev.05/

> If x or y are > MAX_SHORT then you should probably just reject the operation
> immediately, otherwise you end up with a bunch of math that should end up
> doing a NOP, but it is tenuous if it actually succeeds in doing so.

Done. Thanks for pointing that out.

> Also, if x,y are < MIN_SHORT then when you clamp them to MIN_SHORT they may
> be > x2,y2 and so when you clamp the dimensions you will end up clamping a
> massively negative number to Ushort.  I don't have the code off hand to see
> how the method deals with that, but it is probably not 100% kosher in this
> case (not sure if the subtraction might end up looking like a huge positive
> number).  Thus, it is probably better to reject the op if x2,y2 are <
> MIN_SHORT as well rather than rely on the processing.
> Also, rejecting the call here rather than relying on the outcome of the math
> avoids adding NOP requests to the X protocol stream...

This would have been handled by the width/height-check later (tested),
however as you pointed out it's better to exit early.
Would you prefer combining the MIN/MAX checks after calculating x2/y2
to improve redability?

However, I found another problem: clampToUShort returned a short, so
when a dimension > Short.MAX_VALUE was clamped, the check >=0 later
saw some negative value and exited for a valid request. As for now
fillRect is the only consumer, I took the liberty to change its return
type to int.

Thanks a lot for the detailed in-depth review (and you patience).

- Clemens