Implement vertex buffer caching in the tessellated path renderer.

Implement caching of filled paths in the tessellated path renderer.

Paths are cached as tessellated triangle meshes in vertex buffers on the GPU. Stroked paths are not (yet) cached.

Paths containing no curved segments (linear paths) are reused at all scales. Paths containing curved segments are reused within a scale tolerance threshold.

In order to invalidate the cache when an SkPath is changed or deleted, 
this required implementing genID change notification in SkPath. This is 
modelled almost exactly on SkPixelRef::GenIDChangeListener. 
However, It does not currently implement the check for unique genIDs, 
so notifiers will fire when the first instance of an SkPathRef 
using a given genID is destroyed.

Another caveat is that you cannot successfully add a change notifier 
to an empty path, since it uses the "canonical" empty path which is 
never modified or destroyed. For this reason, we prevent adding 
listeners to it.

BUG=skia:4121, skia:4122, 497403
DOCS_PREVIEW= https://skia.org/?cl=1114353004

Committed: https://skia.googlesource.com/skia/+/468dfa72eb6694145487be17876804dfca3b7adb

Committed: https://skia.googlesource.com/skia/+/84cd621670a357484e1674e06d3d8d6f929a4ab2

[Stephen White, 2015-07-27 21:25:05]

senorblanco@chromium.org changed reviewers:
+ bsalomon@chromium.org

[Stephen White, 2015-07-27 21:25:28]

Brian: PTAL. Thanks!

[Stephen White, 2015-07-28 14:46:52]

senorblanco@chromium.org changed reviewers:
+ bsalomon@google.com, reed@google.com
- bsalomon@chromium.org

[bsalomon, 2015-07-28 14:55:48]

We sometimes copy the genID of SkPathRef... perhaps we should first eliminate
those copies so that we don't have to worry about multiple owners of the genID.
I don't think the genId copying really helps with perf anywhere.

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1346: const SkSize* cachedScale =
static_cast<const SkSize*>(data->data());
Rather than use the data on the VB, can we canonicalize the tessellation levels
and include the scales in the key?

