From a57c1db878e2f8a7eb352c8f253db5488cdca6f6 Mon Sep 17 00:00:00 2001
From: Jiawei Shao <jiawei.shao@intel.com>
Date: Wed, 29 Sep 2021 00:48:22 +0000
Subject: [PATCH] OpenGL: delete shaders and pipelines when they are not used
 any longer

Previously on OpenGL backend the GL pipelines and shaders are never
deleted. With this patch the GL pipelines and shaders will be able to
be destroyed correctly after they are not needed any longer.

BUG=dawn:529

Change-Id: I4f7f22c7b536825363fe1ecc0f5ffd1bb86fd774
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/65140
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
---
 src/dawn_native/opengl/ComputePipelineGL.cpp |  4 ++++
 src/dawn_native/opengl/ComputePipelineGL.h   |  2 +-
 src/dawn_native/opengl/PipelineGL.cpp        | 12 ++++++++++++
 src/dawn_native/opengl/PipelineGL.h          |  4 ++--
 src/dawn_native/opengl/RenderPipelineGL.cpp  |  1 +
 5 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/dawn_native/opengl/ComputePipelineGL.cpp b/src/dawn_native/opengl/ComputePipelineGL.cpp
index fb333444b3..e51e217a8f 100644
--- a/src/dawn_native/opengl/ComputePipelineGL.cpp
+++ b/src/dawn_native/opengl/ComputePipelineGL.cpp
@@ -27,6 +27,10 @@ namespace dawn_native { namespace opengl {
         return pipeline;
     }
 
+    ComputePipeline::~ComputePipeline() {
+        DeleteProgram(ToBackend(GetDevice())->gl);
+    }
+
     MaybeError ComputePipeline::Initialize() {
         DAWN_TRY(
             InitializeBase(ToBackend(GetDevice())->gl, ToBackend(GetLayout()), GetAllStages()));
diff --git a/src/dawn_native/opengl/ComputePipelineGL.h b/src/dawn_native/opengl/ComputePipelineGL.h
index 8e7744060b..dd5c0b3f36 100644
--- a/src/dawn_native/opengl/ComputePipelineGL.h
+++ b/src/dawn_native/opengl/ComputePipelineGL.h
@@ -35,7 +35,7 @@ namespace dawn_native { namespace opengl {
 
       private:
         using ComputePipelineBase::ComputePipelineBase;
-        ~ComputePipeline() override = default;
+        ~ComputePipeline() override;
         MaybeError Initialize() override;
     };
 
diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp
index 8ec9d61c82..83b0e81147 100644
--- a/src/dawn_native/opengl/PipelineGL.cpp
+++ b/src/dawn_native/opengl/PipelineGL.cpp
@@ -87,6 +87,7 @@ namespace dawn_native { namespace opengl {
         // Create an OpenGL shader for each stage and gather the list of combined samplers.
         PerStage<CombinedSamplerInfo> combinedSamplers;
         bool needsDummySampler = false;
+        std::vector<GLuint> glShaders;
         for (SingleShaderStage stage : IterateStages(activeStages)) {
             const ShaderModule* module = ToBackend(stages[stage].module.Get());
             std::string glsl;
@@ -96,6 +97,7 @@ namespace dawn_native { namespace opengl {
             GLuint shader;
             DAWN_TRY_ASSIGN(shader, CreateShader(gl, GLShaderType(stage), glsl.c_str()));
             gl.AttachShader(mProgram, shader);
+            glShaders.push_back(shader);
         }
 
         if (needsDummySampler) {
@@ -178,9 +180,19 @@ namespace dawn_native { namespace opengl {
 
             textureUnit++;
         }
+
+        for (GLuint glShader : glShaders) {
+            gl.DetachShader(mProgram, glShader);
+            gl.DeleteShader(glShader);
+        }
+
         return {};
     }
 
+    void PipelineGL::DeleteProgram(const OpenGLFunctions& gl) {
+        gl.DeleteProgram(mProgram);
+    }
+
     const std::vector<PipelineGL::SamplerUnit>& PipelineGL::GetTextureUnitsForSampler(
         GLuint index) const {
         ASSERT(index < mUnitsForSamplers.size());
diff --git a/src/dawn_native/opengl/PipelineGL.h b/src/dawn_native/opengl/PipelineGL.h
index e21060696a..be6c1dd553 100644
--- a/src/dawn_native/opengl/PipelineGL.h
+++ b/src/dawn_native/opengl/PipelineGL.h
@@ -47,12 +47,12 @@ namespace dawn_native { namespace opengl {
         const std::vector<GLuint>& GetTextureUnitsForTextureView(GLuint index) const;
         GLuint GetProgramHandle() const;
 
-        void ApplyNow(const OpenGLFunctions& gl);
-
       protected:
+        void ApplyNow(const OpenGLFunctions& gl);
         MaybeError InitializeBase(const OpenGLFunctions& gl,
                                   const PipelineLayout* layout,
                                   const PerStage<ProgrammableStage>& stages);
+        void DeleteProgram(const OpenGLFunctions& gl);
 
       private:
         GLuint mProgram;
diff --git a/src/dawn_native/opengl/RenderPipelineGL.cpp b/src/dawn_native/opengl/RenderPipelineGL.cpp
index d5d84096e4..5781a3e76b 100644
--- a/src/dawn_native/opengl/RenderPipelineGL.cpp
+++ b/src/dawn_native/opengl/RenderPipelineGL.cpp
@@ -240,6 +240,7 @@ namespace dawn_native { namespace opengl {
         const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
         gl.DeleteVertexArrays(1, &mVertexArrayObject);
         gl.BindVertexArray(0);
+        DeleteProgram(gl);
     }
 
     GLenum RenderPipeline::GetGLPrimitiveTopology() const {