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>

src/gpu/GrProgramDesc.cpp

@@ -190,7 +190,8 @@ static bool gen_frag_proc_and_meta_keys(const GrPrimitiveProcessor& primProc,
 }
 
 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 @@ bool GrProgramDesc::Build(GrProgramDesc* desc, const GrRenderTarget* renderTarge
     // 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;
 }

src/gpu/GrProgramDesc.h

@@ -26,16 +26,16 @@ public:
 
     /**
     * 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 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 @@ public:
     }
 
     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.

src/gpu/dawn/GrDawnGpu.cpp

@@ -90,10 +90,9 @@ public:
                       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 @@ sk_sp<GrDawnProgram> GrDawnGpu::getOrCreateRenderPipeline(
         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,
+    if (!Desc::Build(&desc, rt, pipeline, primProc, primitiveType, hasDepthStencil, this)) {
-                     this)) {
         return nullptr;
     }
 

src/gpu/dawn/GrDawnGpu.h

@@ -93,7 +93,6 @@ public:
                                                    const GrPipeline&,
                                                    const GrPrimitiveProcessor&,
                                                    const GrTextureProxy* const* primProcProxies,
-                                                   bool hasPoints,
                                                    GrPrimitiveType primitiveType);
 
     dawn::Sampler getOrCreateSampler(const GrSamplerState& samplerState);

src/gpu/dawn/GrDawnOpsRenderPass.cpp

@@ -147,14 +147,12 @@ void GrDawnOpsRenderPass::applyState(const GrPipeline& pipeline,
                                           const GrTextureProxy* const primProcProxies[],
                                           const GrPipeline::FixedDynamicState* fixedDynamicState,
                                           const GrPipeline::DynamicStateArrays* dynamicStateArrays,
-                                          const GrPrimitiveType primitiveType,
+                                          const GrPrimitiveType primitiveType) {
-                                          bool hasPoints) {
     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 @@ void GrDawnOpsRenderPass::onDraw(const GrPrimitiveProcessor& primProc,
     if (!meshCount) {
         return;
     }
-    bool hasPoints = false;
+
-    for (int i = 0; i < meshCount; ++i) {
+    GrPrimitiveType primitiveType = meshes[0].primitiveType();
-        if (meshes[i].primitiveType() == GrPrimitiveType::kPoints) {
+
-            hasPoints = true;
-        }
-    }
     const GrTextureProxy* const* primProcProxies = nullptr;
     if (dynamicStateArrays && dynamicStateArrays->fPrimitiveProcessorTextures) {
         primProcProxies = dynamicStateArrays->fPrimitiveProcessorTextures;
@@ -193,8 +188,8 @@ void GrDawnOpsRenderPass::onDraw(const GrPrimitiveProcessor& primProc,
         primProcProxies = fixedDynamicState->fPrimitiveProcessorTextures;
     }
     for (int i = 0; i < meshCount; ++i) {
-        applyState(pipeline, primProc, primProcProxies, fixedDynamicState, dynamicStateArrays,
+        this->applyState(pipeline, primProc, primProcProxies, fixedDynamicState,
-                   meshes[0].primitiveType(), hasPoints);
+                         dynamicStateArrays, primitiveType);
         meshes[i].sendToGpu(this);
     }
 }

src/gpu/dawn/GrDawnOpsRenderPass.h

@@ -46,8 +46,7 @@ private:
                     const GrTextureProxy* const primProcProxies[],
                     const GrPipeline::FixedDynamicState* fixedDynamicState,
                     const GrPipeline::DynamicStateArrays* dynamicStateArrays,
-                    const GrPrimitiveType primitiveType,
+                    const GrPrimitiveType primitiveType);
-                    bool hasPoints);
 
     void onDraw(const GrPrimitiveProcessor& primProc,
                 const GrPipeline& pipeline,

src/gpu/gl/GrGLGpu.cpp

@@ -1660,10 +1660,10 @@ void GrGLGpu::disableWindowRectangles() {
 
 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 @@ void GrGLGpu::draw(GrRenderTarget* renderTarget,
 
     SkASSERT(meshCount); // guaranteed by GrOpsRenderPass::draw
 
-    bool hasPoints = false;
+    GrPrimitiveType primitiveType = meshes[0].primitiveType();
-    for (int i = 0; i < meshCount; ++i) {
+
-        if (meshes[i].primitiveType() == GrPrimitiveType::kPoints) {
+#ifdef SK_DEBUG
-            hasPoints = true;
+    // kPoints should never be intermingled in with the other primitive types
-            break;
+    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;
     }
 

src/gpu/gl/GrGLGpu.h

@@ -274,7 +274,7 @@ private:
     // 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 @@ private:
 
         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:

src/gpu/gl/GrGLGpuProgramCache.cpp

@@ -48,13 +48,11 @@ void GrGLGpu::ProgramCache::reset() {
 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;
     }

src/gpu/gl/GrGLPathRendering.cpp

@@ -115,9 +115,10 @@ void GrGLPathRendering::onDrawPath(GrRenderTarget* renderTarget,
                                    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);

src/gpu/glsl/GrGLSLVertexGeoBuilder.cpp

@@ -37,7 +37,8 @@ void GrGLSLVertexGeoBuilder::emitNormalizedSkPosition(SkString* out, const char*
 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());

src/gpu/mtl/GrMtlOpsRenderPass.mm

@@ -20,13 +20,11 @@
 #error This file must be compiled with Arc. Use -fobjc-arc flag
 #endif
 
-GrMtlOpsRenderPass::GrMtlOpsRenderPass(
+GrMtlOpsRenderPass::GrMtlOpsRenderPass(GrMtlGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin,
-        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 @@ void GrMtlOpsRenderPass::submit() {
 }
 
 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;
     }

src/gpu/mtl/GrMtlPipelineStateBuilder.mm

@@ -449,8 +449,7 @@ bool GrMtlPipelineStateBuilder::Desc::Build(Desc* desc,
                                             const GrProgramInfo& programInfo,
                                             GrPrimitiveType primitiveType,
                                             GrMtlGpu* gpu) {
-    if (!GrProgramDesc::Build(desc, renderTarget, programInfo,
+    if (!GrProgramDesc::Build(desc, renderTarget, programInfo, primitiveType, gpu)) {
-                              GrPrimitiveType::kPoints == primitiveType, gpu)) {
         return false;
     }
 

src/gpu/mtl/GrMtlResourceProvider.mm

@@ -40,8 +40,8 @@ GrMtlResourceProvider::GrMtlResourceProvider(GrMtlGpu* gpu)
 GrMtlPipelineState* GrMtlResourceProvider::findOrCreateCompatiblePipelineState(
         GrRenderTarget* renderTarget,
         const GrProgramInfo& programInfo,
-        GrPrimitiveType primType) {
+        GrPrimitiveType primitiveType) {
-    return fPipelineStateCache->refPipelineState(renderTarget, programInfo, primType);
+    return fPipelineStateCache->refPipelineState(renderTarget, programInfo, primitiveType);
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////

src/gpu/vk/GrVkPipelineStateBuilder.cpp

@@ -323,8 +323,7 @@ bool GrVkPipelineStateBuilder::Desc::Build(Desc* desc,
                                            const GrStencilSettings& stencil,
                                            GrPrimitiveType primitiveType,
                                            GrVkGpu* gpu) {
-    if (!GrProgramDesc::Build(desc, renderTarget, programInfo,
+    if (!GrProgramDesc::Build(desc, renderTarget, programInfo, primitiveType, gpu)) {
-                              primitiveType == GrPrimitiveType::kPoints, gpu)) {
         return false;
     }