The rules around data on cached objects are a bit nebulous and I'd just as soon
remove the data if we can.

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.h (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.h:44: GrContext* fContext;
Do we need a long lived ptr back to the context? Is it just for the resource
provider? I think that is accessible by the BatchTarget.

[Stephen White, 2015-07-28 14:58:22]

Patchset #14 (id:260001) has been deleted

[Stephen White, 2015-07-28 15:10:28]

Thanks for your review.

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1346: const SkSize* cachedScale =
static_cast<const SkSize*>(data->data());
On 2015/07/28 14:55:48, bsalomon wrote:
> Rather than use the data on the VB, can we canonicalize the tessellation
levels
> and include the scales in the key?
> 
> The rules around data on cached objects are a bit nebulous and I'd just as
soon
> remove the data if we can.

I could do that, but then a single path with an animated scale will quickly fill
up the cache with lots of copies of the same path at different scales. This way,
we only have a single cached copy of a given path, with I think is a better use
of cache resources.

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.h (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.h:44: GrContext* fContext;
On 2015/07/28 14:55:48, bsalomon wrote:
> Do we need a long lived ptr back to the context? Is it just for the resource
> provider? I think that is accessible by the BatchTarget.

It's to get the GrGpu to call createVertexBuffer(), as well as to get the
resource provider. I modelled it on GrAADistanceFieldPathRenderer, but if
there's a better way of doing this, let me know.

[Stephen White, 2015-07-28 20:51:40]

New patch: made pathRef() private, and used friendship for access.

[Stephen White, 2015-07-28 20:54:08]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-28 20:54:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1114353004/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1114353004/300001

[commit-bot: I haz the power, 2015-07-28 20:57:52]

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

[commit-bot: I haz the power, 2015-07-28 20:57:53]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[reed1, 2015-07-28 21:02:55]

https://codereview.chromium.org/1114353004/diff/300001/include/core/SkPathRef.h
File include/core/SkPathRef.h (right):

https://codereview.chromium.org/1114353004/diff/300001/include/core/SkPathRef...
include/core/SkPathRef.h:248: void
removeGenIDChangeListener(GenIDChangeListener* listener);
are there any callers for remove? we don't offer this on pixelref, so I was
surprised to see it here.

[Stephen White, 2015-07-28 21:15:08]

On 2015/07/28 21:02:55, reed1 wrote:
>
https://codereview.chromium.org/1114353004/diff/300001/include/core/SkPathRef.h
> File include/core/SkPathRef.h (right):
> 
>
https://codereview.chromium.org/1114353004/diff/300001/include/core/SkPathRef...
> include/core/SkPathRef.h:248: void
> removeGenIDChangeListener(GenIDChangeListener* listener);
> are there any callers for remove? we don't offer this on pixelref, so I was
> surprised to see it here.

Not only are there no callers, there's no implementation. :) Removed.

[Stephen White, 2015-07-28 21:16:10]

New patch up; PTAL.

[Stephen White, 2015-07-28 21:16:24]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-28 21:16:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1114353004/320001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1114353004/320001

[bsalomon, 2015-07-28 21:23:30]

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1346: const SkSize* cachedScale =
static_cast<const SkSize*>(data->data());
On 2015/07/28 15:10:28, Stephen White wrote:
> On 2015/07/28 14:55:48, bsalomon wrote:
> > Rather than use the data on the VB, can we canonicalize the tessellation
> levels
> > and include the scales in the key?
> > 
> > The rules around data on cached objects are a bit nebulous and I'd just as
> soon
> > remove the data if we can.
> 
> I could do that, but then a single path with an animated scale will quickly
fill
> up the cache with lots of copies of the same path at different scales. This
way,
> we only have a single cached copy of a given path, with I think is a better
use
> of cache resources.

That makes sense. I wonder if there is an efficient way of accomplishing the
same goal without stashing it in the SkData. Otherwise, I'd like to see if we
can alter the cache to excise the data when a content resource is recycled as a
scratch resource.

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.h (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.h:44: GrContext* fContext;
On 2015/07/28 15:10:28, Stephen White wrote:
> On 2015/07/28 14:55:48, bsalomon wrote:
> > Do we need a long lived ptr back to the context? Is it just for the resource
> > provider? I think that is accessible by the BatchTarget.
> 
> It's to get the GrGpu to call createVertexBuffer(), as well as to get the
> resource provider. I modelled it on GrAADistanceFieldPathRenderer, but if
> there's a better way of doing this, let me know.

Gotcha, that code predates some refactoring. We want to stop access GrGpu
through GrContext. We should extend GrResourceProvider to provide facilities for
creating cached VBs rather than directly using GrGpu.

[commit-bot: I haz the power, 2015-07-28 21:24:55]

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

[commit-bot: I haz the power, 2015-07-28 21:24:55]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2015-07-28 21:27:40]

https://codereview.chromium.org/1114353004/diff/320001/src/core/SkPathRef.cpp
File src/core/SkPathRef.cpp (right):

https://codereview.chromium.org/1114353004/diff/320001/src/core/SkPathRef.cpp...
src/core/SkPathRef.cpp:236: if (0 == fGenerationID) {
Let's kill this code. It can lead to two owners of the same gen ID and a call to
a listener when one but not the other is destroyed.

https://codereview.chromium.org/1114353004/diff/320001/src/core/SkPathRef.cpp...
src/core/SkPathRef.cpp:290: fGenerationID = ref.fGenerationID;
Same.. based on the above comment it is unlikely that the copied gen id will be
useful since it is about to be reset upon edit.

[Stephen White, 2015-07-28 22:11:23]

Thanks for your review. New patch up.

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1346: const SkSize* cachedScale =
static_cast<const SkSize*>(data->data());
On 2015/07/28 21:23:30, bsalomon wrote:
> On 2015/07/28 15:10:28, Stephen White wrote:
> > On 2015/07/28 14:55:48, bsalomon wrote:
> > > Rather than use the data on the VB, can we canonicalize the tessellation
> > levels
> > > and include the scales in the key?
> > > 
> > > The rules around data on cached objects are a bit nebulous and I'd just as
> > soon
> > > remove the data if we can.
> > 
> > I could do that, but then a single path with an animated scale will quickly
> fill
> > up the cache with lots of copies of the same path at different scales. This
> way,
> > we only have a single cached copy of a given path, with I think is a better
> use
> > of cache resources.
> 
> That makes sense. I wonder if there is an efficient way of accomplishing the
> same goal without stashing it in the SkData. Otherwise, I'd like to see if we
> can alter the cache to excise the data when a content resource is recycled as
a
> scratch resource.

I've done the latter (I think..)

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.h (right):

https://codereview.chromium.org/1114353004/diff/240001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.h:44: GrContext* fContext;
On 2015/07/28 21:23:30, bsalomon wrote:
> On 2015/07/28 15:10:28, Stephen White wrote:
> > On 2015/07/28 14:55:48, bsalomon wrote:
> > > Do we need a long lived ptr back to the context? Is it just for the
resource
> > > provider? I think that is accessible by the BatchTarget.
> > 
> > It's to get the GrGpu to call createVertexBuffer(), as well as to get the
> > resource provider. I modelled it on GrAADistanceFieldPathRenderer, but if
> > there's a better way of doing this, let me know.
> 
> Gotcha, that code predates some refactoring. We want to stop access GrGpu
> through GrContext. We should extend GrResourceProvider to provide facilities
for
> creating cached VBs rather than directly using GrGpu.

Logged as http://skbug.com/4130.

https://codereview.chromium.org/1114353004/diff/320001/src/core/SkPathRef.cpp
File src/core/SkPathRef.cpp (right):

https://codereview.chromium.org/1114353004/diff/320001/src/core/SkPathRef.cpp...
src/core/SkPathRef.cpp:236: if (0 == fGenerationID) {
On 2015/07/28 21:27:40, bsalomon wrote:
> Let's kill this code. It can lead to two owners of the same gen ID and a call
to
> a listener when one but not the other is destroyed.

Done.

https://codereview.chromium.org/1114353004/diff/320001/src/core/SkPathRef.cpp...
src/core/SkPathRef.cpp:290: fGenerationID = ref.fGenerationID;
On 2015/07/28 21:27:40, bsalomon wrote:
> Same.. based on the above comment it is unlikely that the copied gen id will
be
> useful since it is about to be reset upon edit.

Done. Looks like resetToSize() above will leave it at zero, which should do the
right thing.

[Stephen White, 2015-07-28 22:11:30]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-28 22:11:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1114353004/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1114353004/340001

[commit-bot: I haz the power, 2015-07-28 22:19:25]

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

[commit-bot: I haz the power, 2015-07-28 22:19:25]

Dry run: This issue passed the CQ dry run.

[Stephen White, 2015-07-30 21:33:54]

New patch up: removed the "size" custom SkData on the VB; placed a bucketed
tolerance in the cache key instead.

This required losing the isLinear optimization (where linear paths always match
the cache) since we don't always have that
info at key computation time. Will reimplement it on SkPath instead.

[Stephen White, 2015-08-03 17:44:06]

Patchset #23 (id:460001) has been deleted

[Stephen White, 2015-08-03 17:50:23]

PTAL: new patch up. Updated to ToT, removed persistent GrContext and switched to
GrResourceProvider.

[bsalomon, 2015-08-03 18:08:27]

overall looks good.

https://codereview.chromium.org/1114353004/diff/480001/include/core/SkPath.h
File include/core/SkPath.h (right):

https://codereview.chromium.org/1114353004/diff/480001/include/core/SkPath.h#...
include/core/SkPath.h:1011: friend class TessellatingPathBatch; // for pathRef()
Why not just pass the listeners through the skpath to pathref? Or was this done
to avoid having listeners in the public API? Maybe the right place to add this
interface is src/core/SkPathPriv.h? That way we don't need to add a friend and
also don't need to worry about clients using the mechanism.

https://codereview.chromium.org/1114353004/diff/480001/include/gpu/GrResource...
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/1114353004/diff/480001/include/gpu/GrResource...
include/gpu/GrResourceKey.h:250: void setCustomData(const SkData* data) {
Can you modify one of the tests (or add a new test) in
tests/ResourceCacheTest.cpp to test this?

https://codereview.chromium.org/1114353004/diff/480001/include/gpu/GrResource...
include/gpu/GrResourceKey.h:258: SkAutoTUnref<const SkData> fData;
Does fData need to be public?

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1350: if (info->fTolerance == 0 ||
(info->fTolerance > 0.01f * tol && info->fTolerance < 3.0f * tol)) {
nit, col limit

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1421: int tessellate(GrUniqueKey* key,
GrResourceProvider* resourceProvider, SkAutoTUnref<GrVertexBuffer>&
vertexBuffer) {
nit, col limit

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1471:
vertexBuffer.reset(resourceProvider->createVertexBuffer(size,
wrap

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1506: SkAutoTUnref<GrVertexBuffer>
vertexBuffer(batchTarget->resourceProvider()->findAndRefTByUniqueKey<GrVertexBuffer>(key));
wrap

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1509: SkScalar tol =
GrPathUtils::scaleToleranceToSrc(screenSpaceTol, fViewMatrix,
fPath.getBounds());
wrap

[Stephen White, 2015-08-03 18:54:41]

https://codereview.chromium.org/1114353004/diff/480001/include/core/SkPath.h
File include/core/SkPath.h (right):

https://codereview.chromium.org/1114353004/diff/480001/include/core/SkPath.h#...
include/core/SkPath.h:1011: friend class TessellatingPathBatch; // for pathRef()
On 2015/08/03 18:08:26, bsalomon wrote:
> Why not just pass the listeners through the skpath to pathref? Or was this
done
> to avoid having listeners in the public API? Maybe the right place to add this
> interface is src/core/SkPathPriv.h? That way we don't need to add a friend and
> also don't need to worry about clients using the mechanism.

I didn't want to put it on the SkPath, partly for the reason you noted, and
partly because the listener really is associated with the SkPathRef, not the
SkPath: if you add it to an empty SkPath, it'll fail (since it'll be trying to
add it to the canonical empty SkPathRef). At least by requiring to go through
the SkPathRef, this is a little more clear. Not sure that's a strong argument,
though. I'll try putting it in SkPathPriv.h to see how that looks.

https://codereview.chromium.org/1114353004/diff/480001/include/gpu/GrResource...
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/1114353004/diff/480001/include/gpu/GrResource...
include/gpu/GrResourceKey.h:250: void setCustomData(const SkData* data) {
On 2015/08/03 18:08:26, bsalomon wrote:
> Can you modify one of the tests (or add a new test) in
> tests/ResourceCacheTest.cpp to test this?

Done.

https://codereview.chromium.org/1114353004/diff/480001/include/gpu/GrResource...
include/gpu/GrResourceKey.h:258: SkAutoTUnref<const SkData> fData;
On 2015/08/03 18:08:26, bsalomon wrote:
> Does fData need to be public?

Fixed.

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1350: if (info->fTolerance == 0 ||
(info->fTolerance > 0.01f * tol && info->fTolerance < 3.0f * tol)) {
On 2015/08/03 18:08:26, bsalomon wrote:
> nit, col limit

Fixed (by removing lower bound).

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1421: int tessellate(GrUniqueKey* key,
GrResourceProvider* resourceProvider, SkAutoTUnref<GrVertexBuffer>&
vertexBuffer) {
On 2015/08/03 18:08:27, bsalomon wrote:
> nit, col limit

Done.

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1471:
vertexBuffer.reset(resourceProvider->createVertexBuffer(size,
On 2015/08/03 18:08:27, bsalomon wrote:
> wrap

Done.

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1506: SkAutoTUnref<GrVertexBuffer>
vertexBuffer(batchTarget->resourceProvider()->findAndRefTByUniqueKey<GrVertexBuffer>(key));
On 2015/08/03 18:08:27, bsalomon wrote:
> wrap

Done.

https://codereview.chromium.org/1114353004/diff/480001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1509: SkScalar tol =
GrPathUtils::scaleToleranceToSrc(screenSpaceTol, fViewMatrix,
fPath.getBounds());
On 2015/08/03 18:08:27, bsalomon wrote:
> wrap

Done.

[Stephen White, 2015-08-03 18:56:59]

New patch up: PTAL. Thanks!

[Stephen White, 2015-08-03 18:58:48]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-03 18:59:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1114353004/500001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1114353004/500001

[commit-bot: I haz the power, 2015-08-03 18:59:33]

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

[commit-bot: I haz the power, 2015-08-03 18:59:34]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  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...)

[Stephen White, 2015-08-03 19:11:19]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-03 19:11:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1114353004/520001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1114353004/520001

[commit-bot: I haz the power, 2015-08-03 19:18:55]

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

[commit-bot: I haz the power, 2015-08-03 19:18:56]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2015-08-03 19:34:54]

https://codereview.chromium.org/1114353004/diff/520001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/520001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1343: bool cacheMatch(GrVertexBuffer*
vertexBuffer, SkScalar tol, int* actualCount) {
cache_match? The other functions are all hacker_named.

https://codereview.chromium.org/1114353004/diff/520001/tests/ResourceCacheTes...
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/1114353004/diff/520001/tests/ResourceCacheTes...
tests/ResourceCacheTest.cpp:1235: REPORTER_ASSERT(reporter, *(int*)
key3.getCustomData()->data() == 4132);
Can you test that if you stick a resource in the cache, unref it,  and retrieve
it by key that the custom data is correct? Just want to make sure we don't ever
lose track of the data while the resource isn't being used.

[Stephen White, 2015-08-03 19:52:24]

New patch up. PTAL. Thanks!

https://codereview.chromium.org/1114353004/diff/520001/src/gpu/GrTessellating...
File src/gpu/GrTessellatingPathRenderer.cpp (right):

https://codereview.chromium.org/1114353004/diff/520001/src/gpu/GrTessellating...
src/gpu/GrTessellatingPathRenderer.cpp:1343: bool cacheMatch(GrVertexBuffer*
vertexBuffer, SkScalar tol, int* actualCount) {
On 2015/08/03 19:34:54, bsalomon wrote:
> cache_match? The other functions are all hacker_named.

Done.

https://codereview.chromium.org/1114353004/diff/520001/tests/ResourceCacheTes...
File tests/ResourceCacheTest.cpp (right):

https://codereview.chromium.org/1114353004/diff/520001/tests/ResourceCacheTes...
tests/ResourceCacheTest.cpp:1235: REPORTER_ASSERT(reporter, *(int*)
key3.getCustomData()->data() == 4132);
On 2015/08/03 19:34:54, bsalomon wrote:
> Can you test that if you stick a resource in the cache, unref it,  and
retrieve
> it by key that the custom data is correct? Just want to make sure we don't
ever
> lose track of the data while the resource isn't being used.

Done.

[bsalomon, 2015-08-03 19:54:48]

lgtm

[Stephen White, 2015-08-03 19:56:58]

The CQ bit was checked by senorblanco@chromium.org

[commit-bot: I haz the power, 2015-08-03 19:57:03]

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

[commit-bot: I haz the power, 2015-08-03 20:04:06]

Committed patchset #26 (id:540001) as
https://skia.googlesource.com/skia/+/468dfa72eb6694145487be17876804dfca3b7adb

[Stephen White, 2015-08-04 16:54:39]

The CQ bit was checked by senorblanco@chromium.org

[Stephen White, 2015-08-04 16:54:39]

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/1114353004/#ps560001
(title: "Merge resource cache fix, leak fix, inverse winding fix")

[commit-bot: I haz the power, 2015-08-04 16:54:46]

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

[commit-bot: I haz the power, 2015-08-04 17:02:01]

Committed patchset #27 (id:560001) as
https://skia.googlesource.com/skia/+/84cd621670a357484e1674e06d3d8d6f929a4ab2