Remove SkCanvas::drawBitmapMatrix()

Remove SkCanvas::drawBitmapMatrix()

R=mtklein@google.com, reed@google.com, robertphillips@google.com

Committed: https://skia.googlesource.com/skia/+/c54d8db4d169ea5f2af2a9a2349df007bd428475

[f(malita), 2014-12-09 23:50:08]

fmalita@chromium.org changed reviewers:
+ mtklein@google.com

[f(malita), 2014-12-09 23:50:08]

Chrome/Blink are drawBitmapMatrix-free, and hopefully so is Android.

https://codereview.chromium.org/789033002/diff/1/gm/bitmapmatrix.cpp
File gm/bitmapmatrix.cpp (right):

https://codereview.chromium.org/789033002/diff/1/gm/bitmapmatrix.cpp#newcode32
gm/bitmapmatrix.cpp:32: return SkString("drawbitmapmatrix");
Should we just remove this GM instead of converting? Not sure whether it
exercises any interesting bitmap paths.

[mtklein, 2014-12-10 14:48:10]

I have some small comments, but overall this LGTM.  I'm happy to see it go, and
the GM too FWIW.  I don't see any calls in Android or Firefox.

I guess the obvious question is, why did this ever exist?  Was drawBitmapMatrix
ever semantically different from save-concat-drawBitmap-restore, or did we have
some more efficient path for it?

https://codereview.chromium.org/789033002/diff/1/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/789033002/diff/1/src/core/SkPictureFlat.h#new...
src/core/SkPictureFlat.h:30: DRAW_BITMAP_MATRIX, // deprecated
// deprecated, M41 was last Chrome to write this to an .skp
?

https://codereview.chromium.org/789033002/diff/1/src/pdf/SkPDFShader.cpp
File src/pdf/SkPDFShader.cpp (left):

https://codereview.chromium.org/789033002/diff/1/src/pdf/SkPDFShader.cpp#oldc...
src/pdf/SkPDFShader.cpp:1018: canvas.drawBitmapMatrix(*image, xMirror);
Seems like this file uses drawBitmapMatrix often enough that we might want to
recreate it as a static function, particularly because we're forced to manually
scope the save/restore blocks so often.

[robertphillips, 2014-12-10 15:00:37]

lgtm

[reed1, 2014-12-10 15:19:51]

lgtm

[f(malita), 2014-12-10 15:33:56]

On 2014/12/10 14:48:10, mtklein wrote:
> I guess the obvious question is, why did this ever exist?  Was
drawBitmapMatrix
> ever semantically different from save-concat-drawBitmap-restore, or did we
have
> some more efficient path for it?

I think it's always been semantically equivalent to the above - just a
convenient shortcut.

The motivation for deleting it now is that it sort of gets in the way of
refactoring some internal code paths to address
https://code.google.com/p/skia/issues/detail?id=3179.

https://codereview.chromium.org/789033002/diff/1/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/789033002/diff/1/src/core/SkPictureFlat.h#new...
src/core/SkPictureFlat.h:30: DRAW_BITMAP_MATRIX, // deprecated
On 2014/12/10 14:48:09, mtklein wrote:
> // deprecated, M41 was last Chrome to write this to an .skp
> ?

Done.

https://codereview.chromium.org/789033002/diff/1/src/pdf/SkPDFShader.cpp
File src/pdf/SkPDFShader.cpp (left):

https://codereview.chromium.org/789033002/diff/1/src/pdf/SkPDFShader.cpp#oldc...
src/pdf/SkPDFShader.cpp:1018: canvas.drawBitmapMatrix(*image, xMirror);
On 2014/12/10 14:48:09, mtklein wrote:
> Seems like this file uses drawBitmapMatrix often enough that we might want to
> recreate it as a static function, particularly because we're forced to
manually
> scope the save/restore blocks so often.

Done.

[mtklein, 2014-12-10 16:15:37]

https://codereview.chromium.org/789033002/diff/20001/src/pdf/SkPDFShader.cpp
File src/pdf/SkPDFShader.cpp (right):

https://codereview.chromium.org/789033002/diff/20001/src/pdf/SkPDFShader.cpp#...
src/pdf/SkPDFShader.cpp:472: static void drawBitmapMatrix(SkCanvas& canvas,
const SkBitmap& bm, const SkMatrix& matrix) {
-> draw_bitmap_matrix?

I'd pass canvas as SkCanvas*, but maybe that's just me.

[reed1, 2014-12-10 16:34:51]

On 2014/12/10 16:15:37, mtklein wrote:
> https://codereview.chromium.org/789033002/diff/20001/src/pdf/SkPDFShader.cpp
> File src/pdf/SkPDFShader.cpp (right):
> 
>
https://codereview.chromium.org/789033002/diff/20001/src/pdf/SkPDFShader.cpp#...
> src/pdf/SkPDFShader.cpp:472: static void drawBitmapMatrix(SkCanvas& canvas,
> const SkBitmap& bm, const SkMatrix& matrix) {
> -> draw_bitmap_matrix?
> 
> I'd pass canvas as SkCanvas*, but maybe that's just me.

+1 for * for mutable stuff (except readbuffer, don't ask)

[f(malita), 2014-12-10 17:00:57]

On 2014/12/10 16:34:51, reed1 wrote:
> On 2014/12/10 16:15:37, mtklein wrote:
> > https://codereview.chromium.org/789033002/diff/20001/src/pdf/SkPDFShader.cpp
> > File src/pdf/SkPDFShader.cpp (right):
> > 
> >
>
https://codereview.chromium.org/789033002/diff/20001/src/pdf/SkPDFShader.cpp#...
> > src/pdf/SkPDFShader.cpp:472: static void drawBitmapMatrix(SkCanvas& canvas,
> > const SkBitmap& bm, const SkMatrix& matrix) {
> > -> draw_bitmap_matrix?
> > 
> > I'd pass canvas as SkCanvas*, but maybe that's just me.
> 
> +1 for * for mutable stuff (except readbuffer, don't ask)

Done.

[f(malita), 2014-12-10 17:02:20]

Committed patchset #3 (id:40001) manually as
c54d8db4d169ea5f2af2a9a2349df007bd428475 (presubmit successful).