Calculate inverse scale for distance fields in vertex shader

Calculate inverse scale for distance field text in vertex shader

This is for the uniform scale case only. Using the dFdx() function on certain
Mali GPUs causes issues because the precision is too low, so we have to
compute 1/scale from the view matrix instead.

BUG=skia:3528
Committed: https://skia.googlesource.com/skia/+/5b143038cb47763974d2750ed78d436eb6c38bea

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

[jvanverth1, 2015-04-03 20:03:05]

jvanverth@google.com changed reviewers:
+ bsalomon@google.com, joshualitt@google.com

[bsalomon, 2015-04-03 21:16:30]

Does this have any affect on batching? Otherwise, lgtm

https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
File src/gpu/effects/GrDistanceFieldTextureEffect.cpp (right):

https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:77: bool isSimilarity =
!!(dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag);
SkToBool?

https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:602: bool isUniformScale =
!!(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
SkToBool?

[jvanverth1, 2015-04-06 14:14:47]

On 2015/04/03 21:16:30, bsalomon wrote:
> Does this have any affect on batching? Otherwise, lgtm

In thinking about it, it could. By (effectively) using geometry to scale, we
potentially batch a lot more than doing this, because with this we'll be 
tweaking the view matrix every time we change text size. So really this is no
better than just passing down another uniform. In the long run it will be 
better to use some kind of check to know whether we're on a Mali 400 or not --
but the bug is out there now, as we're using distance fields to scale up 
text beyond a certain size.

Comments addressed.

https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
File src/gpu/effects/GrDistanceFieldTextureEffect.cpp (right):

https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:77: bool isSimilarity =
!!(dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag);
On 2015/04/03 21:16:30, bsalomon wrote:
> SkToBool?

Done.

https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:602: bool isUniformScale =
!!(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
On 2015/04/03 21:16:30, bsalomon wrote:
> SkToBool?

Done.

[bsalomon, 2015-04-06 16:10:39]

On 2015/04/06 14:14:47, jvanverth1 wrote:
> On 2015/04/03 21:16:30, bsalomon wrote:
> > Does this have any affect on batching? Otherwise, lgtm
> 
> In thinking about it, it could. By (effectively) using geometry to scale, we
> potentially batch a lot more than doing this, because with this we'll be 
> tweaking the view matrix every time we change text size. So really this is no
> better than just passing down another uniform. In the long run it will be 
> better to use some kind of check to know whether we're on a Mali 400 or not --
> but the bug is out there now, as we're using distance fields to scale up 
> text beyond a certain size.
> 
> Comments addressed.
> 
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> File src/gpu/effects/GrDistanceFieldTextureEffect.cpp (right):
> 
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> src/gpu/effects/GrDistanceFieldTextureEffect.cpp:77: bool isSimilarity =
> !!(dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag);
> On 2015/04/03 21:16:30, bsalomon wrote:
> > SkToBool?
> 
> Done.
> 
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> src/gpu/effects/GrDistanceFieldTextureEffect.cpp:602: bool isUniformScale =
> !!(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
> On 2015/04/03 21:16:30, bsalomon wrote:
> > SkToBool?
> 
> Done.

Perhaps we should consider adding a float scale attr post batch conversion...
anyway, lgtm.

[jvanverth1, 2015-04-06 16:45:20]

On 2015/04/06 16:10:39, bsalomon wrote:
> On 2015/04/06 14:14:47, jvanverth1 wrote:
> > On 2015/04/03 21:16:30, bsalomon wrote:
> > > Does this have any affect on batching? Otherwise, lgtm
> > 
> > In thinking about it, it could. By (effectively) using geometry to scale, we
> > potentially batch a lot more than doing this, because with this we'll be 
> > tweaking the view matrix every time we change text size. So really this is
no
> > better than just passing down another uniform. In the long run it will be 
> > better to use some kind of check to know whether we're on a Mali 400 or not
--
> > but the bug is out there now, as we're using distance fields to scale up 
> > text beyond a certain size.
> > 
> > Comments addressed.
> > 
> >
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> > File src/gpu/effects/GrDistanceFieldTextureEffect.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> > src/gpu/effects/GrDistanceFieldTextureEffect.cpp:77: bool isSimilarity =
> > !!(dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag);
> > On 2015/04/03 21:16:30, bsalomon wrote:
> > > SkToBool?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> > src/gpu/effects/GrDistanceFieldTextureEffect.cpp:602: bool isUniformScale =
> > !!(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
> > On 2015/04/03 21:16:30, bsalomon wrote:
> > > SkToBool?
> > 
> > Done.
> 
> Perhaps we should consider adding a float scale attr post batch conversion...
> anyway, lgtm.

I've updated the patch to only apply this fix if we're scaling up from 162
point, which should restore batching for the majority of cases.

[joshualitt, 2015-04-06 18:31:40]

On 2015/04/06 16:45:20, jvanverth1 wrote:
> On 2015/04/06 16:10:39, bsalomon wrote:
> > On 2015/04/06 14:14:47, jvanverth1 wrote:
> > > On 2015/04/03 21:16:30, bsalomon wrote:
> > > > Does this have any affect on batching? Otherwise, lgtm
> > > 
> > > In thinking about it, it could. By (effectively) using geometry to scale,
we
> > > potentially batch a lot more than doing this, because with this we'll be 
> > > tweaking the view matrix every time we change text size. So really this is
> no
> > > better than just passing down another uniform. In the long run it will be 
> > > better to use some kind of check to know whether we're on a Mali 400 or
not
> --
> > > but the bug is out there now, as we're using distance fields to scale up 
> > > text beyond a certain size.
> > > 
> > > Comments addressed.
> > > 
> > >
> >
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> > > File src/gpu/effects/GrDistanceFieldTextureEffect.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> > > src/gpu/effects/GrDistanceFieldTextureEffect.cpp:77: bool isSimilarity =
> > > !!(dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag);
> > > On 2015/04/03 21:16:30, bsalomon wrote:
> > > > SkToBool?
> > > 
> > > Done.
> > > 
> > >
> >
>
https://codereview.chromium.org/1029423003/diff/20001/src/gpu/effects/GrDista...
> > > src/gpu/effects/GrDistanceFieldTextureEffect.cpp:602: bool isUniformScale
=
> > > !!(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
> > > On 2015/04/03 21:16:30, bsalomon wrote:
> > > > SkToBool?
> > > 
> > > Done.
> > 
> > Perhaps we should consider adding a float scale attr post batch
conversion...
> > anyway, lgtm.
> 
> I've updated the patch to only apply this fix if we're scaling up from 162
> point, which should restore batching for the majority of cases.

lgtm

[jvanverth1, 2015-04-06 18:32:29]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2015-04-06 18:32:30]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1029423003/#ps100001
(title: "Swap sense of key bit")

[commit-bot: I haz the power, 2015-04-06 18:32:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1029423003/100001

[commit-bot: I haz the power, 2015-04-06 18:39:03]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/5b143038cb47763974d2750ed78d436eb6c38bea

[jvanverth1, 2015-04-06 20:58:39]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1066613003/ by jvanverth@google.com.

The reason for reverting is: This appears to be breaking large text on Nexus 7..

[jvanverth1, 2015-04-07 15:49:53]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2015-04-07 15:49:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com, bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1029423003/#ps120001
(title: "Fix shader for Tegra3")

[commit-bot: I haz the power, 2015-04-07 15:50:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1029423003/120001

[commit-bot: I haz the power, 2015-04-07 16:06:03]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/bb4a1cf0e66c98c723f04f473a3221b2a4d8ece1