diff options
author | Le Hoang Quyen <lehoangquyen@chromium.org> | 2023-12-12 00:33:03 +0800 |
---|---|---|
committer | SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> | 2023-12-12 17:28:50 +0000 |
commit | 16298087c2772faf97e4ee0198bb9532288d5445 (patch) | |
tree | 9ad089a66eea9ade48ca6bedcf869c38a8c86338 | |
parent | 7685acfb622190506f54533b5083737cb36f86ca (diff) | |
download | skia-16298087c2772faf97e4ee0198bb9532288d5445.tar.gz |
Add "shaderWasCached" param to ShaderErrorHandler::compileError
Add additional parameter to ShaderErrorHandler::compileError to let
Chrome decide what to do if the faulty shader was cached or not. E.g.
if it was then delete the cache.
Bug: chromium:1442633
Change-Id: I33726a3fee53f6a7d16d829320a9f44264369772
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/788476
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Quyen Le <lehoangquyen@chromium.org>
-rw-r--r-- | include/gpu/ShaderErrorHandler.h | 12 | ||||
-rw-r--r-- | src/gpu/PipelineUtils.cpp | 3 | ||||
-rw-r--r-- | src/gpu/ganesh/gl/GrGLGpu.cpp | 18 | ||||
-rw-r--r-- | src/gpu/ganesh/gl/builders/GrGLProgramBuilder.cpp | 30 | ||||
-rw-r--r-- | src/gpu/ganesh/gl/builders/GrGLProgramBuilder.h | 1 | ||||
-rw-r--r-- | src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.cpp | 7 | ||||
-rw-r--r-- | src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.h | 2 | ||||
-rw-r--r-- | src/gpu/ganesh/mtl/GrMtlUtil.mm | 3 | ||||
-rw-r--r-- | src/gpu/graphite/dawn/DawnGraphiteUtils.cpp | 3 | ||||
-rw-r--r-- | src/gpu/graphite/mtl/MtlGraphiteUtils.mm | 3 |
10 files changed, 65 insertions, 17 deletions
diff --git a/include/gpu/ShaderErrorHandler.h b/include/gpu/ShaderErrorHandler.h index 8960da5c5a..7e294dc6a9 100644 --- a/include/gpu/ShaderErrorHandler.h +++ b/include/gpu/ShaderErrorHandler.h @@ -18,7 +18,17 @@ class SK_API ShaderErrorHandler { public: virtual ~ShaderErrorHandler() = default; - virtual void compileError(const char* shader, const char* errors) = 0; + /** + * compileError(shader, errors) is kept for backward compatibility with older clients. + */ + virtual void compileError([[maybe_unused]] const char* shader, + [[maybe_unused]] const char* errors) {} + virtual void compileError(const char* shader, + const char* errors, + [[maybe_unused]] bool shaderWasCached) { + // Default implementation. Ignore shaderWasCached. + this->compileError(shader, errors); + } protected: ShaderErrorHandler() = default; diff --git a/src/gpu/PipelineUtils.cpp b/src/gpu/PipelineUtils.cpp index f2634bdab2..b19e2d6862 100644 --- a/src/gpu/PipelineUtils.cpp +++ b/src/gpu/PipelineUtils.cpp @@ -27,7 +27,8 @@ static bool sksl_to_backend(SkSL::Compiler* compiler, src, settings); if (!program || !(compiler->*toBackend)(*program, output)) { - errorHandler->compileError(src.c_str(), compiler->errorText().c_str()); + errorHandler->compileError( + src.c_str(), compiler->errorText().c_str(), /*shaderWasCached=*/false); return false; } diff --git a/src/gpu/ganesh/gl/GrGLGpu.cpp b/src/gpu/ganesh/gl/GrGLGpu.cpp index cac15634f2..99dbfb5c88 100644 --- a/src/gpu/ganesh/gl/GrGLGpu.cpp +++ b/src/gpu/ganesh/gl/GrGLGpu.cpp @@ -3347,6 +3347,7 @@ bool GrGLGpu::createCopyProgram(GrTexture* srcTex) { fCopyPrograms[progIdx].fProgram, GR_GL_VERTEX_SHADER, glsl[kVertex_GrShaderType], + /*shaderWasCached=*/false, fProgramCache->stats(), errorHandler); SkASSERT(interface == SkSL::Program::Interface()); @@ -3362,6 +3363,7 @@ bool GrGLGpu::createCopyProgram(GrTexture* srcTex) { fCopyPrograms[progIdx].fProgram, GR_GL_FRAGMENT_SHADER, glsl[kFragment_GrShaderType], + /*shaderWasCached=*/false, fProgramCache->stats(), errorHandler); SkASSERT(interface == SkSL::Program::Interface()); @@ -3373,7 +3375,12 @@ bool GrGLGpu::createCopyProgram(GrTexture* srcTex) { const std::string* sksl[kGrShaderTypeCount] = {&vertexSkSL, &fragmentSkSL}; GL_CALL(LinkProgram(fCopyPrograms[progIdx].fProgram)); - if (!GrGLCheckLinkStatus(this, fCopyPrograms[progIdx].fProgram, errorHandler, sksl, glsl)) { + if (!GrGLCheckLinkStatus(this, + fCopyPrograms[progIdx].fProgram, + /*shaderWasCached=*/false, + errorHandler, + sksl, + glsl)) { // Failed to link, delete everything cleanup_program(this, &fCopyPrograms[progIdx].fProgram, &vshader, &fshader); return false; @@ -3525,6 +3532,7 @@ bool GrGLGpu::createMipmapProgram(int progIdx) { fMipmapPrograms[progIdx].fProgram, GR_GL_VERTEX_SHADER, glsl[kVertex_GrShaderType], + /*shaderWasCached=*/false, fProgramCache->stats(), errorHandler); SkASSERT(interface == SkSL::Program::Interface()); @@ -3539,6 +3547,7 @@ bool GrGLGpu::createMipmapProgram(int progIdx) { fMipmapPrograms[progIdx].fProgram, GR_GL_FRAGMENT_SHADER, glsl[kFragment_GrShaderType], + /*shaderWasCached=*/false, fProgramCache->stats(), errorHandler); SkASSERT(interface == SkSL::Program::Interface()); @@ -3549,7 +3558,12 @@ bool GrGLGpu::createMipmapProgram(int progIdx) { const std::string* sksl[kGrShaderTypeCount] = {&vertexSkSL, &fragmentSkSL}; GL_CALL(LinkProgram(fMipmapPrograms[progIdx].fProgram)); - if (!GrGLCheckLinkStatus(this, fMipmapPrograms[progIdx].fProgram, errorHandler, sksl, glsl)) { + if (!GrGLCheckLinkStatus(this, + fMipmapPrograms[progIdx].fProgram, + /*shaderWasCached=*/false, + errorHandler, + sksl, + glsl)) { // Program linking failed, clean up cleanup_program(this, &fMipmapPrograms[progIdx].fProgram, &vshader, &fshader); return false; diff --git a/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.cpp b/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.cpp index 034ae8a0a1..b54ec46cd8 100644 --- a/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.cpp +++ b/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.cpp @@ -102,12 +102,14 @@ bool GrGLProgramBuilder::compileAndAttachShaders(const std::string& glsl, GrGLuint programId, GrGLenum type, SkTDArray<GrGLuint>* shaderIds, + bool shaderWasCached, GrContextOptions::ShaderErrorHandler* errHandler) { GrGLGpu* gpu = this->gpu(); GrGLuint shaderId = GrGLCompileAndAttachShader(gpu->glContext(), programId, type, glsl, + shaderWasCached, gpu->pipelineBuilder()->stats(), errHandler); if (!shaderId) { @@ -282,7 +284,7 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr // failure (we can still recover by compiling the program from source, below). // Clients won't be directly notified, but they can infer this from the trace // events, and from the traffic to the persistent cache. - cached = GrGLCheckLinkStatus(fGpu, programID, + cached = GrGLCheckLinkStatus(fGpu, programID, /*shaderWasCached=*/true, /*errorHandler=*/nullptr, nullptr, nullptr); if (cached) { this->addInputVars(interface); @@ -341,8 +343,12 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr } this->addInputVars(interface); - if (!this->compileAndAttachShaders(glsl[kFragment_GrShaderType], programID, - GR_GL_FRAGMENT_SHADER, &shadersToDelete, errorHandler)) { + if (!this->compileAndAttachShaders(glsl[kFragment_GrShaderType], + programID, + GR_GL_FRAGMENT_SHADER, + &shadersToDelete, + cached, + errorHandler)) { cleanup_program(fGpu, programID, shadersToDelete); return nullptr; } @@ -364,8 +370,12 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr return nullptr; } } - if (!this->compileAndAttachShaders(glsl[kVertex_GrShaderType], programID, - GR_GL_VERTEX_SHADER, &shadersToDelete, errorHandler)) { + if (!this->compileAndAttachShaders(glsl[kVertex_GrShaderType], + programID, + GR_GL_VERTEX_SHADER, + &shadersToDelete, + cached, + errorHandler)) { cleanup_program(fGpu, programID, shadersToDelete); return nullptr; } @@ -378,7 +388,7 @@ sk_sp<GrGLProgram> GrGLProgramBuilder::finalize(const GrGLPrecompiledProgram* pr { TRACE_EVENT0_ALWAYS("skia.shaders", "driver_link_program"); GL_CALL(LinkProgram(programID)); - if (!GrGLCheckLinkStatus(fGpu, programID, errorHandler, sksl, glsl)) { + if (!GrGLCheckLinkStatus(fGpu, programID, cached, errorHandler, sksl, glsl)) { cleanup_program(fGpu, programID, shadersToDelete); return nullptr; } @@ -481,8 +491,12 @@ bool GrGLProgramBuilder::PrecompileProgram(GrDirectContext* dContext, return false; } - if (GrGLuint shaderID = GrGLCompileAndAttachShader(glGpu->glContext(), programID, type, - glsl, glGpu->pipelineBuilder()->stats(), + if (GrGLuint shaderID = GrGLCompileAndAttachShader(glGpu->glContext(), + programID, + type, + glsl, + /*shaderWasCached=*/false, + glGpu->pipelineBuilder()->stats(), errorHandler)) { shadersToDelete.push_back(shaderID); return true; diff --git a/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.h b/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.h index 56a6113087..9ae18ee9f0 100644 --- a/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.h +++ b/src/gpu/ganesh/gl/builders/GrGLProgramBuilder.h @@ -65,6 +65,7 @@ private: GrGLuint programId, GrGLenum type, SkTDArray<GrGLuint>* shaderIds, + bool shaderWasCached, GrContextOptions::ShaderErrorHandler* errorHandler); void computeCountsAndStrides(GrGLuint programID, diff --git a/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.cpp b/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.cpp index 161187ba7d..85a799763f 100644 --- a/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.cpp +++ b/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.cpp @@ -20,6 +20,7 @@ GrGLuint GrGLCompileAndAttachShader(const GrGLContext& glCtx, GrGLuint programId, GrGLenum type, const std::string& glsl, + bool shaderWasCached, GrThreadSafePipelineBuilder::Stats* stats, GrContextOptions::ShaderErrorHandler* errorHandler) { TRACE_EVENT0_ALWAYS("skia.shaders", "driver_compile_shader"); @@ -53,7 +54,8 @@ GrGLuint GrGLCompileAndAttachShader(const GrGLContext& glCtx, GrGLsizei length = GR_GL_INIT_ZERO; GR_GL_CALL(gli, GetShaderInfoLog(shaderId, infoLen+1, &length, (char*)log.get())); } - errorHandler->compileError(glsl.c_str(), infoLen > 0 ? (const char*)log.get() : ""); + errorHandler->compileError( + glsl.c_str(), infoLen > 0 ? (const char*)log.get() : "", shaderWasCached); GR_GL_CALL(gli, DeleteShader(shaderId)); return 0; } @@ -69,6 +71,7 @@ GrGLuint GrGLCompileAndAttachShader(const GrGLContext& glCtx, bool GrGLCheckLinkStatus(const GrGLGpu* gpu, GrGLuint programID, + bool shaderWasCached, GrContextOptions::ShaderErrorHandler* errorHandler, const std::string* sksl[kGrShaderTypeCount], const std::string glsl[kGrShaderTypeCount]) { @@ -101,7 +104,7 @@ bool GrGLCheckLinkStatus(const GrGLGpu* gpu, } const char* errorMsg = (infoLen > 0) ? (const char*)log.get() : "link failed but did not provide an info log"; - errorHandler->compileError(allShaders.c_str(), errorMsg); + errorHandler->compileError(allShaders.c_str(), errorMsg, shaderWasCached); } return SkToBool(linked); } diff --git a/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.h b/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.h index 8f57f27e58..ea845f3e7c 100644 --- a/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.h +++ b/src/gpu/ganesh/gl/builders/GrGLShaderStringBuilder.h @@ -18,11 +18,13 @@ GrGLuint GrGLCompileAndAttachShader(const GrGLContext& glCtx, GrGLuint programId, GrGLenum type, const std::string& glsl, + bool shaderWasCached, GrThreadSafePipelineBuilder::Stats*, GrContextOptions::ShaderErrorHandler* errorHandler); bool GrGLCheckLinkStatus(const GrGLGpu* gpu, GrGLuint programID, + bool shaderWasCached, GrContextOptions::ShaderErrorHandler* errorHandler, const std::string* sksl[kGrShaderTypeCount], const std::string glsl[kGrShaderTypeCount]); diff --git a/src/gpu/ganesh/mtl/GrMtlUtil.mm b/src/gpu/ganesh/mtl/GrMtlUtil.mm index 90ac40d654..7fb35f5649 100644 --- a/src/gpu/ganesh/mtl/GrMtlUtil.mm +++ b/src/gpu/ganesh/mtl/GrMtlUtil.mm @@ -84,7 +84,8 @@ id<MTLLibrary> GrCompileMtlShaderLibrary(const GrMtlGpu* gpu, options, &error); } if (!compiledLibrary) { - errorHandler->compileError(msl.c_str(), error.debugDescription.UTF8String); + errorHandler->compileError( + msl.c_str(), error.debugDescription.UTF8String, /*shaderWasCached=*/false); return nil; } diff --git a/src/gpu/graphite/dawn/DawnGraphiteUtils.cpp b/src/gpu/graphite/dawn/DawnGraphiteUtils.cpp index 5d2375c384..4bba77e688 100644 --- a/src/gpu/graphite/dawn/DawnGraphiteUtils.cpp +++ b/src/gpu/graphite/dawn/DawnGraphiteUtils.cpp @@ -118,7 +118,8 @@ static bool check_shader_module(wgpu::ShaderModule* module, std::to_string(entry.linePos) + ' ' + entry.message + '\n'; } - self->fErrorHandler->compileError(self->fShaderText, errors.c_str()); + self->fErrorHandler->compileError( + self->fShaderText, errors.c_str(), /*shaderWasCached=*/false); } } diff --git a/src/gpu/graphite/mtl/MtlGraphiteUtils.mm b/src/gpu/graphite/mtl/MtlGraphiteUtils.mm index 0efdff41cb..b536f5e824 100644 --- a/src/gpu/graphite/mtl/MtlGraphiteUtils.mm +++ b/src/gpu/graphite/mtl/MtlGraphiteUtils.mm @@ -92,7 +92,8 @@ sk_cfp<id<MTLLibrary>> MtlCompileShaderLibrary(const MtlSharedContext* sharedCon error:&error]); if (!compiledLibrary) { std::string mslStr(msl); - errorHandler->compileError(mslStr.c_str(), error.debugDescription.UTF8String); + errorHandler->compileError( + mslStr.c_str(), error.debugDescription.UTF8String, /*shaderWasCached=*/false); return nil; } |