[OpenJDK 2D-Dev] Request for review: Additional fix for JDK-7159455

[Jim Graham]

Hi Clemens,

Now that we've narrowed it down to a quadrant rotation at best, can't we 
get by with only transforming x,y and x+w,y+h?  Or is the adjust method 
used from other locations that might have a non-quadrant transform?  (Or 
do any callers depend on srcCoords and dstCoords being fully 
populated?).  If it's a mixture of quadrant and general transforms, then 
I think it may make sense to have the adjust method do the test, 
determine how much it needs to transform, and then return the result, 
but that is probably a minor optimization compared to other work going 
on (not sure if benchmarks would notice the difference).

I also noticed that the bounds are rounded.  If the intent is to catch 
every single pixel, then shouldn't those be floor/ceil operations?  If 
the intent is to "rasterize" the transformed rectangle then filled 
rectangles (and shapes) use a "round down" calculation, but Math.round() 
is a "round up" calculation.  How is the resulting compositeBounds used?

Also, I noticed that you added a new wild-card import into one of the 
files.  I believe we deprecated that practice in the FX source base, but 
I don't know what the standard practice is in OpenJDK and didn't see any 
statements about it in the OpenJDK style guildelines.

				...jim

On 10/16/13 10:55 AM, Clemens Eisserer wrote:
> Hi,
>
> As Jim pointed out, my earlier fix for bug JDK-7159455 wasn't entirely
> correct as it allowed some shear-transformations to pass a check for a
> fast-path although they should have been rejected. He proposed a way
> of using the capabilities already provided by AffineTransform, which
> is correct, shorter and most likely more efficient (thanks!).
>
> Please find the follow-up fix for  JDK-7159455 at:
> http://cr.openjdk.java.net/~ceisserer/fix12/webrev.00/
>
> Thanks, Clemens
>