From d3e7130dc9035acea23a767df136968cce0f7879 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Thu, 6 Dec 2018 11:17:35 -0500 Subject: [PATCH] Add a hash of all vertex attributes to the GrProgramDesc key Recent work to dynamically switch the CPU type of the color attribute exposed this problem - we were ignoring the attribute types, and getting cache hits on incorrect GrProgram objects with stale layout information. This fixes GMs in configs that may use F16 colors (eg glenarrow). Bug: skia: Change-Id: Ic87c47601c26e53c2173713a6cf14d209af8e246 Reviewed-on: https://skia-review.googlesource.com/c/175243 Reviewed-by: Brian Salomon Commit-Queue: Brian Osman --- include/private/GrTypesPriv.h | 3 +++ src/gpu/GrPrimitiveProcessor.h | 18 ++++++++++++++++++ src/gpu/GrProgramDesc.cpp | 1 + 3 files changed, 22 insertions(+) diff --git a/include/private/GrTypesPriv.h b/include/private/GrTypesPriv.h index ec0a6bf29f..828ff77ef2 100644 --- a/include/private/GrTypesPriv.h +++ b/include/private/GrTypesPriv.h @@ -350,7 +350,10 @@ enum GrSLType { kTexture2DSampler_GrSLType, kTextureExternalSampler_GrSLType, kTexture2DRectSampler_GrSLType, + + kLast_GrSLType = kTexture2DRectSampler_GrSLType }; +static const int kGrSLTypeCount = kLast_GrSLType + 1; /** * The type of texture. Backends other than GL currently only use the 2D value but the type must diff --git a/src/gpu/GrPrimitiveProcessor.h b/src/gpu/GrPrimitiveProcessor.h index be3d86ab5c..5d7f0359d9 100644 --- a/src/gpu/GrPrimitiveProcessor.h +++ b/src/gpu/GrPrimitiveProcessor.h @@ -128,6 +128,7 @@ public: void init(const Attribute* attrs, int count) { fAttributes = attrs; + fRawCount = count; fCount = 0; fStride = 0; for (int i = 0; i < count; ++i) { @@ -139,6 +140,7 @@ public: } const Attribute* fAttributes = nullptr; + int fRawCount = 0; int fCount = 0; size_t fStride = 0; }; @@ -185,6 +187,22 @@ public: virtual void getGLSLProcessorKey(const GrShaderCaps&, GrProcessorKeyBuilder*) const = 0; + void getAttributeKey(GrProcessorKeyBuilder* b) const { + // Ensure that our CPU and GPU type fields fit together in a 32-bit value, and we never + // collide with the "uninitialized" value. + static_assert(kGrVertexAttribTypeCount < (1 << 8), ""); + static_assert(kGrSLTypeCount < (1 << 8), ""); + + auto add_attributes = [=](const Attribute* attrs, int attrCount) { + for (int i = 0; i < attrCount; ++i) { + b->add32(attrs[i].isInitialized() ? (attrs[i].cpuType() << 16) | attrs[i].gpuType() + : ~0); + } + }; + add_attributes(fVertexAttributes.fAttributes, fVertexAttributes.fRawCount); + add_attributes(fInstanceAttributes.fAttributes, fInstanceAttributes.fRawCount); + } + /** Returns a new instance of the appropriate *GL* implementation class for the given GrProcessor; caller is responsible for deleting the object. */ diff --git a/src/gpu/GrProgramDesc.cpp b/src/gpu/GrProgramDesc.cpp index aa84c98d78..d888a633f3 100644 --- a/src/gpu/GrProgramDesc.cpp +++ b/src/gpu/GrProgramDesc.cpp @@ -212,6 +212,7 @@ bool GrProgramDesc::Build(GrProgramDesc* desc, GrProcessorKeyBuilder b(&desc->key()); primProc.getGLSLProcessorKey(shaderCaps, &b); + primProc.getAttributeKey(&b); if (!gen_meta_key(primProc, shaderCaps, 0, &b)) { desc->key().reset(); return false;