Bug 910754 - [Skia] Defer deletion of our shader objects until after linking the program r=upstream(bsalomon)

This commit is contained in:
George Wright 2014-02-13 01:19:51 -05:00
parent fee812bd01
commit 1eb404b7fc
2 changed files with 38 additions and 20 deletions

View File

@ -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 {

View File

@ -248,7 +248,7 @@ protected:
int effectCnt,
GrGLSLExpr4* inOutFSColor);
virtual bool compileAndAttachShaders(GrGLuint programId) const;
virtual bool compileAndAttachShaders(GrGLuint programId, SkTDArray<GrGLuint>* shaderIds) const;
virtual void bindProgramLocations(GrGLuint programId) const;
void appendDecls(const VarArray&, SkString*) const;
@ -422,7 +422,7 @@ public:
}
protected:
virtual bool compileAndAttachShaders(GrGLuint programId) const SK_OVERRIDE;
virtual bool compileAndAttachShaders(GrGLuint programId, SkTDArray<GrGLuint>* shaderIds) const SK_OVERRIDE;
virtual void bindProgramLocations(GrGLuint programId) const SK_OVERRIDE;
private: