Add specialized content key class for resources.

Add specialized content key class for resources.

Committed: https://skia.googlesource.com/skia/+/24db3b1c35fb935660229da164fc5ad31977387f

[bsalomon, 2015-01-21 19:54:57]

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

[bsalomon, 2015-01-21 19:54:58]

https://codereview.chromium.org/858123002/diff/60001/include/gpu/GrResourceKey.h
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/858123002/diff/60001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:18: class GrResourceKey {
The diff sucks... this is basically the old GrScratchKey made subclassable for
new subclasses GrScratchKey and GrResourceKey.

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKey.h
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:18: class GrResourceKey {
The diff sucks... this is basically the old GrScratchKey made subclassable for
new subclasses GrScratchKey and GrResourceKey.

I changed size() to have an impl that doesn't validate() because sometimes we
need the size without validating (e.g. to avoid recursion within validate()).

The builder is also subclassed in the two subclasses of GrResourceKey.

[bsalomon, 2015-01-22 16:33:54]

ping

[robertphillips, 2015-01-22 16:49:12]

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKey.h
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:54: fKey.reset(SkToInt(bytes / sizeof(uint32_t)));
// bytes is guaranteed to be a multiple of 4 ?

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:131: class GrScratchKey : public GrResourceKey {
Shouldn't this be last ?

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:135: public:
Does this need to be public ?

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:179: class GrContentKey : public GrResourceKey {
Shouldn't this be at the end ?

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:183: public:
Can this be hidden ?

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:197: 
return this->INHERITED::isValid(); ?

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:220: for (int i = 0; i < innerKeyCnt; ++i) {
Maybe move this comment up ?

https://codereview.chromium.org/858123002/diff/80001/src/effects/SkBlurMaskFi...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/858123002/diff/80001/src/effects/SkBlurMaskFi...
src/effects/SkBlurMaskFilter.cpp:755: 
profileSize ?

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrContext.cpp#ne...
src/gpu/GrContext.cpp:418: if (flags) {
Aren't you getting different values here ?

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrContext.cpp#ne...
src/gpu/GrContext.cpp:437: if (flags) {
and here ?

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrPath.cpp
File src/gpu/GrPath.cpp (right):

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrPath.cpp#newco...
src/gpu/GrPath.cpp:19: GrContentKey::Builder builder(key, kDomain, 3);
Add assert that this is tightly packed ?

[bsalomon, 2015-01-22 19:23:17]

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKey.h
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:54: fKey.reset(SkToInt(bytes / sizeof(uint32_t)));
On 2015/01/22 16:49:12, robertphillips wrote:
> // bytes is guaranteed to be a multiple of 4 ?

Added an assert to that effect.

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:131: class GrScratchKey : public GrResourceKey {
On 2015/01/22 16:49:12, robertphillips wrote:
> Shouldn't this be last ?

it breaks using it in the builder to put it last

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:135: public:
On 2015/01/22 16:49:12, robertphillips wrote:
> Does this need to be public ?

Removed it

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:183: public:
On 2015/01/22 16:49:11, robertphillips wrote:
> Can this be hidden ?

Removed it.

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:197: 
On 2015/01/22 16:49:12, robertphillips wrote:
> return this->INHERITED::isValid(); ?

Done.

https://codereview.chromium.org/858123002/diff/80001/include/gpu/GrResourceKe...
include/gpu/GrResourceKey.h:220: for (int i = 0; i < innerKeyCnt; ++i) {
On 2015/01/22 16:49:12, robertphillips wrote:
> Maybe move this comment up ?

Done.

https://codereview.chromium.org/858123002/diff/80001/src/effects/SkBlurMaskFi...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/858123002/diff/80001/src/effects/SkBlurMaskFi...
src/effects/SkBlurMaskFilter.cpp:755: 
On 2015/01/22 16:49:12, robertphillips wrote:
> profileSize ?

Done.

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrContext.cpp#ne...
src/gpu/GrContext.cpp:418: if (flags) {
On 2015/01/22 16:49:12, robertphillips wrote:
> Aren't you getting different values here ?

yikes, surely.

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrPath.cpp
File src/gpu/GrPath.cpp (right):

https://codereview.chromium.org/858123002/diff/80001/src/gpu/GrPath.cpp#newco...
src/gpu/GrPath.cpp:19: GrContentKey::Builder builder(key, kDomain, 3);
On 2015/01/22 16:49:12, robertphillips wrote:
> Add assert that this is tightly packed ?

killed it

[robertphillips, 2015-01-22 20:02:50]

lgtm

[bsalomon, 2015-01-22 20:49:53]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2015-01-22 20:50:24]

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

[commit-bot: I haz the power, 2015-01-22 20:50:32]

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

[commit-bot: I haz the power, 2015-01-22 20:50:34]

Failed to apply patch for src/gpu/GrContext.cpp:
While running git apply --index -3 -p1;
  error: patch failed: src/gpu/GrContext.cpp:325
  Falling back to three-way merge...
  Applied patch to 'src/gpu/GrContext.cpp' with conflicts.
  U src/gpu/GrContext.cpp

Patch:       src/gpu/GrContext.cpp
Index: src/gpu/GrContext.cpp
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index
785691dbba8af5e0d1dfcd848fe40f83c29aa22e..c8e1d0250d0f6aa49ddb966311d8fae2a589b04f
100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -230,26 +230,6 @@ GrTextContext* GrContext::createTextContext(GrRenderTarget*
renderTarget,
 

////////////////////////////////////////////////////////////////////////////////
 
-GrTexture* GrContext::findAndRefTexture(const GrSurfaceDesc& desc,
-                                        const GrCacheID& cacheID,
-                                        const GrTextureParams* params) {
-    GrResourceKey resourceKey = GrTexturePriv::ComputeKey(fGpu, params, desc,
cacheID);
-
-    GrGpuResource* resource = this->findAndRefCachedResource(resourceKey);
-    if (resource) {
-        SkASSERT(static_cast<GrSurface*>(resource)->asTexture());
-        return static_cast<GrSurface*>(resource)->asTexture();
-    }
-    return NULL;
-}
-
-bool GrContext::isTextureInCache(const GrSurfaceDesc& desc,
-                                 const GrCacheID& cacheID,
-                                 const GrTextureParams* params) const {
-    GrResourceKey resourceKey = GrTexturePriv::ComputeKey(fGpu, params, desc,
cacheID);
-    return fResourceCache2->hasContentKey(resourceKey);
-}
-
 static void stretch_image(void* dst,
                           int dstW,
                           int dstH,
@@ -275,20 +255,54 @@ static void stretch_image(void* dst,
     }
 }
 
+enum ResizeFlags {
+    /**
+     * The kStretchToPOT bit is set when the texture is NPOT and is being
repeated or mipped but the
+     * hardware doesn't support that feature.
+     */
+    kStretchToPOT_ResizeFlag    = 0x1,
+    /**
+     * The kBilerp bit can only be set when the kStretchToPOT flag is set and
indicates whether the
+     * stretched texture should be bilerped.
+     */
+    kBilerp_ResizeFlag          = 0x2,
+};
+
+static uint32_t get_texture_flags(const GrGpu* gpu,
+                                  const GrTextureParams* params,
+                                  const GrSurfaceDesc& desc) {
+    uint32_t flags = 0;
+    bool tiled = params && params->isTiled();
+    if (tiled && !gpu->caps()->npotTextureTileSupport()) {
+        if (!SkIsPow2(desc.fWidth) || !SkIsPow2(desc.fHeight)) {
+            flags |= kStretchToPOT_ResizeFlag;
+            switch(params->filterMode()) {
+                case GrTextureParams::kNone_FilterMode:
+                    break;
+                case GrTextureParams::kBilerp_FilterMode:
+                case GrTextureParams::kMipMap_FilterMode:
+                    flags |= kBilerp_ResizeFlag;
+                    break;
+            }
+        }
+    }
+    return flags;
+}
 // The desired texture is NPOT and tiled but that isn't supported by
 // the current hardware. Resize the texture to be a POT
 GrTexture* GrContext::createResizedTexture(const GrSurfaceDesc& desc,
-                                           const GrCacheID& cacheID,
+                                           const GrContentKey& origKey,
                                            const void* srcData,
                                            size_t rowBytes,
                                            bool filter) {
-    SkAutoTUnref<GrTexture> clampedTexture(this->findAndRefTexture(desc,
cacheID, NULL));
+    SkAutoTUnref<GrTexture> clampedTexture(this->findAndRefTexture(desc,
origKey, NULL));
     if (NULL == clampedTexture) {
-        clampedTexture.reset(this->createTexture(NULL, desc, cacheID, srcData,
rowBytes));
+        clampedTexture.reset(this->createTexture(NULL, desc, origKey, srcData,
rowBytes));
 
         if (NULL == clampedTexture) {
             return NULL;
         }
+        clampedTexture->cacheAccess().setContentKey(origKey);
     }
 
     GrSurfaceDesc rtDesc = desc;
@@ -325,6 +339,9 @@ GrTexture* GrContext::createResizedTexture(const
GrSurfaceDesc& desc,
             verts[0].setIRectFan(0, 0, texture->width(), texture->height(), 2 *
sizeof(SkPoint));
             verts[1].setIRectFan(0, 0, 1, 1, 2 * sizeof(SkPoint));
             fDrawBuffer->drawNonIndexed(&drawState, gp,
kTriangleFan_GrPrimitiveType, 0, 4);
+        } else {
+            texture->unref();
+            texture = NULL;
         }
     } else {
         // TODO: Our CPU stretch doesn't filter. But we create separate
@@ -354,30 +371,39 @@ GrTexture* GrContext::createResizedTexture(const
GrSurfaceDesc& desc,
     return texture;
 }
 
+static GrContentKey::Domain ResizeDomain() {
+    static const GrContentKey::Domain kDomain = GrContentKey::GenerateDomain();
+    return kDomain;
+}
+
 GrTexture* GrContext::createTexture(const GrTextureParams* params,
                                     const GrSurfaceDesc& desc,
-                                    const GrCacheID& cacheID,
+                                    const GrContentKey& origKey,
                                     const void* srcData,
                                     size_t rowBytes,
-                                    GrResourceKey* cacheKey) {
-    GrResourceKey resourceKey = GrTexturePriv::ComputeKey(fGpu, params, desc,
cacheID);
-
+                                    GrContentKey* outKey) {
     GrTexture* texture;
-    if (GrTexturePriv::NeedsResizing(resourceKey)) {
-        // We do not know how to resize compressed textures.
-        SkASSERT(!GrPixelConfigIsCompressed(desc.fConfig));
+    uint32_t flags = get_texture_flags(fGpu, params, desc);
+    SkTCopyOnFirstWrite<GrContentKey> key(origKey);
+    if (flags) {
+        // We don't have a code path to resize compressed textures.
+        if (GrPixelConfigIsCompressed(desc.fConfig)) {
+            return false;
+        }
+        texture = this->createResizedTexture(desc, origKey, srcData, rowBytes,
+                                             SkToBool(flags &
kBilerp_ResizeFlag));
+
+        GrContentKey::Builder builder(key.writable(), origKey, ResizeDomain(),
1);
+        builder[0] = flags;
 
-        texture = this->createResizedTexture(desc, cacheID,
-                                             srcData, rowBytes,
-                                            
GrTexturePriv::NeedsBilerp(resourceKey));
     } else {
         texture = fGpu->createTexture(desc, true, srcData, rowBytes);
     }
 
     if (texture) {
-        if (texture->cacheAccess().setContentKey(resourceKey)) {
-            if (cacheKey) {
-                *cacheKey = resourceKey;
+        if (texture->cacheAccess().setContentKey(*key)) {
+            if (outKey) {
+                *outKey = *key;
             }
         } else {
             texture->unref();
@@ -388,6 +414,37 @@ GrTexture* GrContext::createTexture(const GrTextureParams*
params,
     return texture;
 }
 
+GrTexture* GrContext::findAndRefTexture(const GrSurfaceDesc& desc,
+                                        const GrContentKey& origKey,
+                                        const GrTextureParams* params) {
+    uint32_t flags = get_texture_flags(fGpu, params, desc);
+    SkTCopyOnFirstWrite<GrContentKey> key(origKey);
+    if (flags) {
+        GrContentKey::Builder builder(key.writable(), origKey, ResizeDomain(),
1);
+        builder[0] = flags;
+    }
+
+    GrGpuResource* resource = this->findAndRefCachedResource(*key);
+    if (resource) {
+        SkASSERT(static_cast<GrSurface*>(resource)->asTexture());
+        return static_cast<GrSurface*>(resource)->asTexture();
+    }
+    return NULL;
+}
+
+bool GrContext::isTextureInCache(const GrSurfaceDesc& desc,
+                                 const GrContentKey& origKey,
+                                 const GrTextureParams* params) const {
+    uint32_t flags = get_texture_flags(fGpu, params, desc);
+    SkTCopyOnFirstWrite<GrContentKey> key(origKey);
+    if (flags) {
+        GrContentKey::Builder builder(key.writable(), origKey, ResizeDomain(),
1);
+        builder[0] = flags;
+    }
+
+    return fResourceCache2->hasContentKey(*key);
+}
+
 GrTexture* GrContext::refScratchTexture(const GrSurfaceDesc& inDesc,
ScratchTexMatch match,
                                         bool calledDuringFlush) {
     // kNoStencil has no meaning if kRT isn't set.
@@ -1716,12 +1773,12 @@ const GrFragmentProcessor*
GrContext::createUPMToPMEffect(GrTexture* texture,
     }
 }
 
-void GrContext::addResourceToCache(const GrResourceKey& resourceKey,
GrGpuResource* resource) {
-    resource->cacheAccess().setContentKey(resourceKey);
+void GrContext::addResourceToCache(const GrContentKey& key, GrGpuResource*
resource) {
+    resource->cacheAccess().setContentKey(key);
 }
 
-GrGpuResource* GrContext::findAndRefCachedResource(const GrResourceKey&
resourceKey) {
-    return fResourceCache2->findAndRefContentResource(resourceKey);
+GrGpuResource* GrContext::findAndRefCachedResource(const GrContentKey& key) {
+    return fResourceCache2->findAndRefContentResource(key);
 }
 
 void GrContext::addGpuTraceMarker(const GrGpuTraceMarker* marker) {

[bsalomon, 2015-01-22 21:25:13]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2015-01-22 21:25:59]

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

[commit-bot: I haz the power, 2015-01-22 21:27:32]

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

[commit-bot: I haz the power, 2015-01-22 21:27:33]

Try jobs failed on following builders:
  Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Android-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)

[bsalomon, 2015-01-22 21:57:25]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2015-01-22 21:57:51]

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

[commit-bot: I haz the power, 2015-01-22 22:00:12]

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

[commit-bot: I haz the power, 2015-01-22 22:00:13]

Try jobs failed on following builders:
  Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Android-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)

[bsalomon, 2015-01-22 22:49:28]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2015-01-22 22:50:20]

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

[commit-bot: I haz the power, 2015-01-23 12:24:09]

Committed patchset #10 (id:180001) as
https://skia.googlesource.com/skia/+/24db3b1c35fb935660229da164fc5ad31977387f

[mtklein, 2015-01-23 14:34:19]

On 2015/01/23 12:24:09, I haz the power (commit-bot) wrote:
> Committed patchset #10 (id:180001) as
> https://skia.googlesource.com/skia/+/24db3b1c35fb935660229da164fc5ad31977387f

Looks like this is the only GPU-related change in the blamelist for
https://code.google.com/p/skia/issues/detail?id=3340