update comment on setShader to clarify alpha-bitmap behavior in bitmapshaders

update comment on setShader to clarify alpha-bitmap behavior in bitmapshaders

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

[reed1, 2014-03-18 14:28:15]

comment inspired by https://codereview.chromium.org/196473010/

[bsalomon, 2014-03-18 14:30:49]

lgtm

[bsalomon, 2014-03-18 14:30:50]

lgtm

lgtm

[yunchao, 2014-03-18 14:38:35]

lgtm with a suggestion.

https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp
File gm/bitmapshader.cpp (right):

https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp#newcode39
gm/bitmapshader.cpp:39: paint->setShader(shader);
I suggest that the shader set its own matrix. like this:
        SkMatrix s;
        s.reset();
        s.setScale(2, 2);
        s.postTranslate(5, 5);
        shader->setLocalMatrix(s);

Because drawBitmap may have error for gpu if shader set its own local matrix
when the bitmap is kA8_Config.

[reed1, 2014-03-18 15:11:09]

On 2014/03/18 14:38:35, Richard Ho wrote:
> lgtm with a suggestion.
> 
> https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp
> File gm/bitmapshader.cpp (right):
> 
> https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp#newcode39
> gm/bitmapshader.cpp:39: paint->setShader(shader);
> I suggest that the shader set its own matrix. like this:
>         SkMatrix s;
>         s.reset();
>         s.setScale(2, 2);
>         s.postTranslate(5, 5);
>         shader->setLocalMatrix(s);
> 
> Because drawBitmap may have error for gpu if shader set its own local matrix
> when the bitmap is kA8_Config.

Great suggestion -- I will try that.

[reed1, 2014-03-18 15:22:59]

On 2014/03/18 15:11:09, reed1 wrote:
> On 2014/03/18 14:38:35, Richard Ho wrote:
> > lgtm with a suggestion.
> > 
> > https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp
> > File gm/bitmapshader.cpp (right):
> > 
> >
https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp#newcode39
> > gm/bitmapshader.cpp:39: paint->setShader(shader);
> > I suggest that the shader set its own matrix. like this:
> >         SkMatrix s;
> >         s.reset();
> >         s.setScale(2, 2);
> >         s.postTranslate(5, 5);
> >         shader->setLocalMatrix(s);
> > 
> > Because drawBitmap may have error for gpu if shader set its own local matrix
> > when the bitmap is kA8_Config.
> 
> Great suggestion -- I will try that.

Actually, that will take a little time to construct well. I may land this CL as
is, since we still want to track the bug you've found, and then if you like, we
can augment this GM or create a new one to explore localMatrix behavior.

[reed1, 2014-03-18 15:23:11]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-03-18 15:23:21]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/reed@google.com/203203005/1

[yunchao, 2014-03-18 15:26:30]

On 2014/03/18 15:22:59, reed1 wrote:
> On 2014/03/18 15:11:09, reed1 wrote:
> > On 2014/03/18 14:38:35, Richard Ho wrote:
> > > lgtm with a suggestion.
> > > 
> > > https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp
> > > File gm/bitmapshader.cpp (right):
> > > 
> > >
> https://codereview.chromium.org/203203005/diff/1/gm/bitmapshader.cpp#newcode39
> > > gm/bitmapshader.cpp:39: paint->setShader(shader);
> > > I suggest that the shader set its own matrix. like this:
> > >         SkMatrix s;
> > >         s.reset();
> > >         s.setScale(2, 2);
> > >         s.postTranslate(5, 5);
> > >         shader->setLocalMatrix(s);
> > > 
> > > Because drawBitmap may have error for gpu if shader set its own local
matrix
> > > when the bitmap is kA8_Config.
> > 
> > Great suggestion -- I will try that.
> 
> Actually, that will take a little time to construct well. I may land this CL
as
> is, since we still want to track the bug you've found, and then if you like,
we
> can augment this GM or create a new one to explore localMatrix behavior.

sounds good.

[commit-bot: I haz the power, 2014-03-18 16:28:17]

Change committed as 13851