Remove GrFontDescKey.

Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the copies are performance critical. This gets us closer to removing GrFontScaler.

Also removes some unused member functions on GrBatchTextStrike and GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1988833003

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

[bsalomon, 2016-05-17 16:47:15]

Description was changed from

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

BUG=chromium:608566
==========

to

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-05-17 16:51:33]

Description was changed from

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

Also removes some unused member functions on GrBatchFontStrike and GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-05-17 16:51:54]

Description was changed from

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

Also removes some unused member functions on GrBatchFontStrike and GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

Also removes some unused member functions on GrBatchTextStrike and GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-05-17 16:52:23]

bsalomon@google.com changed reviewers:
+ bungeman@google.com, robertphillips@google.com

[robertphillips, 2016-05-17 17:02:24]

lgtm

[bsalomon, 2016-05-17 17:08:43]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-05-17 17:08:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1988833003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1988833003/20001

[commit-bot: I haz the power, 2016-05-17 17:10:21]

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

[commit-bot: I haz the power, 2016-05-17 17:10:22]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[bsalomon, 2016-05-17 17:22:40]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-05-17 17:22:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1988833003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-17 17:22:50]

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

[commit-bot: I haz the power, 2016-05-17 17:24:23]

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

[commit-bot: I haz the power, 2016-05-17 17:24:24]

Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[bungeman-skia, 2016-05-17 17:47:44]

https://codereview.chromium.org/1988833003/diff/20001/src/gpu/text/GrBatchFon...
File src/gpu/text/GrBatchFontCache.cpp (right):

https://codereview.chromium.org/1988833003/diff/20001/src/gpu/text/GrBatchFon...
src/gpu/text/GrBatchFontCache.cpp:162: fBatchFontCache = cache;     // no need
to ref, it won't go away before we do
This assignment seems lonely down here, why isn't it in the above member
initializer list? Also, 'cache' needs to be non-nullptr, which isn't immediately
obvious.

https://codereview.chromium.org/1988833003/diff/40001/src/gpu/text/GrBatchFon...
File src/gpu/text/GrBatchFontCache.cpp (right):

https://codereview.chromium.org/1988833003/diff/40001/src/gpu/text/GrBatchFon...
src/gpu/text/GrBatchFontCache.cpp:156:
GrBatchTextStrike::GrBatchTextStrike(GrBatchFontCache* cache, const
SkDescriptor& key)
It seems like it would be good to mention that a copy is made of 'key', but the
'cache' is expected to outlive this object (this is a tear-off of the cache).

[bsalomon, 2016-05-17 18:01:12]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-05-17 18:01:23]

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

[bsalomon, 2016-05-17 18:01:30]

The CQ bit was unchecked by bsalomon@google.com

[bsalomon, 2016-05-17 18:08:42]

https://codereview.chromium.org/1988833003/diff/20001/src/gpu/text/GrBatchFon...
File src/gpu/text/GrBatchFontCache.cpp (right):

https://codereview.chromium.org/1988833003/diff/20001/src/gpu/text/GrBatchFon...
src/gpu/text/GrBatchFontCache.cpp:162: fBatchFontCache = cache;     // no need
to ref, it won't go away before we do
On 2016/05/17 17:47:44, bungeman-skia wrote:
> This assignment seems lonely down here, why isn't it in the above member
> initializer list? Also, 'cache' needs to be non-nullptr, which isn't
immediately
> obvious.

Done. Added comment to the .h

https://codereview.chromium.org/1988833003/diff/40001/src/gpu/text/GrBatchFon...
File src/gpu/text/GrBatchFontCache.cpp (right):

https://codereview.chromium.org/1988833003/diff/40001/src/gpu/text/GrBatchFon...
src/gpu/text/GrBatchFontCache.cpp:156:
GrBatchTextStrike::GrBatchTextStrike(GrBatchFontCache* cache, const
SkDescriptor& key)
On 2016/05/17 17:47:44, bungeman-skia wrote:
> It seems like it would be good to mention that a copy is made of 'key', but
the
> 'cache' is expected to outlive this object (this is a tear-off of the cache).

Added mention of this to the class description comment.

[bungeman-skia, 2016-05-17 18:30:07]

It looks less difficult to follow than it did before, and so long as making
copies of the descriptor is ok performance wise, lgtm.

[bsalomon, 2016-05-17 18:33:35]

On 2016/05/17 18:30:07, bungeman-skia wrote:
> It looks less difficult to follow than it did before, and so long as making
> copies of the descriptor is ok performance wise, lgtm.

My understanding is that a copy will be made roughly once per strike. I think it
will be acceptable. Also, my read is that in most cases there was already a copy
into the GrFontDescKey that then got ref'ed rather than copied later.

I'm deliberately landing this before removal of GrFontScaler to separate any
perf impacts from that (forthcoming) change.

[bsalomon, 2016-05-17 18:33:39]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-05-17 18:33:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1988833003/#ps80001
(title: "spelling fix")

[commit-bot: I haz the power, 2016-05-17 18:33:48]

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

[commit-bot: I haz the power, 2016-05-17 18:58:26]

Description was changed from

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

Also removes some unused member functions on GrBatchTextStrike and GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Remove GrFontDescKey.

This was a reference counted wrapper of SkDescriptor. It doesn't seem like the
copies are performance critical. This gets us closer to removing GrFontScaler.

Also removes some unused member functions on GrBatchTextStrike and GrFontScaler.

BUG=chromium:608566
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/c5fd5c41abd1e50293643c40e11e1f62cd22d3e9
==========

[commit-bot: I haz the power, 2016-05-17 18:58:28]

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