Reland "OpenGL: delete shaders and pipelines when they are not used any longer"

This is a reland of a57c1db878

in current Dawn implementation RenderPipelineGL and ComputePipelineGL
will be destroyed before calling their initialize() function when we
can find a proper pipeline object in the front-end cache, so mProgram
may not be assigned to a valid GL program ID when the destructor of
PipelineGL is called.

In this CL we always initialize mProgram to 0 in the constructor of
PipelineGL so that it won't be a garbage value in the destructor of
PipelineGL (0 is safe for glDeleteProgram() according to OpenGL SPEC).

Original change's description:
> 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>

Bug: dawn:529
Change-Id: Ie04ab069b9d26658f2b0d1b070d86bb650f3c878
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/65486
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
This commit is contained in:
Jiawei Shao 2021-10-04 12:31:22 +00:00 committed by Dawn LUCI CQ
parent 08ec67af0d
commit afe9138408
5 changed files with 23 additions and 4 deletions

View File

@ -27,6 +27,10 @@ namespace dawn_native { namespace opengl {
return pipeline; return pipeline;
} }
ComputePipeline::~ComputePipeline() {
DeleteProgram(ToBackend(GetDevice())->gl);
}
MaybeError ComputePipeline::Initialize() { MaybeError ComputePipeline::Initialize() {
DAWN_TRY( DAWN_TRY(
InitializeBase(ToBackend(GetDevice())->gl, ToBackend(GetLayout()), GetAllStages())); InitializeBase(ToBackend(GetDevice())->gl, ToBackend(GetLayout()), GetAllStages()));

View File

@ -35,7 +35,7 @@ namespace dawn_native { namespace opengl {
private: private:
using ComputePipelineBase::ComputePipelineBase; using ComputePipelineBase::ComputePipelineBase;
~ComputePipeline() override = default; ~ComputePipeline() override;
MaybeError Initialize() override; MaybeError Initialize() override;
}; };

View File

@ -45,7 +45,9 @@ namespace dawn_native { namespace opengl {
} // namespace } // namespace
PipelineGL::PipelineGL() = default; PipelineGL::PipelineGL() : mProgram(0) {
}
PipelineGL::~PipelineGL() = default; PipelineGL::~PipelineGL() = default;
MaybeError PipelineGL::InitializeBase(const OpenGLFunctions& gl, MaybeError PipelineGL::InitializeBase(const OpenGLFunctions& gl,
@ -87,6 +89,7 @@ namespace dawn_native { namespace opengl {
// Create an OpenGL shader for each stage and gather the list of combined samplers. // Create an OpenGL shader for each stage and gather the list of combined samplers.
PerStage<CombinedSamplerInfo> combinedSamplers; PerStage<CombinedSamplerInfo> combinedSamplers;
bool needsDummySampler = false; bool needsDummySampler = false;
std::vector<GLuint> glShaders;
for (SingleShaderStage stage : IterateStages(activeStages)) { for (SingleShaderStage stage : IterateStages(activeStages)) {
const ShaderModule* module = ToBackend(stages[stage].module.Get()); const ShaderModule* module = ToBackend(stages[stage].module.Get());
std::string glsl; std::string glsl;
@ -96,6 +99,7 @@ namespace dawn_native { namespace opengl {
GLuint shader; GLuint shader;
DAWN_TRY_ASSIGN(shader, CreateShader(gl, GLShaderType(stage), glsl.c_str())); DAWN_TRY_ASSIGN(shader, CreateShader(gl, GLShaderType(stage), glsl.c_str()));
gl.AttachShader(mProgram, shader); gl.AttachShader(mProgram, shader);
glShaders.push_back(shader);
} }
if (needsDummySampler) { if (needsDummySampler) {
@ -178,9 +182,19 @@ namespace dawn_native { namespace opengl {
textureUnit++; textureUnit++;
} }
for (GLuint glShader : glShaders) {
gl.DetachShader(mProgram, glShader);
gl.DeleteShader(glShader);
}
return {}; return {};
} }
void PipelineGL::DeleteProgram(const OpenGLFunctions& gl) {
gl.DeleteProgram(mProgram);
}
const std::vector<PipelineGL::SamplerUnit>& PipelineGL::GetTextureUnitsForSampler( const std::vector<PipelineGL::SamplerUnit>& PipelineGL::GetTextureUnitsForSampler(
GLuint index) const { GLuint index) const {
ASSERT(index < mUnitsForSamplers.size()); ASSERT(index < mUnitsForSamplers.size());

View File

@ -47,12 +47,12 @@ namespace dawn_native { namespace opengl {
const std::vector<GLuint>& GetTextureUnitsForTextureView(GLuint index) const; const std::vector<GLuint>& GetTextureUnitsForTextureView(GLuint index) const;
GLuint GetProgramHandle() const; GLuint GetProgramHandle() const;
void ApplyNow(const OpenGLFunctions& gl);
protected: protected:
void ApplyNow(const OpenGLFunctions& gl);
MaybeError InitializeBase(const OpenGLFunctions& gl, MaybeError InitializeBase(const OpenGLFunctions& gl,
const PipelineLayout* layout, const PipelineLayout* layout,
const PerStage<ProgrammableStage>& stages); const PerStage<ProgrammableStage>& stages);
void DeleteProgram(const OpenGLFunctions& gl);
private: private:
GLuint mProgram; GLuint mProgram;

View File

@ -238,6 +238,7 @@ namespace dawn_native { namespace opengl {
const OpenGLFunctions& gl = ToBackend(GetDevice())->gl; const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
gl.DeleteVertexArrays(1, &mVertexArrayObject); gl.DeleteVertexArrays(1, &mVertexArrayObject);
gl.BindVertexArray(0); gl.BindVertexArray(0);
DeleteProgram(gl);
} }
GLenum RenderPipeline::GetGLPrimitiveTopology() const { GLenum RenderPipeline::GetGLPrimitiveTopology() const {