Remove setLocalMatrix calls from picture shader GM.

Remove setLocalMatrix calls from picture shader GM.

This makes all --skr tests pass for me.  Enabling it by default in DM.

BUG=skia:2378
Committed: http://code.google.com/p/skia/source/detail?r=14549

Committed: http://code.google.com/p/skia/source/detail?r=14551

[reed1, 2014-05-02 13:38:26]

works for me, but Florin is the original author (I think)

[f(malita), 2014-05-02 13:57:35]

On 2014/05/02 13:38:26, reed1 wrote:
> works for me, but Florin is the original author (I think)

I'm missing the background story here: why are we trying to avoid
setLocalMatrix() calls?

Two concerns:

* the new approach no longer verifies correct SkPictureShader tile caching (we
instantiate a fresh shader each time)
* external clients can still hit whatever setLocalMatrix() issue we're dodging
here?

[f(malita), 2014-05-02 14:01:20]

On 2014/05/02 13:57:35, Florin Malita wrote:
> On 2014/05/02 13:38:26, reed1 wrote:
> > works for me, but Florin is the original author (I think)
> 
> I'm missing the background story here: why are we trying to avoid
> setLocalMatrix() calls?
> 
> Two concerns:
> 
> * the new approach no longer verifies correct SkPictureShader tile caching (we
> instantiate a fresh shader each time)
> * external clients can still hit whatever setLocalMatrix() issue we're dodging
> here?

Never mind, I see Dominik's CL. If setLocalMatrix is being deprecated, then I'm
fine with this.

LGTM.

[mtklein, 2014-05-02 14:04:57]

> * the new approach no longer verifies correct SkPictureShader tile caching (we
> instantiate a fresh shader each time)

Ooh, don't want to break this.  Is this a feature we could still test without
setLocalMatrix, or does it become impossible without it?

[f(malita), 2014-05-02 14:20:35]

On 2014/05/02 14:04:57, mtklein wrote:
> > * the new approach no longer verifies correct SkPictureShader tile caching
(we
> > instantiate a fresh shader each time)
> 
> Ooh, don't want to break this.  Is this a feature we could still test without
> setLocalMatrix, or does it become impossible without it?

Well, the local matrix is part of the (logical) cache key. But if the plan is
for it to become immutable, then I don't think it's a problem to lose this
coverage (and remove it from the key when the public API gets trimmed).

Let's add a FIXME to SkPictureShader::refBitmapShader() where we check
this->getLocalMatrix() != fCachedLocalMatrix to remove it after setLocalMatrix()
is gone.

[mtklein, 2014-05-02 19:07:09]

> Let's add a FIXME to SkPictureShader::refBitmapShader() where we check
> this->getLocalMatrix() != fCachedLocalMatrix to remove it after
setLocalMatrix()
> is gone.
Done.

[mtklein, 2014-05-02 19:07:35]

On 2014/05/02 19:07:09, mtklein wrote:
> > Let's add a FIXME to SkPictureShader::refBitmapShader() where we check
> > this->getLocalMatrix() != fCachedLocalMatrix to remove it after
> setLocalMatrix()
> > is gone.
> Done.

Mike, can I get your stamp for SkShader.h?

[reed1, 2014-05-02 20:06:28]

lgtm

[mtklein, 2014-05-02 20:16:28]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-05-02 20:16:42]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/260863007/20001

[commit-bot: I haz the power, 2014-05-02 20:30:23]

Change committed as 14549

[mtklein, 2014-05-02 20:50:54]

A revert of this CL has been created in
https://codereview.chromium.org/265013003/ by mtklein@google.com.

The reason for reverting is: changed GM unexpectedly.  will sort out and try
again.

[mtklein, 2014-05-02 21:16:51]

On 2014/05/02 20:50:54, mtklein wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/265013003/ by mailto:mtklein@google.com.
> 
> The reason for reverting is: changed GM unexpectedly.  will sort out and try
> again.

Dumb bug, now fixed.  Turns out you really need to pass that local matrix to the
constructor, not just drop it on the floor in Create()...

[mtklein, 2014-05-02 21:17:03]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-05-02 21:17:17]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/260863007/40001

[commit-bot: I haz the power, 2014-05-02 21:24:03]

Change committed as 14551