Yes, I was wondering about that. I think I accidentally left that in there from the original patch and didn't comment/remove it.
-- Scott On Sep 6, 2012, at 5:04 PM, Mike Swingler <[email protected]> wrote: > Isn't this line a no-op, since you concat'ing the identity transform? > > + CGContextConcatCTM(qsdo->cgRef, CGAffineTransformMake(1, 0, 0, 1, 0, 0)); > > Otherwise, looks fine. > > Cheers, > Mike Swingler > Apple Inc. > > On Sep 4, 2012, at 8:53 PM, Scott Kovatch <[email protected]> wrote: > >> I need one more reviewer/+1... anyone? >> >> -- Scott K. >> >> Begin forwarded message: >> >>> From: Scott Kovatch <[email protected]> >>> Subject: Re: Review: Remove private API from graphics code >>> Date: September 4, 2012 8:16:03 AM PDT >>> To: Phil Race <[email protected]> >>> Cc: "[email protected]" <[email protected]>, 2d-dev >>> <[email protected]>, "[email protected] OS X" >>> <[email protected]> >>> >>> I posted a new webrev at >>> http://cr.openjdk.java.net/~skovatch/7187834/webrev.01/ >>> >>> In ImageSurfaceData, it looks like we are trying to get back to a pure CTM >>> (i.e., no transform of any kind applied.) before we draw the image. We have >>> already saved the state, but saving the state doesn't reset it. That's why >>> there are extra gyrations to invert and concat the inversion. In >>> QuartzRenderer.m the information you found applies, so I'm just using that >>> now. >>> >>> These changes fix the printing bugs in Java2Demo that I saw with the first >>> webrev. >>> >>> -- Scott >>> >>> >>> On Aug 31, 2012, at 11:50 AM, Phil Race <[email protected]> wrote: >>> >>>> The recommendation is to restore the graphics state rather than inverting >>>> :- >>>> >>>> https://developer.apple.com/library/mac/#documentation/graphicsimaging/conceptual/drawingwithquartz2d/dq_affine/dq_affine.html >>>> >>>> "Quartz also provides an affine transform function that inverts a matrix, >>>> |CGAffineTransformInvert|. Inversion is generally used to provide reverse >>>> transformation of points within transformed objects. Inversion can be >>>> useful when you need to recover a value that has been transformed by a >>>> matrix: Invert the matrix, and multiply the value by the inverted matrix, >>>> and the result is the original value. You usually don’t need to invert >>>> transforms because you can reverse the effects of transforming the CTM by >>>> saving and restoring the graphics state." >>>> >>>> -phil. >>>> >>>> On 8/31/2012 11:16 AM, Phil Race wrote: >>>>> Scott, >>>>> >>>>> These files were added by Bino to support printing. Quartz isn't used >>>>> except for printing in JDK 7, so as I understand it, testing on-screen in >>>>> Java2Demo should not exercise this code. I'm surprised that you saw >>>>> it being exercised. Did you do any printing testing ? >>>>> >>>>> The matrix inversion seems unlikely to be applied to any non-invertible >>>>> matrices, so that's fine, but I wonder if you have lost precision here >>>>> due to floating point inaccuracies ? >>>>> >>>>> If you originally had a simple scale or identity, rotated it, and then >>>>> applied the inverse to unrotate it, do you really end up with exactly >>>>> the same results. The more you do this the more inaccuracies creep in, >>>>> which may be part of the reason for the original approach. >>>>> I find it a little hard to believe that there isn't a direct public way to >>>>> restore a transform. >>>>> >>>>> The changes for mountain lion are safe for snow leopard I presume? >>>>> I believe the builds still happen on snow leopard. >>>>> >>>>> Also this should have been sent to 2d-dev, not awt-dev. >>>>> These files, APIs, and printing are all 2D, not awt. >>>>> >>>>> -phil. >>>>> >>>>> I am not sure why they are not used but it >>>>> On 8/31/2012 10:45 AM, Scott Kovatch wrote: >>>>>> http://cr.openjdk.java.net/~skovatch/7187834/webrev.00/ >>>>>> >>>>>> This is based on the patch submitted by Marco Dinacci. I had to modify >>>>>> it a bit to get it to compile against 7u-dev, but nothing major. >>>>>> >>>>>> The changes in ImageSurfaceData.h were needed as a result of my use of >>>>>> Xcode 4.4.1 on Mountain Lion. >>>>>> >>>>>> I ran the Java2Demo and don't see any problems. I paid close attention >>>>>> to the Images tab, since it looks like this code is heavily used by it. >>>>>> >>>>>> -- Scott K. >>>>>> >>>>>> ---------------------------------------- >>>>>> Scott Kovatch >>>>>> [email protected] >>>>>> Santa Clara/Pleasanton, CA >>>>>> >>>>>> >>>>> >>>> >>> >> >
