matdbg would cause a crash if program was invalid

google · Jun 18, 2024 · cab799f · cab799f
1 parent e95edef
commit cab799f
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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;