Beef up GrContext::AutoMatrix to handle doing GrPaint matrix adjustments.

Beef up GrContext::AutoMatrix to handle doing GrPaint matrix adjustments.

R=robertphillips@google.com
Committed: https://code.google.com/p/skia/source/detail?r=5918

[robertphillips, 2012-10-11 21:43:44]

Several questions

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:727: */
missing & on matrix parameter!

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:733: this->preConcat(newMatrix, paint);
pull out both return trues and place at end?

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:735: } else {
I believe this path needs a restore call

https://codereview.appspot.com/6656047/diff/2001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.appspot.com/6656047/diff/2001/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:875: if (!isNormalBlur) {
Why is this call needed?

https://codereview.appspot.com/6656047/diff/2001/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1506: // drawSprite is defined to be in device coords
regardless of SkDraw's matrix.
why isn't this still an AutoMatrix?

[bsalomon, 2012-10-12 13:59:32]

This is the third time I typed this change. The first time I didn't finish, the
second time I reverted it without properly stashing first. I got very sloppy on
round three!

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:727: */
On 2012/10/11 21:43:44, robertphillips wrote:
> missing & on matrix parameter!

D'oh.

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:733: this->preConcat(newMatrix, paint);
On 2012/10/11 21:43:44, robertphillips wrote:
> pull out both return trues and place at end?

Done.

https://codereview.appspot.com/6656047/diff/2001/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:735: } else {
On 2012/10/11 21:43:44, robertphillips wrote:
> I believe this path needs a restore call
nice catch!

https://codereview.appspot.com/6656047/diff/2001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.appspot.com/6656047/diff/2001/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:875: if (!isNormalBlur) {
On 2012/10/11 21:43:44, robertphillips wrote:
> Why is this call needed?

I forgot to remove the one above!

https://codereview.appspot.com/6656047/diff/2001/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1506: // drawSprite is defined to be in device coords
regardless of SkDraw's matrix.
On 2012/10/11 21:43:44, robertphillips wrote:
> why isn't this still an AutoMatrix?

We set the GrContext's matrix on every draw now. So we don't really care what it
is when this function exits. There are probably other places where we don't need
to restore but they weren't as obvious as this and drawDevice.

I modified the change to add a bool to prepareDraw that overrides the SkDraw's
matrix with the identity matrix. (It might be nice if SkCanvas just did this for
us.)

[robertphillips, 2012-10-12 14:18:49]

LGTM + 2 requests for comments

https://codereview.appspot.com/6656047/diff/10001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.appspot.com/6656047/diff/10001/src/gpu/SkGpuDevice.cpp#new...
src/gpu/SkGpuDevice.cpp:1504: int left, int top, const SkPaint& paint) {
I think we should still have a comment here

https://codereview.appspot.com/6656047/diff/10001/src/gpu/SkGpuDevice.cpp#new...
src/gpu/SkGpuDevice.cpp:1589: 
Here too

[bsalomon, 2012-10-12 14:52:04]

https://codereview.appspot.com/6656047/diff/10001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.appspot.com/6656047/diff/10001/src/gpu/SkGpuDevice.cpp#new...
src/gpu/SkGpuDevice.cpp:1504: int left, int top, const SkPaint& paint) {
On 2012/10/12 14:18:49, robertphillips wrote:
> I think we should still have a comment here

Done.

https://codereview.appspot.com/6656047/diff/10001/src/gpu/SkGpuDevice.cpp#new...
src/gpu/SkGpuDevice.cpp:1589: 
On 2012/10/12 14:18:49, robertphillips wrote:
> Here too

Done.