From cab799f531ce2fe871083b201efa2c142ed0bbb5 Mon Sep 17 00:00:00 2001 From: Mathias Agopian <mathias@google.com> Date: Mon, 17 Jun 2024 17:11:23 -0700 Subject: [PATCH] matdbg would cause a crash if program was invalid --- filament/backend/src/opengl/OpenGLDriver.cpp | 20 +++++++++++-------- filament/backend/src/opengl/OpenGLProgram.h | 15 ++++++++++++-- .../src/opengl/ShaderCompilerService.cpp | 2 +- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 414d0c8aa211..4f88c9717f5a 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -304,6 +304,13 @@ void OpenGLDriver::bindSampler(GLuint unit, GLuint sampler) noexcept { void OpenGLDriver::setPushConstant(backend::ShaderStage stage, uint8_t index, backend::PushConstantVariant value) { assert_invariant(stage == ShaderStage::VERTEX || stage == ShaderStage::FRAGMENT); + +#if FILAMENT_ENABLE_MATDBG + if (UTILS_UNLIKELY(!mValidProgram)) { + return; + } +#endif + utils::Slice<std::pair<GLint, ConstantType>> constants; if (stage == ShaderStage::VERTEX) { constants = mCurrentPushConstants->vertexConstants; @@ -340,15 +347,11 @@ void OpenGLDriver::bindTexture(GLuint unit, GLTexture const* t) noexcept { } bool OpenGLDriver::useProgram(OpenGLProgram* p) noexcept { - if (UTILS_UNLIKELY(!p->isValid())) { - // If the program is not valid, we can't call use(). - return false; - } - // set-up textures and samplers in the proper TMUs (as specified in setSamplers) - p->use(this, mContext); + bool const success = p->use(this, mContext); + assert_invariant(success == p->isValid()); - if (UTILS_UNLIKELY(mContext.isES2())) { + if (UTILS_UNLIKELY(mContext.isES2() && success)) { for (uint32_t i = 0; i < Program::UNIFORM_BINDING_COUNT; i++) { auto [id, buffer, age] = mContext.getEs2UniformBinding(i); if (buffer) { @@ -359,7 +362,8 @@ bool OpenGLDriver::useProgram(OpenGLProgram* p) noexcept { // when mPlatform.isSRGBSwapChainSupported() is false (no need to check though). p->setRec709ColorSpace(mRec709OutputColorspace); } - return true; + + return success; } diff --git a/filament/backend/src/opengl/OpenGLProgram.h b/filament/backend/src/opengl/OpenGLProgram.h index 19be485ac6bd..2cb43131ea14 100644 --- a/filament/backend/src/opengl/OpenGLProgram.h +++ b/filament/backend/src/opengl/OpenGLProgram.h @@ -53,11 +53,21 @@ class OpenGLProgram : public HwProgram { bool isValid() const noexcept { return mToken || gl.program != 0; } - void use(OpenGLDriver* const gld, OpenGLContext& context) noexcept { - if (UTILS_UNLIKELY(!gl.program)) { + bool use(OpenGLDriver* const gld, OpenGLContext& context) noexcept { + // both non-null is impossible by construction + assert_invariant(!mToken || !gl.program); + + if (UTILS_UNLIKELY(mToken && !gl.program)) { + // first time a program is used initialize(*gld); } + if (UTILS_UNLIKELY(!gl.program)) { + // compilation failed (token should be null) + assert_invariant(!mToken); + return false; + } + context.useProgram(gl.program); if (UTILS_UNLIKELY(mUsedBindingsCount)) { // We rely on GL state tracking to avoid unnecessary glBindTexture / glBindSampler @@ -74,6 +84,7 @@ class OpenGLProgram : public HwProgram { updateSamplers(gld); } + return true; } // For ES2 only diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp index 5b9397ddd4bb..dd64a1b794c3 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.cpp +++ b/filament/backend/src/opengl/ShaderCompilerService.cpp @@ -359,7 +359,7 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( GLuint ShaderCompilerService::getProgram(ShaderCompilerService::program_token_t& token) { GLuint const program = initialize(token); assert_invariant(token == nullptr); -#ifndef FILAMENT_ENABLE_MATDBG +#if !FILAMENT_ENABLE_MATDBG assert_invariant(program); #endif return program;