Always include surface origin in keys for program/pipeline state
This was effectively happening before, but only after we (always) failed
to find an entry without the origin. If we ever have a system that can
determine ahead of time that we won't depend on surface origin, we can
update this logic. For now, just simplify the code and remove the
useless cache lookups.
Change-Id: I2b6178a8a83a48f33fdc066e185a2a4c13aa8ddc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229284
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/src/gpu/gl/GrGLGpuProgramCache.cpp b/src/gpu/gl/GrGLGpuProgramCache.cpp
index 7523292..1213132 100644
--- a/src/gpu/gl/GrGLGpuProgramCache.cpp
+++ b/src/gpu/gl/GrGLGpuProgramCache.cpp
@@ -90,13 +90,12 @@
GrCapsDebugf(gpu->caps(), "Failed to gl program descriptor!\n");
return nullptr;
}
+ // If we knew the shader won't depend on origin, we could skip this (and use the same program
+ // for both origins). Instrumenting all fragment processors would be difficult and error prone.
+ desc.setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(origin));
+
std::unique_ptr<Entry>* entry = fMap.find(desc);
if (!entry) {
- // Didn't find an origin-independent version, check with the specific origin
- desc.setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(origin));
- entry = fMap.find(desc);
- }
- if (!entry) {
// We have a cache miss
#ifdef PROGRAM_CACHE_STATS
++fCacheMisses;
diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.cpp b/src/gpu/gl/builders/GrGLProgramBuilder.cpp
index 60b4b80..25e718f 100644
--- a/src/gpu/gl/builders/GrGLProgramBuilder.cpp
+++ b/src/gpu/gl/builders/GrGLProgramBuilder.cpp
@@ -89,7 +89,6 @@
GrGLuint programId,
GrGLenum type,
SkTDArray<GrGLuint>* shaderIds,
- const SkSL::Program::Inputs& inputs,
GrContextOptions::ShaderErrorHandler* errHandler) {
GrGLGpu* gpu = this->gpu();
GrGLuint shaderId = GrGLCompileAndAttachShader(gpu->glContext(),
@@ -103,11 +102,6 @@
}
*shaderIds->append() = shaderId;
- if (inputs.fFlipY) {
- GrProgramDesc* d = this->desc();
- d->setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(this->origin()));
- }
-
return true;
}
@@ -302,8 +296,7 @@
this->computeCountsAndStrides(programID, primProc, false);
}
if (!this->compileAndAttachShaders(glsl[kFragment_GrShaderType], programID,
- GR_GL_FRAGMENT_SHADER, &shadersToDelete, inputs,
- errorHandler)) {
+ GR_GL_FRAGMENT_SHADER, &shadersToDelete, errorHandler)) {
this->cleanupProgram(programID, shadersToDelete);
return nullptr;
}
@@ -322,8 +315,7 @@
}
}
if (!this->compileAndAttachShaders(glsl[kVertex_GrShaderType], programID,
- GR_GL_VERTEX_SHADER, &shadersToDelete, inputs,
- errorHandler)) {
+ GR_GL_VERTEX_SHADER, &shadersToDelete, errorHandler)) {
this->cleanupProgram(programID, shadersToDelete);
return nullptr;
}
@@ -350,7 +342,7 @@
}
}
if (!this->compileAndAttachShaders(glsl[kGeometry_GrShaderType], programID,
- GR_GL_GEOMETRY_SHADER, &shadersToDelete, inputs,
+ GR_GL_GEOMETRY_SHADER, &shadersToDelete,
errorHandler)) {
this->cleanupProgram(programID, shadersToDelete);
return nullptr;
diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.h b/src/gpu/gl/builders/GrGLProgramBuilder.h
index 4624129..680dfd0 100644
--- a/src/gpu/gl/builders/GrGLProgramBuilder.h
+++ b/src/gpu/gl/builders/GrGLProgramBuilder.h
@@ -56,7 +56,6 @@
GrGLuint programId,
GrGLenum type,
SkTDArray<GrGLuint>* shaderIds,
- const SkSL::Program::Inputs& inputs,
GrContextOptions::ShaderErrorHandler* errorHandler);
void computeCountsAndStrides(GrGLuint programID, const GrPrimitiveProcessor& primProc,
diff --git a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
index f580161..d3c5479 100644
--- a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
+++ b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
@@ -77,9 +77,6 @@
if (inputs.fRTHeight) {
this->addRTHeightUniform(SKSL_RTHEIGHT_NAME);
}
- if (inputs.fFlipY) {
- desc->setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(this->origin()));
- }
return shaderLibrary;
}
diff --git a/src/gpu/mtl/GrMtlResourceProvider.mm b/src/gpu/mtl/GrMtlResourceProvider.mm
index 4535be8..c9ac263 100644
--- a/src/gpu/mtl/GrMtlResourceProvider.mm
+++ b/src/gpu/mtl/GrMtlResourceProvider.mm
@@ -139,14 +139,12 @@
GrCapsDebugf(fGpu->caps(), "Failed to build mtl program descriptor!\n");
return nullptr;
}
+ // If we knew the shader won't depend on origin, we could skip this (and use the same program
+ // for both origins). Instrumenting all fragment processors would be difficult and error prone.
+ desc.setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(origin));
std::unique_ptr<Entry>* entry = fMap.find(desc);
if (!entry) {
- // Didn't find an origin-independent version, check with the specific origin
- desc.setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(origin));
- entry = fMap.find(desc);
- }
- if (!entry) {
#ifdef GR_PIPELINE_STATE_CACHE_STATS
++fCacheMisses;
#endif
diff --git a/src/gpu/vk/GrVkPipelineStateBuilder.cpp b/src/gpu/vk/GrVkPipelineStateBuilder.cpp
index f01b395..34ee3af 100644
--- a/src/gpu/vk/GrVkPipelineStateBuilder.cpp
+++ b/src/gpu/vk/GrVkPipelineStateBuilder.cpp
@@ -80,10 +80,6 @@
if (outInputs->fRTHeight) {
this->addRTHeightUniform(SKSL_RTHEIGHT_NAME);
}
- if (outInputs->fFlipY) {
- desc->setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(
- this->origin()));
- }
return true;
}
diff --git a/src/gpu/vk/GrVkPipelineStateCache.cpp b/src/gpu/vk/GrVkPipelineStateCache.cpp
index a167adf..0c25c6f 100644
--- a/src/gpu/vk/GrVkPipelineStateCache.cpp
+++ b/src/gpu/vk/GrVkPipelineStateCache.cpp
@@ -100,14 +100,12 @@
GrCapsDebugf(fGpu->caps(), "Failed to build vk program descriptor!\n");
return nullptr;
}
+ // If we knew the shader won't depend on origin, we could skip this (and use the same program
+ // for both origins). Instrumenting all fragment processors would be difficult and error prone.
+ desc.setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(origin));
std::unique_ptr<Entry>* entry = fMap.find(desc);
if (!entry) {
- // Didn't find an origin-independent version, check with the specific origin
- desc.setSurfaceOriginKey(GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(origin));
- entry = fMap.find(desc);
- }
- if (!entry) {
#ifdef GR_PIPELINE_STATE_CACHE_STATS
++fCacheMisses;
#endif