Use uint16s for texture coordinates when rendering text.

Use uint16s for texture coordinates when rendering text.

Allows us to push more vertices into a given vertex buffer, with
a slight performance improvement.

Committed: https://skia.googlesource.com/skia/+/059034d252007d0dd86fff5ffdbb53cbcb10d34b

Committed: https://skia.googlesource.com/skia/+/5a105ff05303ac82a867b8b84a1edd145bd46218

[jvanverth1, 2015-02-13 15:47:54]

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

[jvanverth1, 2015-02-13 15:47:55]

Seems to provide a mild improvement in text rendering -- running telemetry on my
Nexus 7 gives around 1.87% improvement (58.9 to 60 fps) for the nytimes site.

[robertphillips, 2015-02-13 16:10:42]

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrAADistanceFiel...
File src/gpu/GrAADistanceFieldPathRenderer.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrAADistanceFiel...
src/gpu/GrAADistanceFieldPathRenderer.cpp:349: SkScalar dy =
pathData->fBounds.fTop;
round_out_WH ?
comment ?
Would it not be better to explicit round out the bounds and then offset it to
the atlas location?

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrBitmapTextCont...
File src/gpu/GrBitmapTextContext.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrBitmapTextCont...
src/gpu/GrBitmapTextContext.cpp:505: }
overlength ?

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrDistanceFieldT...
File src/gpu/GrDistanceFieldTextContext.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:619: }
overlength ?

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrBitmap...
File src/gpu/effects/GrBitmapTextGeoProc.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrBitmap...
src/gpu/effects/GrBitmapTextGeoProc.cpp:39: pb->addVarying("TextureCoords", &v);
Don't we have #defines for these ?
Also comment ?

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

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:339: 
comment about why necessary here ?

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:341: fRecipTextureSizeUni =
args.fPB->addUniform(GrGLProgramBuilder::kVertex_Visibility,
line these guys up ?

[jvanverth1, 2015-02-13 19:58:56]

Updated with changes.

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrAADistanceFiel...
File src/gpu/GrAADistanceFieldPathRenderer.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrAADistanceFiel...
src/gpu/GrAADistanceFieldPathRenderer.cpp:349: SkScalar dy =
pathData->fBounds.fTop;
On 2015/02/13 16:10:41, robertphillips wrote:
> round_out_WH ?
> comment ?
> Would it not be better to explicit round out the bounds and then offset it to
> the atlas location?

Rounding out the bounds seems to add an extra texel, which doesn't give the
right result. This is a little wonky, but gives the same result as before.

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrBitmapTextCont...
File src/gpu/GrBitmapTextContext.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrBitmapTextCont...
src/gpu/GrBitmapTextContext.cpp:505: }
On 2015/02/13 16:10:42, robertphillips wrote:
> overlength ?

Done.

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrDistanceFieldT...
File src/gpu/GrDistanceFieldTextContext.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:619: }
On 2015/02/13 16:10:42, robertphillips wrote:
> overlength ?

Done.

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrBitmap...
File src/gpu/effects/GrBitmapTextGeoProc.cpp (right):

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrBitmap...
src/gpu/effects/GrBitmapTextGeoProc.cpp:39: pb->addVarying("TextureCoords", &v);
On 2015/02/13 16:10:42, robertphillips wrote:
> Don't we have #defines for these ?
> Also comment ?

Done.

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

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:339: 
On 2015/02/13 16:10:42, robertphillips wrote:
> comment about why necessary here ?

Done.

https://codereview.chromium.org/917373002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.cpp:341: fRecipTextureSizeUni =
args.fPB->addUniform(GrGLProgramBuilder::kVertex_Visibility,
On 2015/02/13 16:10:42, robertphillips wrote:
> line these guys up ?

Done.

[robertphillips, 2015-02-13 22:38:26]

lgtm

[robertphillips, 2015-02-13 22:38:33]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-02-13 22:38:47]

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

[commit-bot: I haz the power, 2015-02-13 22:38:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-13 22:38:52]

Presubmit check for 917373002-40001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Since the CL is editing public API, you must have an LGTM from one of:
('reed@chromium.org', 'reed@google.com', 'bsalomon@chromium.org',
'bsalomon@google.com', 'djsollen@chromium.org', 'djsollen@google.com')

[jvanverth1, 2015-02-17 14:27:46]

jvanverth@google.com changed reviewers:
+ bsalomon@google.com, djsollen@google.com, reed@google.com

[jvanverth1, 2015-02-17 14:27:47]

I need approval from one of the keepers of the public API. PTAL.

[bsalomon, 2015-02-17 15:12:34]

lgtm w/ tiny nit

https://codereview.chromium.org/917373002/diff/40001/include/gpu/GrTypesPriv.h
File include/gpu/GrTypesPriv.h (right):

https://codereview.chromium.org/917373002/diff/40001/include/gpu/GrTypesPriv....
include/gpu/GrTypesPriv.h:157: 2*sizeof(uint16_t)      //
kVec2s_GrVertexAttribType
tiny nit, shouldn't this be int16_t?

[jvanverth1, 2015-02-17 15:34:42]

New patchsets have been uploaded after l-g-t-m from
robertphillips@google.com,bsalomon@google.com

[jvanverth1, 2015-02-17 15:38:50]

https://codereview.chromium.org/917373002/diff/40001/include/gpu/GrTypesPriv.h
File include/gpu/GrTypesPriv.h (right):

https://codereview.chromium.org/917373002/diff/40001/include/gpu/GrTypesPriv....
include/gpu/GrTypesPriv.h:157: 2*sizeof(uint16_t)      //
kVec2s_GrVertexAttribType
On 2015/02/17 15:12:34, bsalomon wrote:
> tiny nit, shouldn't this be int16_t?

Done.

[jvanverth1, 2015-02-17 15:38:56]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2015-02-17 15:39:27]

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

[commit-bot: I haz the power, 2015-02-17 16:03:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-17 16:03:41]

Try jobs failed on following builders:
  Build-Win-VS2013-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-VS2013-x86...)

[jvanverth1, 2015-02-17 16:08:39]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2015-02-17 16:09:58]

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

[commit-bot: I haz the power, 2015-02-17 16:40:00]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/059034d252007d0dd86fff5ffdbb53cbcb10d34b

[reed2, 2015-02-18 02:38:20]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/920333003/ by reed@chromium.org.

The reason for reverting is: speculative revert for DEPS failures

https://codereview.chromium.org/932973002/.

[scroggo, 2015-02-18 14:27:46]

On 2015/02/18 02:38:20, reed2 wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
> https://codereview.chromium.org/920333003/ by mailto:reed@chromium.org.
> 
> The reason for reverting is: speculative revert for DEPS failures
> 
> https://codereview.chromium.org/932973002/.

It seems to have also caused a valgrind error:
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu12-ShuttleA-GTX55...

(skbug.com/3438)

[jvanverth1, 2015-02-18 19:16:02]

Updated to use float uvs for distance field paths (via
GrDistanceFieldNoGammaTextureEffect) and use the cut-down version of
GrDistanceFieldTextureEffect (which should be equivalent to the NoGamma version,
but with short uvs) for the cases where we don't apply gamma to A8 text.

[jvanverth1, 2015-02-18 19:21:56]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2015-02-18 19:22:19]

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

[commit-bot: I haz the power, 2015-02-18 19:36:40]

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