Tighten up creation of the GrProgramDesc and remove some unnecessary checks
This takes care of some of the deferred work from:
https://skia-review.googlesource.com/c/skia/+/244118 (Add GrProgramInfo to centralize management of program information)
Bug: skia:9455
Change-Id: If1cdfc0a0614babd18f7f4bccd445e03a6dbb9ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/247301
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/GrProgramDesc.cpp b/src/gpu/GrProgramDesc.cpp
index ac7b8b9..c416e23 100644
--- a/src/gpu/GrProgramDesc.cpp
+++ b/src/gpu/GrProgramDesc.cpp
@@ -259,6 +259,10 @@
programInfo.pipeline().numCoverageFragmentProcessors()) {
return false;
}
+ // 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.
+ header->fSurfaceOriginKey =
+ GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(programInfo.origin());
header->fProcessorFeatures = (uint8_t)processorFeatures;
SkASSERT(header->processorFeatures() == processorFeatures); // Ensure enough bits.
header->fSnapVerticesToPixelCenters = programInfo.pipeline().snapVerticesToPixelCenters();
diff --git a/src/gpu/GrProgramDesc.h b/src/gpu/GrProgramDesc.h
index 75c763c..275b868 100644
--- a/src/gpu/GrProgramDesc.h
+++ b/src/gpu/GrProgramDesc.h
@@ -85,15 +85,9 @@
return !(*this == other);
}
- void setSurfaceOriginKey(int key) {
- KeyHeader* header = this->atOffset<KeyHeader, kHeaderOffset>();
- header->fSurfaceOriginKey = key;
- }
-
struct KeyHeader {
- bool hasSurfaceOriginKey() const {
- return SkToBool(fSurfaceOriginKey);
- }
+ SkDEBUGCODE(bool hasSurfaceOriginKey() const { return SkToBool(fSurfaceOriginKey); })
+
GrProcessor::CustomFeatures processorFeatures() const {
return (GrProcessor::CustomFeatures)fProcessorFeatures;
}
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index bf2b81a..713358b 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2172,9 +2172,7 @@
int meshCount) {
this->handleDirtyContext();
- if (meshCount == 0) {
- return;
- }
+ SkASSERT(meshCount); // guaranteed by GrOpsRenderPass::draw
bool hasPoints = false;
for (int i = 0; i < meshCount; ++i) {
diff --git a/src/gpu/gl/GrGLGpuProgramCache.cpp b/src/gpu/gl/GrGLGpuProgramCache.cpp
index 61ac321..3b683bf 100644
--- a/src/gpu/gl/GrGLGpuProgramCache.cpp
+++ b/src/gpu/gl/GrGLGpuProgramCache.cpp
@@ -51,17 +51,13 @@
bool isPoints) {
- // TODO: can this be unified between GL and Vk?
+ // TODO: can this be unified between GL, Vk and Mtl?
// Get GrGLProgramDesc
GrProgramDesc desc;
if (!GrProgramDesc::Build(&desc, renderTarget, programInfo, isPoints, gpu)) {
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(programInfo.origin()));
std::unique_ptr<Entry>* entry = fMap.find(desc);
if (entry && !(*entry)->fProgram) {
diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.mm b/src/gpu/mtl/GrMtlOpsRenderPass.mm
index 13d7fc8..19050ee 100644
--- a/src/gpu/mtl/GrMtlOpsRenderPass.mm
+++ b/src/gpu/mtl/GrMtlOpsRenderPass.mm
@@ -75,9 +75,8 @@
const GrMesh meshes[],
int meshCount,
const SkRect& bounds) {
- if (!meshCount) {
- return;
- }
+
+ SkASSERT(meshCount); // guaranteed by GrOpsRenderPass::draw
GrPrimitiveType primitiveType = meshes[0].primitiveType();
GrMtlPipelineState* pipelineState = this->prepareDrawState(programInfo, primitiveType);
diff --git a/src/gpu/mtl/GrMtlResourceProvider.mm b/src/gpu/mtl/GrMtlResourceProvider.mm
index 1074622..bc9b2ff 100644
--- a/src/gpu/mtl/GrMtlResourceProvider.mm
+++ b/src/gpu/mtl/GrMtlResourceProvider.mm
@@ -149,10 +149,6 @@
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(programInfo.origin()));
std::unique_ptr<Entry>* entry = fMap.find(desc);
if (!entry) {
diff --git a/src/gpu/vk/GrVkOpsRenderPass.cpp b/src/gpu/vk/GrVkOpsRenderPass.cpp
index bd2c48e..864d82e 100644
--- a/src/gpu/vk/GrVkOpsRenderPass.cpp
+++ b/src/gpu/vk/GrVkOpsRenderPass.cpp
@@ -494,9 +494,8 @@
void GrVkOpsRenderPass::onDraw(const GrProgramInfo& programInfo,
const GrMesh meshes[], int meshCount,
const SkRect& bounds) {
- if (!meshCount) {
- return;
- }
+
+ SkASSERT(meshCount); // guaranteed by GrOpsRenderPass::draw
#ifdef SK_DEBUG
if (programInfo.hasDynamicPrimProcTextures()) {
diff --git a/src/gpu/vk/GrVkPipelineStateCache.cpp b/src/gpu/vk/GrVkPipelineStateCache.cpp
index fb4cfa8a..da6a69f 100644
--- a/src/gpu/vk/GrVkPipelineStateCache.cpp
+++ b/src/gpu/vk/GrVkPipelineStateCache.cpp
@@ -93,7 +93,7 @@
renderTarget->renderTargetPriv().numStencilBits());
}
- // TODO: can this be unified between Vulkan and GL?
+ // TODO: can this be unified between GL, Vk and Mtl?
// Get GrVkProgramDesc
GrVkPipelineStateBuilder::Desc desc;
if (!GrVkPipelineStateBuilder::Desc::Build(&desc, renderTarget, programInfo, stencil,
@@ -101,10 +101,6 @@
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(programInfo.origin()));
std::unique_ptr<Entry>* entry = fMap.find(desc);
if (!entry) {