Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(297)

Unified Diff: src/gpu/gl/GrGLShaderBuilder.cpp

Issue 164973002: Defer deletion of our shaders until after linking the gl program to work around an Android emulator… (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/gpu/gl/GrGLShaderBuilder.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/gpu/gl/GrGLShaderBuilder.cpp
diff --git a/src/gpu/gl/GrGLShaderBuilder.cpp b/src/gpu/gl/GrGLShaderBuilder.cpp
index 89f4cbf4a628f1a47ac5debe730d05c72b015a88..264bc3b0050eb8675b310c9ad2f040246475daf6 100644
--- a/src/gpu/gl/GrGLShaderBuilder.cpp
+++ b/src/gpu/gl/GrGLShaderBuilder.cpp
@@ -572,7 +572,6 @@ const char* GrGLShaderBuilder::enableSecondaryOutput() {
return dual_source_output_name();
}
-
bool GrGLShaderBuilder::finish(GrGLuint* outProgramId) {
SK_TRACE_EVENT0("GrGLShaderBuilder::finish");
@@ -582,7 +581,9 @@ bool GrGLShaderBuilder::finish(GrGLuint* outProgramId) {
return false;
}
- if (!this->compileAndAttachShaders(programId)) {
+ SkTDArray<GrGLuint> shadersToDelete;
+
+ if (!this->compileAndAttachShaders(programId, &shadersToDelete)) {
GL_CALL(DeleteProgram(programId));
return false;
}
@@ -625,22 +626,27 @@ bool GrGLShaderBuilder::finish(GrGLuint* outProgramId) {
if (!fUniformManager.isUsingBindUniform()) {
fUniformManager.getUniformLocations(programId, fUniforms);
}
+
+ for (int i = 0; i < shadersToDelete.count(); ++i) {
+ GL_CALL(DeleteShader(shadersToDelete[i]));
+ }
+
*outProgramId = programId;
return true;
}
-// Compiles a GL shader, attaches it to a program, and releases the shader's reference.
-// (That way there's no need to hang on to the GL shader id and delete it later.)
-static bool attach_shader(const GrGLContext& glCtx,
- GrGLuint programId,
- GrGLenum type,
- const SkString& shaderSrc) {
+// Compiles a GL shader and attaches it to a program. Returns the shader ID if
+// successful, or 0 if not.
+static GrGLuint attach_shader(const GrGLContext& glCtx,
+ GrGLuint programId,
+ GrGLenum type,
+ const SkString& shaderSrc) {
const GrGLInterface* gli = glCtx.interface();
GrGLuint shaderId;
GR_GL_CALL_RET(gli, shaderId, CreateShader(type));
if (0 == shaderId) {
- return false;
+ return 0;
}
const GrGLchar* sourceStr = shaderSrc.c_str();
@@ -672,7 +678,7 @@ static bool attach_shader(const GrGLContext& glCtx,
}
SkDEBUGFAIL("Shader compilation failed!");
GR_GL_CALL(gli, DeleteShader(shaderId));
- return false;
+ return 0;
}
}
if (c_PrintShaders) {
@@ -680,12 +686,16 @@ static bool attach_shader(const GrGLContext& glCtx,
GrPrintf("\n");
}
+ // Attach the shader, but defer deletion until after we have linked the program.
+ // This works around a bug in the Android emulator's GLES2 wrapper which
+ // will immediately delete the shader object and free its memory even though it's
+ // attached to a program, which then causes glLinkProgram to fail.
GR_GL_CALL(gli, AttachShader(programId, shaderId));
- GR_GL_CALL(gli, DeleteShader(shaderId));
- return true;
+
+ return shaderId;
}
-bool GrGLShaderBuilder::compileAndAttachShaders(GrGLuint programId) const {
+bool GrGLShaderBuilder::compileAndAttachShaders(GrGLuint programId, SkTDArray<GrGLuint>* shaderIds) const {
SkString fragShaderSrc(GrGetGLSLVersionDecl(this->ctxInfo()));
fragShaderSrc.append(fFSExtensions);
append_default_precision_qualifier(kDefaultFragmentPrecision,
@@ -700,10 +710,14 @@ bool GrGLShaderBuilder::compileAndAttachShaders(GrGLuint programId) const {
fragShaderSrc.append("void main() {\n");
fragShaderSrc.append(fFSCode);
fragShaderSrc.append("}\n");
- if (!attach_shader(fGpu->glContext(), programId, GR_GL_FRAGMENT_SHADER, fragShaderSrc)) {
+
+ GrGLuint fragShaderId = attach_shader(fGpu->glContext(), programId, GR_GL_FRAGMENT_SHADER, fragShaderSrc);
+ if (!fragShaderId) {
return false;
}
+ *shaderIds->append() = fragShaderId;
+
return true;
}
@@ -870,7 +884,7 @@ GrGLProgramEffects* GrGLFullShaderBuilder::createAndEmitEffects(
return programEffectsBuilder.finish();
}
-bool GrGLFullShaderBuilder::compileAndAttachShaders(GrGLuint programId) const {
+bool GrGLFullShaderBuilder::compileAndAttachShaders(GrGLuint programId, SkTDArray<GrGLuint>* shaderIds) const {
const GrGLContext& glCtx = this->gpu()->glContext();
SkString vertShaderSrc(GrGetGLSLVersionDecl(this->ctxInfo()));
this->appendUniformDecls(kVertex_Visibility, &vertShaderSrc);
@@ -879,9 +893,11 @@ bool GrGLFullShaderBuilder::compileAndAttachShaders(GrGLuint programId) const {
vertShaderSrc.append("void main() {\n");
vertShaderSrc.append(fVSCode);
vertShaderSrc.append("}\n");
- if (!attach_shader(glCtx, programId, GR_GL_VERTEX_SHADER, vertShaderSrc)) {
+ GrGLuint vertShaderId = attach_shader(glCtx, programId, GR_GL_VERTEX_SHADER, vertShaderSrc);
+ if (!vertShaderId) {
return false;
}
+ *shaderIds->append() = vertShaderId;
#if GR_GL_EXPERIMENTAL_GS
if (fDesc.getHeader().fExperimentalGS) {
@@ -907,13 +923,15 @@ bool GrGLFullShaderBuilder::compileAndAttachShaders(GrGLuint programId) const {
"\t}\n"
"\tEndPrimitive();\n");
geomShaderSrc.append("}\n");
- if (!attach_shader(glCtx, programId, GR_GL_GEOMETRY_SHADER, geomShaderSrc)) {
+ GrGLuint geomShaderId = attach_shader(glCtx, programId, GR_GL_GEOMETRY_SHADER, geomShaderSrc);
+ if (!geomShaderId) {
return false;
}
+ *shaderIds->append() = geomShaderId;
}
#endif
- return this->INHERITED::compileAndAttachShaders(programId);
+ return this->INHERITED::compileAndAttachShaders(programId, shaderIds);
}
void GrGLFullShaderBuilder::bindProgramLocations(GrGLuint programId) const {
« no previous file with comments | « src/gpu/gl/GrGLShaderBuilder.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698