fix uninitialized variable on ES2 devices

On ES2 devices (or in forceES2 mode), we emulate the sRGB swapchain in the shader if the h/w doesn't support it. In that case, the emulation is controlled by a uniform that technically lives in the frameUniforms block. However, the frameUniforms buffer is not updated, instead, the uniform is manually set. Unfortunately, the UBO emulation overrides it with the uninitialized variable. BUGS=[377913730]
google · Nov 13, 2024 · cf7360b · cf7360b
1 parent 2a9dcd7
commit cf7360b
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 31 deletions.
diff --git a/...le-hello-triangle/src/main/java/com/google/android/filament/hellotriangle/MainActivity.kt b/...le-hello-triangle/src/main/java/com/google/android/filament/hellotriangle/MainActivity.kt
@@ -112,7 +112,13 @@ class MainActivity : Activity() {
     }
 
     private fun setupFilament() {
-        engine = Engine.Builder().featureLevel(Engine.FeatureLevel.FEATURE_LEVEL_0).build()
+        val config = Engine.Config()
+        //config.forceGLES2Context = true
+
+        engine = Engine.Builder()
+            .config(config)
+            .featureLevel(Engine.FeatureLevel.FEATURE_LEVEL_0)
+            .build()
         renderer = engine.createRenderer()
         scene = engine.createScene()
         view = engine.createView()
@@ -123,7 +129,9 @@ class MainActivity : Activity() {
         scene.skybox = Skybox.Builder().color(0.035f, 0.035f, 0.035f, 1.0f).build(engine)
 
         // post-processing is not supported at feature level 0
-        view.isPostProcessingEnabled = false
+        if (engine.activeFeatureLevel == Engine.FeatureLevel.FEATURE_LEVEL_0) {
+            view.isPostProcessingEnabled = false
+        }
 
         // Tell the view which camera we want to use
         view.camera = camera

diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp
@@ -614,7 +614,7 @@ FeatureLevel OpenGLContext::resolveFeatureLevel(GLint major, GLint minor,
                 featureLevel = FeatureLevel::FEATURE_LEVEL_2;
                 if (gets.max_texture_image_units >= MAX_FRAGMENT_SAMPLER_COUNT &&
                     gets.max_combined_texture_image_units >=
-                    (MAX_FRAGMENT_SAMPLER_COUNT + MAX_VERTEX_SAMPLER_COUNT)) {
+                            (MAX_FRAGMENT_SAMPLER_COUNT + MAX_VERTEX_SAMPLER_COUNT)) {
                     featureLevel = FeatureLevel::FEATURE_LEVEL_3;
                 }
             }
@@ -623,15 +623,13 @@ FeatureLevel OpenGLContext::resolveFeatureLevel(GLint major, GLint minor,
 #   ifndef IOS // IOS is guaranteed to have ES3.x
     else if (UTILS_UNLIKELY(major == 2)) {
         // Runtime OpenGL version is ES 2.x
-#       if defined(BACKEND_OPENGL_LEVEL_GLES30)
-        // mandatory extensions (all supported by Mali-400 and Adreno 304)
-        assert_invariant(exts.OES_depth_texture);
-        assert_invariant(exts.OES_depth24);
-        assert_invariant(exts.OES_packed_depth_stencil);
-        assert_invariant(exts.OES_rgb8_rgba8);
-        assert_invariant(exts.OES_standard_derivatives);
-        assert_invariant(exts.OES_texture_npot);
-#       endif
+        // note: mandatory extensions (all supported by Mali-400 and Adreno 304)
+        //      OES_depth_texture
+        //      OES_depth24
+        //      OES_packed_depth_stencil
+        //      OES_rgb8_rgba8
+        //      OES_standard_derivatives
+        //      OES_texture_npot
         featureLevel = FeatureLevel::FEATURE_LEVEL_0;
     }
 #   endif // IOS

diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp
@@ -394,24 +394,21 @@ void OpenGLDriver::bindTexture(GLuint unit, GLTexture const* t) noexcept {
 }
 
 bool OpenGLDriver::useProgram(OpenGLProgram* p) noexcept {
-    if (UTILS_UNLIKELY(mBoundProgram == p)) {
-        // program didn't change, don't do anything.
-        return true;
-    }
-
-    // compile/link the program if needed and call glUseProgram
-    bool const success = p->use(this, mContext);
-    assert_invariant(success == p->isValid());
-
-    if (success) {
-        // TODO: we could even improve this if the program could tell us which of the descriptors
-        //       bindings actually changed. In practice, it is likely that set 0 or 1 might not
-        //       change often.
-        decltype(mInvalidDescriptorSetBindings) changed;
-        changed.setValue((1 << MAX_DESCRIPTOR_SET_COUNT) - 1);
-        mInvalidDescriptorSetBindings |= changed;
-
-        mBoundProgram = p;
+    bool success = true;
+    if (mBoundProgram != p) {
+        // compile/link the program if needed and call glUseProgram
+        success = p->use(this, mContext);
+        assert_invariant(success == p->isValid());
+        if (success) {
+            // TODO: we could even improve this if the program could tell us which of the descriptors
+            //       bindings actually changed. In practice, it is likely that set 0 or 1 might not
+            //       change often.
+            decltype(mInvalidDescriptorSetBindings) changed;
+            changed.setValue((1 << MAX_DESCRIPTOR_SET_COUNT) - 1);
+            mInvalidDescriptorSetBindings |= changed;
+
+            mBoundProgram = p;
+        }
     }
 
     if (UTILS_UNLIKELY(mContext.isES2() && success)) {

diff --git a/filament/backend/src/opengl/OpenGLProgram.cpp b/filament/backend/src/opengl/OpenGLProgram.cpp
@@ -255,7 +255,9 @@ void OpenGLProgram::updateUniforms(
     for (size_t i = 0, c = records.uniforms.size(); i < c; i++) {
         Program::Uniform const& u = records.uniforms[i];
         GLint const loc = records.locations[i];
-        if (loc < 0) {
+        // mRec709Location is special, it is handled by setRec709ColorSpace() and the corresponding
+        // entry in `buffer` is typically not initialized, so we skip it.
+        if (loc < 0 || loc == mRec709Location) {
             continue;
         }
         // u.offset is in 'uint32_t' units