Plumb GrPrimitiveType down to GrProgramDesc::Build
This defers the decision about whether pointSize is emitted until down in GrProgramDesc::Build. Although a bit wonky in GL's case, this anticipates the world where each cluster of meshes always has the same GrPrimitiveType.
Ultimately, we would like the GrPrimitiveType to be part of the GrProgramInfo and known far earlier.
Change-Id: I65566b2d77eab6a69f7af5aa60a069299b58db65
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/246018
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 af81577..9d42d69 100644
--- a/src/gpu/GrProgramDesc.cpp
+++ b/src/gpu/GrProgramDesc.cpp
@@ -190,7 +190,8 @@
}
bool GrProgramDesc::Build(GrProgramDesc* desc, const GrRenderTarget* renderTarget,
- const GrProgramInfo& programInfo, bool hasPointSize, GrGpu* gpu) {
+ const GrProgramInfo& programInfo, GrPrimitiveType primitiveType,
+ GrGpu* gpu) {
// The descriptor is used as a cache key. Thus when a field of the
// descriptor will not affect program generation (because of the attribute
// bindings in use or other descriptor field settings) it should be set
@@ -262,6 +263,6 @@
// Ensure enough bits.
SkASSERT(header->fProcessorFeatures == (int) programInfo.requestedFeatures());
header->fSnapVerticesToPixelCenters = programInfo.pipeline().snapVerticesToPixelCenters();
- header->fHasPointSize = hasPointSize ? 1 : 0;
+ header->fHasPointSize = (primitiveType == GrPrimitiveType::kPoints);
return true;
}
diff --git a/src/gpu/GrProgramDesc.h b/src/gpu/GrProgramDesc.h
index f504393..25a7895 100644
--- a/src/gpu/GrProgramDesc.h
+++ b/src/gpu/GrProgramDesc.h
@@ -26,16 +26,16 @@
/**
* Builds a program descriptor. Before the descriptor can be used, the client must call finalize
- * on the returned GrProgramDesc.
+ * on the filled in GrProgramDesc.
*
- * @param desc The built and finalized descriptor
- * @param renderTarget The target of the draw
- * @param programInfo Program information need to build the key
- * @param hasPointSize Controls whether the shader will output a point size.
- * @param gpu Pointer to the GrGpu object the program will be used with.
+ * @param desc The built and finalized descriptor
+ * @param renderTarget The target of the draw
+ * @param programInfo Program information need to build the key
+ * @param primitiveType Controls whether the shader will output a point size.
+ * @param gpu Pointer to the GrGpu object the program will be used with.
**/
static bool Build(GrProgramDesc*, const GrRenderTarget*, const GrProgramInfo&,
- bool hasPointSize, GrGpu*);
+ GrPrimitiveType, GrGpu*);
static bool BuildFromData(GrProgramDesc* desc, const void* keyData, size_t keyLength) {
if (!SkTFitsIn<int>(keyLength)) {
@@ -86,6 +86,8 @@
}
struct KeyHeader {
+ bool hasPointSize() const { return fHasPointSize; }
+
// Set to uniquely idenitify any swizzling of the shader's output color(s).
uint16_t fOutputSwizzle;
uint8_t fColorFragmentProcessorCnt; // Can be packed into 4 bits if required.
diff --git a/src/gpu/dawn/GrDawnGpu.cpp b/src/gpu/dawn/GrDawnGpu.cpp
index 1c4ee6a..e0ef6ce 100644
--- a/src/gpu/dawn/GrDawnGpu.cpp
+++ b/src/gpu/dawn/GrDawnGpu.cpp
@@ -90,10 +90,9 @@
const GrPipeline& pipeline,
const GrPrimitiveProcessor& primProc,
GrPrimitiveType primitiveType,
- bool hasPoints,
bool hasDepthStencil,
GrGpu* gpu) {
- if (!GrProgramDesc::Build(desc, rt, primProc, hasPoints, pipeline, gpu)) {
+ if (!GrProgramDesc::Build(desc, rt, primProc, primitiveType, pipeline, gpu)) {
return false;
}
GrProcessorKeyBuilder b(&desc->key());
@@ -635,12 +634,10 @@
const GrPipeline& pipeline,
const GrPrimitiveProcessor& primProc,
const GrTextureProxy* const* primProcProxies,
- bool hasPoints,
GrPrimitiveType primitiveType) {
bool hasDepthStencil = rt->renderTargetPriv().getStencilAttachment() != nullptr;
Desc desc;
- if (!Desc::Build(&desc, rt, pipeline, primProc, primitiveType, hasPoints, hasDepthStencil,
- this)) {
+ if (!Desc::Build(&desc, rt, pipeline, primProc, primitiveType, hasDepthStencil, this)) {
return nullptr;
}
diff --git a/src/gpu/dawn/GrDawnGpu.h b/src/gpu/dawn/GrDawnGpu.h
index 83b543d..7e2d8b6 100644
--- a/src/gpu/dawn/GrDawnGpu.h
+++ b/src/gpu/dawn/GrDawnGpu.h
@@ -93,7 +93,6 @@
const GrPipeline&,
const GrPrimitiveProcessor&,
const GrTextureProxy* const* primProcProxies,
- bool hasPoints,
GrPrimitiveType primitiveType);
dawn::Sampler getOrCreateSampler(const GrSamplerState& samplerState);
diff --git a/src/gpu/dawn/GrDawnOpsRenderPass.cpp b/src/gpu/dawn/GrDawnOpsRenderPass.cpp
index e8abcf1..1b74181 100644
--- a/src/gpu/dawn/GrDawnOpsRenderPass.cpp
+++ b/src/gpu/dawn/GrDawnOpsRenderPass.cpp
@@ -147,14 +147,12 @@
const GrTextureProxy* const primProcProxies[],
const GrPipeline::FixedDynamicState* fixedDynamicState,
const GrPipeline::DynamicStateArrays* dynamicStateArrays,
- const GrPrimitiveType primitiveType,
- bool hasPoints) {
+ const GrPrimitiveType primitiveType) {
sk_sp<GrDawnProgram> program = fGpu->getOrCreateRenderPipeline(fRenderTarget,
fOrigin,
pipeline,
primProc,
primProcProxies,
- hasPoints,
primitiveType);
auto bindGroup = program->setData(fGpu, fRenderTarget, fOrigin, primProc, pipeline,
primProcProxies);
@@ -180,12 +178,9 @@
if (!meshCount) {
return;
}
- bool hasPoints = false;
- for (int i = 0; i < meshCount; ++i) {
- if (meshes[i].primitiveType() == GrPrimitiveType::kPoints) {
- hasPoints = true;
- }
- }
+
+ GrPrimitiveType primitiveType = meshes[0].primitiveType();
+
const GrTextureProxy* const* primProcProxies = nullptr;
if (dynamicStateArrays && dynamicStateArrays->fPrimitiveProcessorTextures) {
primProcProxies = dynamicStateArrays->fPrimitiveProcessorTextures;
@@ -193,8 +188,8 @@
primProcProxies = fixedDynamicState->fPrimitiveProcessorTextures;
}
for (int i = 0; i < meshCount; ++i) {
- applyState(pipeline, primProc, primProcProxies, fixedDynamicState, dynamicStateArrays,
- meshes[0].primitiveType(), hasPoints);
+ this->applyState(pipeline, primProc, primProcProxies, fixedDynamicState,
+ dynamicStateArrays, primitiveType);
meshes[i].sendToGpu(this);
}
}
diff --git a/src/gpu/dawn/GrDawnOpsRenderPass.h b/src/gpu/dawn/GrDawnOpsRenderPass.h
index 70d272c..f84ff92 100644
--- a/src/gpu/dawn/GrDawnOpsRenderPass.h
+++ b/src/gpu/dawn/GrDawnOpsRenderPass.h
@@ -46,8 +46,7 @@
const GrTextureProxy* const primProcProxies[],
const GrPipeline::FixedDynamicState* fixedDynamicState,
const GrPipeline::DynamicStateArrays* dynamicStateArrays,
- const GrPrimitiveType primitiveType,
- bool hasPoints);
+ const GrPrimitiveType primitiveType);
void onDraw(const GrPrimitiveProcessor& primProc,
const GrPipeline& pipeline,
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 09897a7..c014ddb 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -1660,10 +1660,10 @@
bool GrGLGpu::flushGLState(GrRenderTarget* renderTarget,
const GrProgramInfo& programInfo,
- bool willDrawPoints) {
+ GrPrimitiveType primitiveType) {
sk_sp<GrGLProgram> program(fProgramCache->refProgram(this, renderTarget, programInfo,
- willDrawPoints));
+ primitiveType));
if (!program) {
GrCapsDebugf(this->caps(), "Failed to create program!\n");
return false;
@@ -2174,14 +2174,22 @@
SkASSERT(meshCount); // guaranteed by GrOpsRenderPass::draw
- bool hasPoints = false;
- for (int i = 0; i < meshCount; ++i) {
- if (meshes[i].primitiveType() == GrPrimitiveType::kPoints) {
- hasPoints = true;
- break;
+ GrPrimitiveType primitiveType = meshes[0].primitiveType();
+
+#ifdef SK_DEBUG
+ // kPoints should never be intermingled in with the other primitive types
+ for (int i = 1; i < meshCount; ++i) {
+ if (primitiveType == GrPrimitiveType::kPoints) {
+ SkASSERT(meshes[i].primitiveType() == GrPrimitiveType::kPoints);
+ } else {
+ SkASSERT(meshes[i].primitiveType() != GrPrimitiveType::kPoints);
}
}
- if (!this->flushGLState(renderTarget, programInfo, hasPoints)) {
+#endif
+
+ // Passing 'primitiveType' here is a bit misleading. In GL's case it works out, since
+ // GL only cares if it is kPoints or not.
+ if (!this->flushGLState(renderTarget, programInfo, primitiveType)) {
return;
}
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 8633c32..29d6e2f 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -274,7 +274,7 @@
// willDrawPoints must be true if point primitives will be rendered after setting the GL state.
// If DynamicStateArrays is not null then dynamicStateArraysLength is the number of dynamic
// state entries in each array.
- bool flushGLState(GrRenderTarget*, const GrProgramInfo&, bool willDrawPoints);
+ bool flushGLState(GrRenderTarget*, const GrProgramInfo&, GrPrimitiveType);
void flushProgram(sk_sp<GrGLProgram>);
@@ -312,7 +312,7 @@
void abandon();
void reset();
- GrGLProgram* refProgram(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&, bool hasPointSize);
+ GrGLProgram* refProgram(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&, GrPrimitiveType);
bool precompileShader(const SkData& key, const SkData& data);
private:
diff --git a/src/gpu/gl/GrGLGpuProgramCache.cpp b/src/gpu/gl/GrGLGpuProgramCache.cpp
index 3b683bf..0109741 100644
--- a/src/gpu/gl/GrGLGpuProgramCache.cpp
+++ b/src/gpu/gl/GrGLGpuProgramCache.cpp
@@ -48,13 +48,11 @@
GrGLProgram* GrGLGpu::ProgramCache::refProgram(GrGLGpu* gpu,
GrRenderTarget* renderTarget,
const GrProgramInfo& programInfo,
- bool isPoints) {
-
-
+ GrPrimitiveType primitiveType) {
// TODO: can this be unified between GL, Vk and Mtl?
// Get GrGLProgramDesc
GrProgramDesc desc;
- if (!GrProgramDesc::Build(&desc, renderTarget, programInfo, isPoints, gpu)) {
+ if (!GrProgramDesc::Build(&desc, renderTarget, programInfo, primitiveType, gpu)) {
GrCapsDebugf(gpu->caps(), "Failed to gl program descriptor!\n");
return nullptr;
}
diff --git a/src/gpu/gl/GrGLPathRendering.cpp b/src/gpu/gl/GrGLPathRendering.cpp
index 327c8d2..abcae33 100644
--- a/src/gpu/gl/GrGLPathRendering.cpp
+++ b/src/gpu/gl/GrGLPathRendering.cpp
@@ -115,9 +115,10 @@
const GrProgramInfo& programInfo,
const GrStencilSettings& stencilPassSettings,
const GrPath* path) {
- if (!this->gpu()->flushGLState(renderTarget, programInfo, false)) {
+ if (!this->gpu()->flushGLState(renderTarget, programInfo, GrPrimitiveType::kPath)) {
return;
}
+
const GrGLPath* glPath = static_cast<const GrGLPath*>(path);
this->flushPathStencilSettings(stencilPassSettings);
diff --git a/src/gpu/glsl/GrGLSLVertexGeoBuilder.cpp b/src/gpu/glsl/GrGLSLVertexGeoBuilder.cpp
index 8edd4ba..5d0980f 100644
--- a/src/gpu/glsl/GrGLSLVertexGeoBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLVertexGeoBuilder.cpp
@@ -37,7 +37,8 @@
void GrGLSLVertexBuilder::onFinalize() {
// We could have the GrGeometryProcessor do this, but its just easier to have it performed
// here. If we ever need to set variable pointsize, then we can reinvestigate.
- if (this->getProgramBuilder()->header().fHasPointSize) {
+ const GrProgramDesc::KeyHeader& header = this->getProgramBuilder()->desc()->header();
+ if (header.hasPointSize()) {
this->codeAppend("sk_PointSize = 1.0;");
}
fProgramBuilder->varyingHandler()->getVertexDecls(&this->inputs(), &this->outputs());
diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.mm b/src/gpu/mtl/GrMtlOpsRenderPass.mm
index 19050ee..f587492 100644
--- a/src/gpu/mtl/GrMtlOpsRenderPass.mm
+++ b/src/gpu/mtl/GrMtlOpsRenderPass.mm
@@ -20,13 +20,11 @@
#error This file must be compiled with Arc. Use -fobjc-arc flag
#endif
-GrMtlOpsRenderPass::GrMtlOpsRenderPass(
- GrMtlGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin,
- const GrOpsRenderPass::LoadAndStoreInfo& colorInfo,
- const GrOpsRenderPass::StencilLoadAndStoreInfo& stencilInfo)
+GrMtlOpsRenderPass::GrMtlOpsRenderPass(GrMtlGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin,
+ const GrOpsRenderPass::LoadAndStoreInfo& colorInfo,
+ const GrOpsRenderPass::StencilLoadAndStoreInfo& stencilInfo)
: INHERITED(rt, origin)
- , fGpu(gpu)
- {
+ , fGpu(gpu) {
this->setupRenderPass(colorInfo, stencilInfo);
}
@@ -54,13 +52,13 @@
}
GrMtlPipelineState* GrMtlOpsRenderPass::prepareDrawState(const GrProgramInfo& programInfo,
- GrPrimitiveType primType) {
+ GrPrimitiveType primitiveType) {
// TODO: resolve textures and regenerate mipmaps as needed
GrMtlPipelineState* pipelineState =
fGpu->resourceProvider().findOrCreateCompatiblePipelineState(fRenderTarget,
programInfo,
- primType);
+ primitiveType);
if (!pipelineState) {
return nullptr;
}
diff --git a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
index 820eed6..090c41b 100644
--- a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
+++ b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
@@ -449,8 +449,7 @@
const GrProgramInfo& programInfo,
GrPrimitiveType primitiveType,
GrMtlGpu* gpu) {
- if (!GrProgramDesc::Build(desc, renderTarget, programInfo,
- GrPrimitiveType::kPoints == primitiveType, gpu)) {
+ if (!GrProgramDesc::Build(desc, renderTarget, programInfo, primitiveType, gpu)) {
return false;
}
diff --git a/src/gpu/mtl/GrMtlResourceProvider.mm b/src/gpu/mtl/GrMtlResourceProvider.mm
index bc9b2ff..5406957 100644
--- a/src/gpu/mtl/GrMtlResourceProvider.mm
+++ b/src/gpu/mtl/GrMtlResourceProvider.mm
@@ -40,8 +40,8 @@
GrMtlPipelineState* GrMtlResourceProvider::findOrCreateCompatiblePipelineState(
GrRenderTarget* renderTarget,
const GrProgramInfo& programInfo,
- GrPrimitiveType primType) {
- return fPipelineStateCache->refPipelineState(renderTarget, programInfo, primType);
+ GrPrimitiveType primitiveType) {
+ return fPipelineStateCache->refPipelineState(renderTarget, programInfo, primitiveType);
}
////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/vk/GrVkPipelineStateBuilder.cpp b/src/gpu/vk/GrVkPipelineStateBuilder.cpp
index 3398f3f..050429d 100644
--- a/src/gpu/vk/GrVkPipelineStateBuilder.cpp
+++ b/src/gpu/vk/GrVkPipelineStateBuilder.cpp
@@ -323,8 +323,7 @@
const GrStencilSettings& stencil,
GrPrimitiveType primitiveType,
GrVkGpu* gpu) {
- if (!GrProgramDesc::Build(desc, renderTarget, programInfo,
- primitiveType == GrPrimitiveType::kPoints, gpu)) {
+ if (!GrProgramDesc::Build(desc, renderTarget, programInfo, primitiveType, gpu)) {
return false;
}