Merge branch 'rc/1.41.0' into release

google · Aug 16, 2023 · b13497e · b13497e
2 parents 1a50420 + 175c9f9
commit b13497e
Show file tree

Hide file tree

Showing 27 changed files with 328 additions and 114 deletions.
diff --git a/BUILDING.md b/BUILDING.md
@@ -5,7 +5,7 @@
 To build Filament, you must first install the following tools:
 
 - CMake 3.19 (or more recent)
-- clang 7.0 (or more recent)
+- clang 14.0 (or more recent)
 - [ninja 1.10](https://github.com/ninja-build/ninja/wiki/Pre-built-Ninja-packages) (or more recent)
 
 Additional dependencies may be required for your operating system. Please refer to the appropriate
@@ -87,10 +87,10 @@ Options can also be set with the CMake GUI.
 
 Make sure you've installed the following dependencies:
 
-- `clang-7` or higher
+- `clang-14` or higher
 - `libglu1-mesa-dev`
-- `libc++-7-dev` (`libcxx-devel` and `libcxx-static` on Fedora) or higher
-- `libc++abi-7-dev` (`libcxxabi-static` on Fedora) or higher
+- `libc++-14-dev` (`libcxx-devel` and `libcxx-static` on Fedora) or higher
+- `libc++abi-14-dev` (`libcxxabi-static` on Fedora) or higher
 - `ninja-build`
 - `libxi-dev`
 - `libxcomposite-dev` (`libXcomposite-devel` on Fedora)
@@ -114,7 +114,7 @@ Your Linux distribution might default to `gcc` instead of `clang`, if that's the
 ```
 $ mkdir out/cmake-release
 $ cd out/cmake-release
-# Or use a specific version of clang, for instance /usr/bin/clang-7
+# Or use a specific version of clang, for instance /usr/bin/clang-14
 $ CC=/usr/bin/clang CXX=/usr/bin/clang++ CXXFLAGS=-stdlib=libc++ \
     cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../release/filament ../..
 ```
@@ -124,8 +124,8 @@ solution is to use `update-alternatives` to both change the default compiler, an
 specific version of clang:
 
 ```
-$ update-alternatives --install /usr/bin/clang clang /usr/bin/clang-7 100
-$ update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-7 100
+$ update-alternatives --install /usr/bin/clang clang /usr/bin/clang-14 100
+$ update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-14 100
 $ update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100
 $ update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100
 ```

diff --git a/README.md b/README.md
@@ -31,7 +31,7 @@ repositories {
 }
 
 dependencies {
-    implementation 'com.google.android.filament:filament-android:1.40.5'
+    implementation 'com.google.android.filament:filament-android:1.41.0'
 }
 ```
 
@@ -40,6 +40,7 @@ Here are all the libraries available in the group `com.google.android.filament`:
 | Artifact      | Description   |
 | ------------- | ------------- |
 | [![filament-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android/badge.svg?subject=filament-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android)  | The Filament rendering engine itself. |
+| [![filament-android-debug](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android-debug/badge.svg?subject=filament-android-debug)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android-debug)  | Debug version of `filament-android`. |
 | [![gltfio-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/gltfio-android/badge.svg?subject=gltfio-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/gltfio-android) | A glTF 2.0 loader for Filament, depends on `filament-android`. |
 | [![filament-utils-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-utils-android/badge.svg?subject=filament-utils-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-utils-android) | KTX loading, Kotlin math, and camera utilities, depends on `gltfio-android`. |
 | [![filamat-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filamat-android/badge.svg?subject=filamat-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filamat-android) | A runtime material builder/compiler. This library is large but contains a full shader compiler/validator/optimizer and supports both OpenGL and Vulkan. |
@@ -50,7 +51,7 @@ Here are all the libraries available in the group `com.google.android.filament`:
 iOS projects can use CocoaPods to install the latest release:
 
 ```
-pod 'Filament', '~> 1.40.5'
+pod 'Filament', '~> 1.41.0'
 ```
 
 ### Snapshots

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -7,6 +7,11 @@ A new header is inserted each time a *tag* is created.
 Instead, if you are authoring a PR for the main branch, add your release note to
 [NEW_RELEASE_NOTES.md](./NEW_RELEASE_NOTES.md).
 
+## v1.41.0
+
+- backend: fix #6997 : picking can fail on Adreno [⚠️ **New Material Version**]
+- backend: A partial workaround for PowerVR devices (#5118, b/190221124) [⚠️ **Recompile Materials**]
+
 ## v1.40.5
 
 - backend: Disable timer queries on all Mali GPUs (fixes b/233754398)

diff --git a/android/filament-android/src/main/java/com/google/android/filament/RenderTarget.java b/android/filament-android/src/main/java/com/google/android/filament/RenderTarget.java
@@ -81,8 +81,6 @@ public Builder() {
         /**
          * Sets a texture to a given attachment point.
          *
-         * <p>All RenderTargets must have a non-null <code>COLOR</code> attachment.</p>
-         *
          * @param attachment The attachment point of the texture.
          * @param texture The associated texture object.
          * @return A reference to this Builder for chaining calls.

diff --git a/android/gradle.properties b/android/gradle.properties
@@ -1,5 +1,5 @@
 GROUP=com.google.android.filament
-VERSION_NAME=1.40.5
+VERSION_NAME=1.41.0
 
 POM_DESCRIPTION=Real-time physically based rendering engine for Android.
 

diff --git a/...samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt b/...samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt
@@ -28,6 +28,7 @@ import com.google.android.filament.Fence
 import com.google.android.filament.IndirectLight
 import com.google.android.filament.Skybox
 import com.google.android.filament.View
+import com.google.android.filament.View.OnPickCallback
 import com.google.android.filament.utils.*
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -56,7 +57,9 @@ class MainActivity : Activity() {
     private lateinit var modelViewer: ModelViewer
     private lateinit var titlebarHint: TextView
     private val doubleTapListener = DoubleTapListener()
+    private val singleTapListener = SingleTapListener()
     private lateinit var doubleTapDetector: GestureDetector
+    private lateinit var singleTapDetector: GestureDetector
     private var remoteServer: RemoteServer? = null
     private var statusToast: Toast? = null
     private var statusText: String? = null
@@ -77,6 +80,7 @@ class MainActivity : Activity() {
         choreographer = Choreographer.getInstance()
 
         doubleTapDetector = GestureDetector(applicationContext, doubleTapListener)
+        singleTapDetector = GestureDetector(applicationContext, singleTapListener)
 
         modelViewer = ModelViewer(surfaceView)
         viewerContent.view = modelViewer.view
@@ -88,6 +92,7 @@ class MainActivity : Activity() {
         surfaceView.setOnTouchListener { _, event ->
             modelViewer.onTouchEvent(event)
             doubleTapDetector.onTouchEvent(event)
+            singleTapDetector.onTouchEvent(event)
             true
         }
 
@@ -229,6 +234,7 @@ class MainActivity : Activity() {
                 modelViewer.scene.skybox = sky
                 modelViewer.scene.indirectLight = ibl
                 viewerContent.indirectLight = ibl
+
             }
         }
     }
@@ -430,4 +436,19 @@ class MainActivity : Activity() {
             return super.onDoubleTap(e)
         }
     }
+
+    // Just for testing purposes
+    inner class SingleTapListener : GestureDetector.SimpleOnGestureListener() {
+        override fun onSingleTapUp(event: MotionEvent): Boolean {
+            modelViewer.view.pick(
+                event.x.toInt(),
+                surfaceView.height - event.y.toInt(),
+                surfaceView.handler, {
+                    val name = modelViewer.asset!!.getName(it.renderable)
+                    Log.v("Filament", "Picked ${it.renderable}: " + name)
+                },
+            )
+            return super.onSingleTapUp(event)
+        }
+    }
 }
diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp
@@ -502,7 +502,7 @@ void OpenGLContext::initExtensionsGLES() noexcept {
 
     // ES 3.x implies EXT_discard_framebuffer and OES_vertex_array_object
     if (state.major >= 3) {
-        ext.EXT_color_buffer_float = true;
+        ext.EXT_discard_framebuffer = true;
         ext.OES_vertex_array_object = true;
     }
 }

diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp
@@ -32,7 +32,6 @@
 #include <utils/Systrace.h>
 
 #include <chrono>
-#include <future>
 #include <string>
 #include <string_view>
 #include <variant>
@@ -85,9 +84,40 @@ struct ShaderCompilerService::ProgramToken {
         GLuint program = 0;
     } gl; // 12 bytes
 
+
+    // Sets the programBinary, typically from the compiler thread, and signal the main thread.
+    // This is similar to std::promise::set_value.
+    void set(ProgramBinary programBinary) noexcept {
+        using std::swap;
+        std::unique_lock const l(lock);
+        swap(binary, programBinary);
+        signaled = true;
+        cond.notify_one();
+    }
+
+    // Get the programBinary, wait if necessary.
+    // This is similar to std::future::get
+    ProgramBinary const& get() const noexcept {
+        std::unique_lock l(lock);
+        cond.wait(l, [this](){ return signaled; });
+        return binary;
+    }
+
+    // Checks if the programBinary is ready.
+    // This is similar to std::future::wait_for(0s)
+    bool isReady() const noexcept {
+        std::unique_lock l(lock);
+        using namespace std::chrono_literals;
+        return cond.wait_for(l, 0s, [this](){ return signaled; });
+    }
+
     BlobCacheKey key;
-    std::future<ProgramBinary> binary;
-    bool canceled = false;
+    mutable utils::Mutex lock;
+    mutable utils::Condition cond;
+    ProgramBinary binary;
+    bool signaled = false;
+
+    bool canceled = false; // not part of the signaling
 };
 
 void ShaderCompilerService::setUserData(const program_token_t& token, void* user) noexcept {
@@ -242,16 +272,22 @@ void ShaderCompilerService::init() noexcept {
 }
 
 void ShaderCompilerService::terminate() noexcept {
-    mCompilerThreadPool.terminate();
-
-    // We could have some pending callbacks here, we need to execute them
+    // We could have some pending callbacks here, we need to execute them.
+    // This is equivalent to calling cancelTickOp() on all active tokens.
     for (auto&& op: mRunAtNextTickOps) {
-        Job const& job = std::get<2>(op);
-        if (job.callback) {
+        auto const& [priority, token, job] = op;
+        if (!token && job.callback) {
+            // This is a little fragile here. We know by construction that jobs that have a
+            // null token are the ones that dispatch the user callbacks.
             mDriver.scheduleCallback(job.handler, job.user, job.callback);
         }
     }
     mRunAtNextTickOps.clear();
+
+    // Finally stop the thread pool immediately. Pending jobs will be discarded. We guarantee by
+    // construction that nobody is waiting on a token (because waiting is only done on the main
+    // backend thread, and if we're here, we're on the backend main thread).
+    mCompilerThreadPool.terminate();
 }
 
 ShaderCompilerService::program_token_t ShaderCompilerService::createProgram(
@@ -268,13 +304,9 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram(
     if (!token->gl.program) {
         CompilerPriorityQueue const priorityQueue = program.getPriorityQueue();
         if (mShaderCompilerThreadCount) {
-            // set the future in the token and pass the promise to the worker thread
-            std::promise<ProgramToken::ProgramBinary> promise;
-            token->binary = promise.get_future();
             // queue a compile job
             mCompilerThreadPool.queue(priorityQueue, token,
-                    [this, &gl, promise = std::move(promise),
-                            program = std::move(program), token]() mutable {
+                    [this, &gl, program = std::move(program), token]() mutable {
 
                         // compile the shaders
                         std::array<GLuint, Program::SHADER_TYPE_COUNT> shaders{};
@@ -326,7 +358,7 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram(
 #endif
                         // we don't need to check for success here, it'll be done on the
                         // main thread side.
-                        promise.set_value(binary);
+                        token->set(std::move(binary));
                     });
         } else
         {
@@ -346,10 +378,8 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram(
                     // TODO: see if we could completely eliminate this callback here
                     //       and instead just rely on token->gl.program being atomically
                     //       set by the compiler thread.
-                    assert_invariant(token->binary.valid());
                     // we're using the compiler thread, check if the program is ready, no-op if not.
-                    using namespace std::chrono_literals;
-                    if (token->binary.wait_for(0s) != std::future_status::ready) {
+                    if (!token->isReady()) {
                         return false;
                     }
                     // program binary is ready, retrieve it without blocking
@@ -453,7 +483,7 @@ GLuint ShaderCompilerService::getProgram(ShaderCompilerService::program_token_t&
         glDeleteProgram(token->gl.program);
     }
 
-    token = nullptr;
+    token.reset();
 }
 
 void ShaderCompilerService::tick() {
@@ -512,7 +542,7 @@ void ShaderCompilerService::notifyWhenAllProgramsAreReady(CompilerPriorityQueue
 // ------------------------------------------------------------------------------------------------
 
 void ShaderCompilerService::getProgramFromCompilerPool(program_token_t& token) noexcept {
-    ProgramToken::ProgramBinary const binary{ token->binary.get() };
+    ProgramToken::ProgramBinary const& binary{ token->get() };
     if (!token->canceled) {
         token->gl.shaders = binary.shaders;
         if (UTILS_LIKELY(mUseSharedContext)) {
@@ -532,9 +562,6 @@ GLuint ShaderCompilerService::initialize(program_token_t& token) noexcept {
     SYSTRACE_CALL();
     if (!token->gl.program) {
         if (mShaderCompilerThreadCount) {
-            // Block until the program is ready. This could take a very long time.
-            assert_invariant(token->binary.valid());
-
             // we need this program right now, so move it to the head of the queue.
             mCompilerThreadPool.makeUrgent(token);
 
@@ -803,8 +830,7 @@ void ShaderCompilerService::runAtNextTick(CompilerPriorityQueue priority,
 void ShaderCompilerService::cancelTickOp(program_token_t token) noexcept {
     // We do a linear search here, but this is rare, and we know the list is pretty small.
     auto& ops = mRunAtNextTickOps;
-    auto pos = std::find_if(ops.begin(), ops.end(),
-            [&](const auto& item) {
+    auto pos = std::find_if(ops.begin(), ops.end(), [&](const auto& item) {
         return std::get<1>(item) == token;
     });
     if (pos != ops.end()) {

diff --git a/filament/include/filament/RenderTarget.h b/filament/include/filament/RenderTarget.h
@@ -91,8 +91,6 @@ class UTILS_PUBLIC RenderTarget : public FilamentAPI {
         /**
          * Sets a texture to a given attachment point.
          *
-         * All RenderTargets must have a non-null COLOR attachment.
-         *
          * When using a DEPTH attachment, it is important to always disable post-processing
          * in the View. Failing to do so will cause the DEPTH attachment to be ignored in most
          * cases.

diff --git a/filament/src/details/RenderTarget.cpp b/filament/src/details/RenderTarget.cpp
@@ -68,10 +68,11 @@ RenderTarget* RenderTarget::Builder::build(Engine& engine) {
     using backend::TextureUsage;
     const FRenderTarget::Attachment& color = mImpl->mAttachments[(size_t)AttachmentPoint::COLOR0];
     const FRenderTarget::Attachment& depth = mImpl->mAttachments[(size_t)AttachmentPoint::DEPTH];
-    ASSERT_PRECONDITION(color.texture, "COLOR0 attachment not set");
 
-    ASSERT_PRECONDITION(color.texture->getUsage() & TextureUsage::COLOR_ATTACHMENT,
-            "Texture usage must contain COLOR_ATTACHMENT");
+    if (color.texture) {
+        ASSERT_PRECONDITION(color.texture->getUsage() & TextureUsage::COLOR_ATTACHMENT,
+                "Texture usage must contain COLOR_ATTACHMENT");
+    }
 
     if (depth.texture) {
         ASSERT_PRECONDITION(depth.texture->getUsage() & TextureUsage::DEPTH_ATTACHMENT,
@@ -135,7 +136,7 @@ FRenderTarget::FRenderTarget(FEngine& engine, const RenderTarget::Builder& build
     for (size_t i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) {
         Attachment const& attachment = mAttachments[i];
         if (attachment.texture) {
-            TargetBufferFlags targetBufferBit = getTargetBufferFlagsAt(i);
+            TargetBufferFlags const targetBufferBit = getTargetBufferFlagsAt(i);
             mAttachmentMask |= targetBufferBit;
             setAttachment(mrt[i], (AttachmentPoint)i);
             if (any(attachment.texture->getUsage() &

diff --git a/filament/src/details/View.cpp b/filament/src/details/View.cpp
@@ -1009,9 +1009,8 @@ void FView::executePickingQueries(backend::DriverApi& driver,
                     &pQuery->result.renderable, 4u * 4u, // 4*float
                     backend::PixelDataFormat::RG, backend::PixelDataType::FLOAT,
                     pQuery->handler, [](void*, size_t, void* user) {
-                        FPickingQuery* pQuery = static_cast<FPickingQuery*>(user);
-                        float const identity = *((float*)((char*)&pQuery->result.renderable));
-                        pQuery->result.renderable = Entity::import(int32_t(identity));
+                        FPickingQuery* const pQuery = static_cast<FPickingQuery*>(user);
+                        // pQuery->result.renderable already contains the right value!
                         pQuery->result.fragCoords = {
                                 pQuery->x, pQuery->y, float(1.0 - pQuery->result.depth) };
                         pQuery->callback(pQuery->result, pQuery);

diff --git a/filament/src/materials/dof/dof.mat b/filament/src/materials/dof/dof.mat
@@ -477,6 +477,7 @@ void fastTile(inout vec4 color, inout float alpha,
     }
     alpha = cocToAlpha(kernelSize);
     color *= rcp(sampleCount(ringCountFast));
+    color = min(vec4(MEDIUMP_FLT_MAX), color);
 }
 
 void foregroundTile(inout vec4 foreground, inout float fgOpacity,
@@ -658,6 +659,7 @@ void postProcess(inout PostProcessInputs postProcess) {
     // the downside of doing this is that we couldn't use a different upscaler for each
     // layer, but this is a lot less costly
     postProcess.color = foreground + (1.0 - fgOpacity) * background;
+    postProcess.color = min(vec4(MEDIUMP_FLT_MAX), postProcess.color);
     postProcess.alpha = fgOpacity  + (1.0 - fgOpacity) * bgOpacity;
 }
 

diff --git a/filament/test/CMakeLists.txt b/filament/test/CMakeLists.txt
@@ -56,6 +56,14 @@ if (TNT_DEV)
     target_link_libraries(test_depth PRIVATE utils)
 endif()
 
+if (ANDROID)
+    add_executable(test_compiler compiler_test.cpp)
+    target_link_libraries(test_compiler PRIVATE gtest)
+    target_link_libraries(test_compiler PRIVATE utils)
+    target_link_libraries(test_compiler PRIVATE EGL)
+    target_link_libraries(test_compiler PRIVATE GLESv3)
+endif()
+
 add_executable(test_material_parser
         filament_test_material_parser.cpp
         ${RESGEN_SOURCE})