Cleanup program building a bit

This CL:
   now passes the GrProgramDesc as a const&
   returns GrGLProgram as an sk_sp
   makes the parameter ordering more consistent
   makes GrVkPipelineState no longer ref-counted

This is pulled out of the DDL pre-compile CL which touches this portion of the code.

Bug: skia:9455
Change-Id: Id4d06f93450e276de5a2662be330ae9523026244
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268777
Reviewed-by: Greg Daniel <[email protected]>
Commit-Queue: Robert Phillips <[email protected]>

Author: rphilli

src/gpu/gl/GrGLGpu.cpp

@@ -1817,7 +1817,7 @@ void GrGLGpu::disableWindowRectangles() {
 
 bool GrGLGpu::flushGLState(GrRenderTarget* renderTarget, const GrProgramInfo& programInfo) {
 
-    sk_sp<GrGLProgram> program(fProgramCache->refProgram(this, renderTarget, programInfo));
+    sk_sp<GrGLProgram> program(fProgramCache->findOrCreateProgram(renderTarget, programInfo));
     if (!program) {
         GrCapsDebugf(this->caps(), "Failed to create program!\n");
         return false;

src/gpu/gl/GrGLGpu.h

@@ -316,7 +316,7 @@ class GrGLGpu final : public GrGpu, private GrMesh::SendToGpuImpl {
 
         void abandon();
         void reset();
-        GrGLProgram* refProgram(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&);
+        sk_sp<GrGLProgram> findOrCreateProgram(GrRenderTarget*, const GrProgramInfo&);
         bool precompileShader(const SkData& key, const SkData& data);
 
     private:

src/gpu/gl/GrGLGpuProgramCache.cpp

@@ -45,14 +45,13 @@ void GrGLGpu::ProgramCache::reset() {
     fMap.reset();
 }
 
-GrGLProgram* GrGLGpu::ProgramCache::refProgram(GrGLGpu* gpu,
-                                               GrRenderTarget* renderTarget,
-                                               const GrProgramInfo& programInfo) {
-    const GrCaps& caps = *gpu->caps();
+sk_sp<GrGLProgram> GrGLGpu::ProgramCache::findOrCreateProgram(GrRenderTarget* renderTarget,
+                                                              const GrProgramInfo& programInfo) {
+    const GrCaps& caps = *fGpu->caps();
 
     GrProgramDesc desc = caps.makeDesc(renderTarget, programInfo);
     if (!desc.isValid()) {
-        GrCapsDebugf(gpu->caps(), "Failed to gl program descriptor!\n");
+        GrCapsDebugf(fGpu->caps(), "Failed to gl program descriptor!\n");
         return nullptr;
     }
 
@@ -61,26 +60,24 @@ GrGLProgram* GrGLGpu::ProgramCache::refProgram(GrGLGpu* gpu,
         // We've pre-compiled the GL program, but don't have the GrGLProgram scaffolding
         const GrGLPrecompiledProgram* precompiledProgram = &((*entry)->fPrecompiledProgram);
         SkASSERT(precompiledProgram->fProgramID != 0);
-        GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
-                                                                 &desc, fGpu,
-                                                                 precompiledProgram);
-        if (nullptr == program) {
+        (*entry)->fProgram = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget, desc,
+                                                               programInfo, precompiledProgram);
+        if (!(*entry)->fProgram) {
             // Should we purge the program ID from the cache at this point?
             SkDEBUGFAIL("Couldn't create program from precompiled program");
             return nullptr;
         }
-        (*entry)->fProgram.reset(program);
     } else if (!entry) {
         // We have a cache miss
-        GrGLProgram* program = GrGLProgramBuilder::CreateProgram(renderTarget, programInfo,
-                                                                 &desc, fGpu);
-        if (nullptr == program) {
+        sk_sp<GrGLProgram> program = GrGLProgramBuilder::CreateProgram(fGpu, renderTarget,
+                                                                       desc, programInfo);
+        if (!program) {
             return nullptr;
         }
-        entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(sk_sp<GrGLProgram>(program))));
+        entry = fMap.insert(desc, std::unique_ptr<Entry>(new Entry(std::move(program))));
     }
 
-    return SkRef((*entry)->fProgram.get());
+    return (*entry)->fProgram;
 }
 
 bool GrGLGpu::ProgramCache::precompileShader(const SkData& key, const SkData& data) {

src/gpu/gl/builders/GrGLProgramBuilder.cpp

@@ -45,21 +45,22 @@ static void cleanup_program(GrGLGpu* gpu, GrGLuint programID,
     cleanup_shaders(gpu, shaderIDs);
 }
 
-GrGLProgram* GrGLProgramBuilder::CreateProgram(GrRenderTarget* renderTarget,
-                                               const GrProgramInfo& programInfo,
-                                               GrProgramDesc* desc,
+sk_sp<GrGLProgram> GrGLProgramBuilder::CreateProgram(
                                                GrGLGpu* gpu,
+                                               GrRenderTarget* renderTarget,
+                                               const GrProgramDesc& desc,
+                                               const GrProgramInfo& programInfo,
                                                const GrGLPrecompiledProgram* precompiledProgram) {
     ATRACE_ANDROID_FRAMEWORK_ALWAYS("shader_compile");
     GrAutoLocaleSetter als("C");
 
     // create a builder.  This will be handed off to effects so they can use it to add
     // uniforms, varyings, textures, etc
-    GrGLProgramBuilder builder(gpu, renderTarget, programInfo, desc);
+    GrGLProgramBuilder builder(gpu, renderTarget, desc, programInfo);
 
     auto persistentCache = gpu->getContext()->priv().getPersistentCache();
     if (persistentCache && !precompiledProgram) {
-        sk_sp<SkData> key = SkData::MakeWithoutCopy(desc->asKey(), desc->keyLength());
+        sk_sp<SkData> key = SkData::MakeWithoutCopy(desc.asKey(), desc.keyLength());
         builder.fCached = persistentCache->load(*key);
         // the eventual end goal is to completely skip emitAndInstallProcs on a cache hit, but it's
         // doing necessary setup in addition to generating the SkSL code. Currently we are only able
@@ -75,9 +76,9 @@ GrGLProgram* GrGLProgramBuilder::CreateProgram(GrRenderTarget* renderTarget,
 
 GrGLProgramBuilder::GrGLProgramBuilder(GrGLGpu* gpu,
                                        GrRenderTarget* renderTarget,
-                                       const GrProgramInfo& programInfo,
-                                       GrProgramDesc* desc)
-        : INHERITED(renderTarget, programInfo, desc)
+                                       const GrProgramDesc& desc,
+                                       const GrProgramInfo& programInfo)
+        : INHERITED(renderTarget, desc, programInfo)
         , fGpu(gpu)
         , fVaryingHandler(this)
         , fUniformHandler(this)
@@ -159,7 +160,7 @@ void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs,
     if (!this->gpu()->getContext()->priv().getPersistentCache()) {
         return;
     }
-    sk_sp<SkData> key = SkData::MakeWithoutCopy(this->desc()->asKey(), this->desc()->keyLength());
+    sk_sp<SkData> key = SkData::MakeWithoutCopy(this->desc().asKey(), this->desc().keyLength());
     if (fGpu->glCaps().programBinarySupport()) {
         // binary cache
         GrGLsizei length = 0;
@@ -198,7 +199,7 @@ void GrGLProgramBuilder::storeShaderInCache(const SkSL::Program::Inputs& inputs,
     }
 }
 
-GrGLProgram* GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
+sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* precompiledProgram) {
     TRACE_EVENT0("skia.gpu", TRACE_FUNC);
 
     // verify we can get a program id
@@ -541,22 +542,22 @@ void GrGLProgramBuilder::resolveProgramResourceLocations(GrGLuint programID, boo
     }
 }
 
-GrGLProgram* GrGLProgramBuilder::createProgram(GrGLuint programID) {
-    return new GrGLProgram(fGpu,
-                           fUniformHandles,
-                           programID,
-                           fUniformHandler.fUniforms,
-                           fUniformHandler.fSamplers,
-                           fVaryingHandler.fPathProcVaryingInfos,
-                           std::move(fGeometryProcessor),
-                           std::move(fXferProcessor),
-                           std::move(fFragmentProcessors),
-                           fFragmentProcessorCnt,
-                           std::move(fAttributes),
-                           fVertexAttributeCnt,
-                           fInstanceAttributeCnt,
-                           fVertexStride,
-                           fInstanceStride);
+sk_sp<GrGLProgram> GrGLProgramBuilder::createProgram(GrGLuint programID) {
+    return sk_sp<GrGLProgram>(new GrGLProgram(fGpu,
+                                              fUniformHandles,
+                                              programID,
+                                              fUniformHandler.fUniforms,
+                                              fUniformHandler.fSamplers,
+                                              fVaryingHandler.fPathProcVaryingInfos,
+                                              std::move(fGeometryProcessor),
+                                              std::move(fXferProcessor),
+                                              std::move(fFragmentProcessors),
+                                              fFragmentProcessorCnt,
+                                              std::move(fAttributes),
+                                              fVertexAttributeCnt,
+                                              fInstanceAttributeCnt,
+                                              fVertexStride,
+                                              fInstanceStride));
 }
 
 bool GrGLProgramBuilder::PrecompileProgram(GrGLPrecompiledProgram* precompiledProgram,

src/gpu/gl/builders/GrGLProgramBuilder.h

@@ -40,18 +40,15 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
      * The program implements what is specified in the stages given as input.
      * After successful generation, the builder result objects are available
      * to be used.
-     * This function may modify the GrProgramDesc by setting the surface origin
-     * key to 0 (unspecified) if it turns out the program does not care about
-     * the surface origin.
      * If a GL program has already been created, the program ID and inputs can
      * be supplied to skip the shader compilation.
-     * @return true if generation was successful.
+     * @return the created program if generation was successful.
      */
-    static GrGLProgram* CreateProgram(GrRenderTarget*,
-                                      const GrProgramInfo&,
-                                      GrProgramDesc*,
-                                      GrGLGpu*,
-                                      const GrGLPrecompiledProgram* = nullptr);
+    static sk_sp<GrGLProgram> CreateProgram(GrGLGpu*,
+                                            GrRenderTarget*,
+                                            const GrProgramDesc&,
+                                            const GrProgramInfo&,
+                                            const GrGLPrecompiledProgram* = nullptr);
 
     static bool PrecompileProgram(GrGLPrecompiledProgram*, GrGLGpu*, const SkData&);
 
@@ -60,7 +57,7 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
     GrGLGpu* gpu() const { return fGpu; }
 
 private:
-    GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);
+    GrGLProgramBuilder(GrGLGpu*, GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
 
     void addInputVars(const SkSL::Program::Inputs& inputs);
     bool compileAndAttachShaders(const SkSL::String& glsl,
@@ -74,14 +71,14 @@ class GrGLProgramBuilder : public GrGLSLProgramBuilder {
     void storeShaderInCache(const SkSL::Program::Inputs& inputs, GrGLuint programID,
                             const SkSL::String shaders[], bool isSkSL,
                             SkSL::Program::Settings* settings);
-    GrGLProgram* finalize(const GrGLPrecompiledProgram*);
+    sk_sp<GrGLProgram> finalize(const GrGLPrecompiledProgram*);
     void bindProgramResourceLocations(GrGLuint programID);
     bool checkLinkStatus(GrGLuint programID, GrContextOptions::ShaderErrorHandler* errorHandler,
                          SkSL::String* sksl[], const SkSL::String glsl[]);
     void resolveProgramResourceLocations(GrGLuint programID, bool force);
 
     // Subclasses create different programs
-    GrGLProgram* createProgram(GrGLuint programID);
+    sk_sp<GrGLProgram> createProgram(GrGLuint programID);
 
     GrGLSLUniformHandler* uniformHandler() override { return &fUniformHandler; }
     const GrGLSLUniformHandler* uniformHandler() const override { return &fUniformHandler; }

src/gpu/glsl/GrGLSLProgramBuilder.cpp

@@ -21,15 +21,15 @@
 const int GrGLSLProgramBuilder::kVarsPerBlock = 8;
 
 GrGLSLProgramBuilder::GrGLSLProgramBuilder(GrRenderTarget* renderTarget,
-                                           const GrProgramInfo& programInfo,
-                                           const GrProgramDesc* desc)
+                                           const GrProgramDesc& desc,
+                                           const GrProgramInfo& programInfo)
         : fVS(this)
         , fGS(this)
         , fFS(this)
         , fStageIndex(-1)
         , fRenderTarget(renderTarget)
-        , fProgramInfo(programInfo)
         , fDesc(desc)
+        , fProgramInfo(programInfo)
         , fGeometryProcessor(nullptr)
         , fXferProcessor(nullptr)
         , fNumFragmentSamplers(0) {}

src/gpu/glsl/GrGLSLProgramBuilder.h

@@ -57,7 +57,7 @@ class GrGLSLProgramBuilder {
         return fRenderTarget->renderTargetPriv().getSampleLocations();
     }
 
-    const GrProgramDesc* desc() const { return fDesc; }
+    const GrProgramDesc& desc() const { return fDesc; }
 
     void appendUniformDecls(GrShaderFlags visibility, SkString*) const;
 
@@ -104,10 +104,9 @@ class GrGLSLProgramBuilder {
     int fStageIndex;
 
     const GrRenderTarget*        fRenderTarget; // TODO: remove this
+    const GrProgramDesc&         fDesc;
     const GrProgramInfo&         fProgramInfo;
 
-    const GrProgramDesc*         fDesc;
-
     GrGLSLBuiltinUniformHandles  fUniformHandles;
 
     std::unique_ptr<GrGLSLPrimitiveProcessor> fGeometryProcessor;
@@ -116,7 +115,7 @@ class GrGLSLProgramBuilder {
     int fFragmentProcessorCnt;
 
 protected:
-    explicit GrGLSLProgramBuilder(GrRenderTarget*, const GrProgramInfo&, const GrProgramDesc*);
+    explicit GrGLSLProgramBuilder(GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
 
     void addFeature(GrShaderFlags shaders, uint32_t featureBit, const char* extensionName);
 

src/gpu/mtl/GrMtlPipelineStateBuilder.h

@@ -29,19 +29,19 @@ class GrMtlPipelineStateBuilder : public GrGLSLProgramBuilder {
      *
      * The GrMtlPipelineState implements what is specified in the GrPipeline and
      * GrPrimitiveProcessor as input. After successful generation, the builder result objects are
-     * available to be used. This function may modify the program key by setting the surface origin
-     * key to 0 (unspecified) if it turns out the program does not care about the surface origin.
-     * @return true if generation was successful.
+     * available to be used.
+     * @return the created pipeline if generation was successful; nullptr otherwise
      */
     static GrMtlPipelineState* CreatePipelineState(GrMtlGpu*,
                                                    GrRenderTarget*,
-                                                   const GrProgramInfo&,
-                                                   GrProgramDesc*);
+                                                   const GrProgramDesc&,
+                                                   const GrProgramInfo&);
 
 private:
-    GrMtlPipelineStateBuilder(GrMtlGpu*, GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);
+    GrMtlPipelineStateBuilder(GrMtlGpu*, GrRenderTarget*,
+                              const GrProgramDesc&, const GrProgramInfo&);
 
-    GrMtlPipelineState* finalize(GrRenderTarget*, const GrProgramInfo&, GrProgramDesc*);
+    GrMtlPipelineState* finalize(GrRenderTarget*, const GrProgramDesc&, const GrProgramInfo&);
 
     const GrCaps* caps() const override;
 
@@ -52,7 +52,6 @@ class GrMtlPipelineStateBuilder : public GrGLSLProgramBuilder {
     id<MTLLibrary> generateMtlShaderLibrary(const SkSL::String& sksl,
                                             SkSL::Program::Kind kind,
                                             const SkSL::Program::Settings& settings,
-                                            GrProgramDesc* desc,
                                             SkSL::String* msl,
                                             SkSL::Program::Inputs* inputs);
     id<MTLLibrary> compileMtlShaderLibrary(const SkSL::String& shader,