A small patch to enable distance field text in textblobs

A small patch to enable distance field text in textblobs

BUG=skia:

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

[joshualitt, 2015-04-17 16:32:34]

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

[joshualitt, 2015-04-17 16:32:35]

PTAL

[jvanverth1, 2015-04-20 17:54:07]

Some comments:

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.cpp (right):

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:284: // Mixed blobs can't be regenerated at this
point
Don't you mean "must be regenerated"? Or "can't be reused"?

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:299: // We can update the positions in the
cachedtextblobs without regenerating the whole blob,
I'd think that this formula would work for distance fields, too. It's just that
distance fields can handle more than this.

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:1517: regenerateColors = !fUseLCDText &&
run.fColor != args.fColor;
Whoops. :)

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.h (right):

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.h:212: bool fHasDistanceField;
I think a bitfield and an enum with two values would be better here.

[joshualitt, 2015-04-20 18:24:57]

feedback inc + a question

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.cpp (right):

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:284: // Mixed blobs can't be regenerated at this
point
On 2015/04/20 17:54:07, jvanverth1 wrote:
> Don't you mean "must be regenerated"? Or "can't be reused"?

Acknowledged.

https://codereview.chromium.org/1098653005/diff/40001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:299: // We can update the positions in the
cachedtextblobs without regenerating the whole blob,
On 2015/04/20 17:54:07, jvanverth1 wrote:
> I'd think that this formula would work for distance fields, too. It's just
that
> distance fields can handle more than this.

would it? I thought we just want the raw delta (x - blob.fX, y - blob.fY)
because distance field text doesn't do positioning on the CPU yet.

[jvanverth1, 2015-04-21 14:04:34]

LGTM + comment:

https://codereview.chromium.org/1098653005/diff/60001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.h (right):

https://codereview.chromium.org/1098653005/diff/60001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.h:217: uint8_t fTextType;
This member will be padded due to the unsigned char* following it, so you might
as well use uint32_t.

[joshualitt, 2015-04-21 14:05:47]

On 2015/04/21 14:04:34, jvanverth1 wrote:
> LGTM + comment:
> 
>
https://codereview.chromium.org/1098653005/diff/60001/src/gpu/GrAtlasTextCont...
> File src/gpu/GrAtlasTextContext.h (right):
> 
>
https://codereview.chromium.org/1098653005/diff/60001/src/gpu/GrAtlasTextCont...
> src/gpu/GrAtlasTextContext.h:217: uint8_t fTextType;
> This member will be padded due to the unsigned char* following it, so you
might
> as well use uint32_t.

I actually repack this guy on a follow on CL, so I'll make sure to put the
uint8_t at the end.

[bsalomon, 2015-04-21 14:13:30]

tiny nit, lgtm

https://codereview.chromium.org/1098653005/diff/60001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.cpp (right):

https://codereview.chromium.org/1098653005/diff/60001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:502: if (canDrawAsDistanceFields(runPaint,
viewMatrix)) {
this->

[joshualitt, 2015-04-21 14:23:16]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-04-21 14:23:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
jvanverth@google.com, bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1098653005/#ps100001
(title: "windows warnings")

[commit-bot: I haz the power, 2015-04-21 14:23:30]

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

[commit-bot: I haz the power, 2015-04-21 14:35:13]

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