From f506b27a312aebd83a42c4d7f98435b0b8eb69ed Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 26 Jul 2023 16:50:06 -0700 Subject: [PATCH 01/19] gltfio: simple test for asset loading (#6990) --- libs/gltfio/CMakeLists.txt | 36 +++++ libs/gltfio/test/gltfio_test.cpp | 229 +++++++++++++++++++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 libs/gltfio/test/gltfio_test.cpp diff --git a/libs/gltfio/CMakeLists.txt b/libs/gltfio/CMakeLists.txt index d06676e699f..8354d394b71 100644 --- a/libs/gltfio/CMakeLists.txt +++ b/libs/gltfio/CMakeLists.txt @@ -4,6 +4,8 @@ project(gltfio C ASM) set(TARGET gltfio) set(PUBLIC_HDR_DIR include) +set(ROOT_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../..) + # ================================================================================================== # Sources and headers # ================================================================================================== @@ -189,6 +191,40 @@ if (NOT WEBGL AND NOT ANDROID AND NOT IOS) endif() +# ================================================================================================== +# Tests +# ================================================================================================== + +set(GLTF_TEST_FILES) +function(add_test_gltf SOURCE TARGET) + set(source_path "${ROOT_DIR}/${SOURCE}") + set(target_path "${CMAKE_CURRENT_BINARY_DIR}/${TARGET}") + add_custom_command( + OUTPUT ${target_path} + DEPENDS ${source_path} + COMMAND ${CMAKE_COMMAND} -E copy ${source_path} ${target_path} + ) + list(APPEND GLTF_TEST_FILES "${target_path}") + set(GLTF_TEST_FILES ${GLTF_TEST_FILES} PARENT_SCOPE) +endfunction() + +add_test_gltf("third_party/models/AnimatedMorphCube/AnimatedMorphCube.glb" "AnimatedMorphCube.glb") + +add_custom_target(test_gltfio_files DEPENDS ${GLTF_TEST_FILES}) + +# The following tests rely on private APIs that are stripped +# away in Release builds +if (TNT_DEV) + set(TEST_TARGET test_gltfio) + + add_executable(${TEST_TARGET} test/gltfio_test.cpp) + add_dependencies(${TEST_TARGET} test_gltfio_files) + + target_link_libraries(${TEST_TARGET} PRIVATE ${TARGET} filament filabridge gtest uberarchive) + target_compile_options(${TEST_TARGET} PRIVATE -Wno-deprecated-register) + set_target_properties(${TEST_TARGET} PROPERTIES FOLDER Tests) +endif() + # ================================================================================================== # Installation # ================================================================================================== diff --git a/libs/gltfio/test/gltfio_test.cpp b/libs/gltfio/test/gltfio_test.cpp new file mode 100644 index 00000000000..5f6fd2d3c0d --- /dev/null +++ b/libs/gltfio/test/gltfio_test.cpp @@ -0,0 +1,229 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "materials/uberarchive.h" + +#include +#include + +using namespace filament; +using namespace backend; +using namespace gltfio; +using namespace utils; + +constexpr uint32_t WIDTH = 64; +constexpr uint32_t HEIGHT = 64; + +char const* ANIMATED_MORPH_CUBE_GLB = "AnimatedMorphCube.glb"; + +static std::ifstream::pos_type getFileSize(const char* filename) { + std::ifstream in(filename, std::ifstream::ate | std::ifstream::binary); + return in.tellg(); +} + +class glTFData { +public: + glTFData(Path filename, Engine* engine, MaterialProvider* materialProvider, + NameComponentManager* nameManager) + : mAssetLoader(AssetLoader::create({engine, materialProvider, nameManager})), + mResourceLoader(new ResourceLoader({ + engine, filename.getAbsolutePath().c_str(), false, /* normalizeSkinningWeights */ + })), + mStbDecoder(createStbProvider(engine)), mKtxDecoder(createKtx2Provider(engine)) { + mResourceLoader->addTextureProvider("image/png", mStbDecoder); + mResourceLoader->addTextureProvider("image/ktx2", mKtxDecoder); + + long contentSize = static_cast(getFileSize(filename.c_str())); + if (contentSize <= 0) { + std::cerr << "Unable to open " << filename.c_str() << std::endl; + exit(1); + } + + // Consume the glTF file. + std::ifstream in(filename.c_str(), std::ifstream::binary | std::ifstream::in); + std::vector buffer(static_cast(contentSize)); + if (!in.read((char*) buffer.data(), contentSize)) { + std::cerr << "Unable to read " << filename.c_str() << std::endl; + exit(1); + } + + // Parse the glTF file and create Filament entities. + mAsset = mAssetLoader->createAsset(buffer.data(), buffer.size()); + buffer.clear(); + buffer.shrink_to_fit(); + + if (!mAsset) { + std::cerr << "Unable to parse " << filename.c_str() << std::endl; + exit(1); + } + + // Load resources + if (!mResourceLoader->asyncBeginLoad(mAsset)) { + std::cerr << "Unable to start loading resources for " << filename << std::endl; + exit(1); + } + mAsset->releaseSourceData(); + } + + ~glTFData() { + mAssetLoader->destroyAsset(mAsset); + delete mResourceLoader; + delete mStbDecoder; + delete mKtxDecoder; + + AssetLoader::destroy(&mAssetLoader); + } + + FilamentAsset* getAsset() const { return mAsset; } + + AssetLoader* mAssetLoader; + ResourceLoader* mResourceLoader = nullptr; + TextureProvider* mStbDecoder = nullptr; + TextureProvider* mKtxDecoder = nullptr; + FilamentAsset* mAsset = nullptr; +}; + +class glTFIOTest : public testing::Test { +protected: + Engine* mEngine = nullptr; + NameComponentManager* mNameManager = nullptr; + MaterialProvider* mMaterialProvider = nullptr; + + // std::unique_ptr mData; + std::unordered_map> mData; + + void SetUp() override { + mEngine = Engine::Builder().backend(Backend::NOOP).build(); + + mNameManager = new NameComponentManager(EntityManager::get()); + mMaterialProvider = createUbershaderProvider(mEngine, UBERARCHIVE_DEFAULT_DATA, + UBERARCHIVE_DEFAULT_SIZE); + + for (auto fname: {ANIMATED_MORPH_CUBE_GLB}) { + Path gltfFile = Path::getCurrentExecutable().getParent() + Path(fname); + mData[fname] = + std::make_unique(gltfFile, mEngine, mMaterialProvider, mNameManager); + } + } + + void TearDown() override { + mData.clear(); + mMaterialProvider->destroyMaterials(); + Engine::destroy(&mEngine); + + delete mMaterialProvider; + delete mNameManager; + } +}; + +TEST_F(glTFIOTest, AnimatedMorphCubeMaterials) { + FilamentAsset const& morphCubeAsset = *mData[ANIMATED_MORPH_CUBE_GLB]->getAsset(); + Entity const* renderables = morphCubeAsset.getRenderableEntities(); + auto& renderableManager = mEngine->getRenderableManager(); + + auto inst = renderableManager.getInstance(renderables[0]); + auto materialInst = renderableManager.getMaterialInstanceAt(inst, 0); + std::string_view name{materialInst->getName()}; + + EXPECT_EQ(name, "Material"); +} + +TEST_F(glTFIOTest, AnimatedMorphCubeTransforms) { + FilamentAsset const& morphCubeAsset = *mData[ANIMATED_MORPH_CUBE_GLB]->getAsset(); + auto const& transformManager = mEngine->getTransformManager(); + auto const& renderableManager = mEngine->getRenderableManager(); + Entity const* renderables = morphCubeAsset.getRenderableEntities(); + + EXPECT_EQ(morphCubeAsset.getRenderableEntityCount(), 1u); + + EXPECT_TRUE(transformManager.hasComponent(renderables[0])); + + auto const inst = transformManager.getInstance(renderables[0]); + math::mat4f const transform = transformManager.getTransform(inst); + math::mat4f const expectedTransform = composeMatrix(math::float3{0.0, 0.0, 0.0}, + math::quatf{0.0, 0.0, 0.7071067, -0.7071068}, math::float3{100.0, 100.0, 100.0}); + + auto const result = inverse(transform) * expectedTransform; + + // We expect the result to be identity + EXPECT_EQ(result, math::mat4f{}); +} + +TEST_F(glTFIOTest, AnimatedMorphCubeRenderables) { + FilamentAsset const& morphCubeAsset = *mData[ANIMATED_MORPH_CUBE_GLB]->getAsset(); + Entity const* renderables = morphCubeAsset.getRenderableEntities(); + auto const& renderableManager = mEngine->getRenderableManager(); + + EXPECT_EQ(morphCubeAsset.getRenderableEntityCount(), 1u); + + EXPECT_TRUE(renderableManager.hasComponent(renderables[0])); + auto const inst = renderableManager.getInstance(renderables[0]); + EXPECT_EQ(renderableManager.getPrimitiveCount(inst), 1u); + AttributeBitset const attribs = renderableManager.getEnabledAttributesAt(inst, 0); + + EXPECT_TRUE(attribs[VertexAttribute::POSITION]); + EXPECT_TRUE(attribs[VertexAttribute::TANGENTS]); + if (mMaterialProvider->needsDummyData(VertexAttribute::COLOR)) { + EXPECT_TRUE(attribs[VertexAttribute::COLOR]); + } else { + EXPECT_FALSE(attribs[VertexAttribute::COLOR]); + } + if (mMaterialProvider->needsDummyData(VertexAttribute::UV0)) { + EXPECT_TRUE(attribs[VertexAttribute::UV0]); + } else { + EXPECT_FALSE(attribs[VertexAttribute::UV0]); + } + if (mMaterialProvider->needsDummyData(VertexAttribute::UV1)) { + EXPECT_TRUE(attribs[VertexAttribute::UV1]); + } else { + EXPECT_FALSE(attribs[VertexAttribute::UV1]); + } + + // The AnimatedMorphCube has two morph targets: "thin" and "angle" + EXPECT_EQ(renderableManager.getMorphTargetCount(inst), 2u); + + // The 0-th MorphTargetBuffer holds both of the targets + auto const morphTargetBuffer = renderableManager.getMorphTargetBufferAt(inst, 0, 0); + EXPECT_EQ(morphTargetBuffer->getCount(), 2u); + + // The number of vertices for the morph target should be the face vertices in a cube => + // (6 faces * 4 vertices per face) = 24 vertices + EXPECT_EQ(morphTargetBuffer->getVertexCount(), 24u); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From b35e24daa7bbe48a56af990de304eac4eef1a671 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 26 Jul 2023 19:11:50 -0700 Subject: [PATCH 02/19] gltfio: exclude unsupported platforms from test (#7000) --- libs/gltfio/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/gltfio/CMakeLists.txt b/libs/gltfio/CMakeLists.txt index 8354d394b71..66b8db65de4 100644 --- a/libs/gltfio/CMakeLists.txt +++ b/libs/gltfio/CMakeLists.txt @@ -214,7 +214,7 @@ add_custom_target(test_gltfio_files DEPENDS ${GLTF_TEST_FILES}) # The following tests rely on private APIs that are stripped # away in Release builds -if (TNT_DEV) +if (TNT_DEV AND NOT WEBGL AND NOT ANDROID AND NOT IOS) set(TEST_TARGET test_gltfio) add_executable(${TEST_TARGET} test/gltfio_test.cpp) From e3568cd89f8a527ef0d73ab134f13ccc821c60d0 Mon Sep 17 00:00:00 2001 From: Mirsfang Date: Fri, 28 Jul 2023 00:34:41 +0800 Subject: [PATCH 03/19] fix macos openggl compile process_ARB_shading_language_packing type conversion error (#6994) --- .../backend/src/opengl/ShaderCompilerService.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp index cef01163b5f..5e301d83cb4 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.cpp +++ b/filament/backend/src/opengl/ShaderCompilerService.cpp @@ -661,8 +661,8 @@ float u16tofp32(highp uint v) { v <<= 16u; highp uint s = v & 0x80000000u; highp uint n = v & 0x7FFFFFFFu; - highp uint nz = n == 0u ? 0u : 0xFFFFFFFF; - return uintBitsToFloat(s | ((((n >> 3u) + (0x70u << 23))) & nz)); + highp uint nz = (n == 0u) ? 0u : 0xFFFFFFFFu; + return uintBitsToFloat(s | ((((n >> 3u) + (0x70u << 23u))) & nz)); } vec2 unpackHalf2x16(highp uint v) { return vec2(u16tofp32(v&0xFFFFu), u16tofp32(v>>16u)); @@ -670,11 +670,11 @@ vec2 unpackHalf2x16(highp uint v) { uint fp32tou16(float val) { uint f32 = floatBitsToUint(val); uint f16 = 0u; - uint sign = (f32 >> 16) & 0x8000u; - int exponent = int((f32 >> 23) & 0xFFu) - 127; + uint sign = (f32 >> 16u) & 0x8000u; + int exponent = int((f32 >> 23u) & 0xFFu) - 127; uint mantissa = f32 & 0x007FFFFFu; if (exponent > 15) { - f16 = sign | (0x1Fu << 10); + f16 = sign | (0x1Fu << 10u); } else if (exponent > -15) { exponent += 15; mantissa >>= 13; @@ -687,7 +687,7 @@ uint fp32tou16(float val) { highp uint packHalf2x16(vec2 v) { highp uint x = fp32tou16(v.x); highp uint y = fp32tou16(v.y); - return (y << 16) | x; + return (y << 16u) | x; } )"sv; } From 03b8dc802777c15a27dd7960847cf64982510baa Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 26 Jul 2023 16:50:39 -0700 Subject: [PATCH 04/19] The "back" key will now terminate the gltf_viewer activity This is useful for testing our shutdown code. --- .../java/com/google/android/filament/gltf/MainActivity.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt b/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt index 8291f020406..06c9f141cd1 100644 --- a/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt +++ b/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt @@ -337,6 +337,11 @@ class MainActivity : Activity() { remoteServer?.close() } + override fun onBackPressed() { + super.onBackPressed() + finish() + } + fun loadModelData(message: RemoteServer.ReceivedMessage) { Log.i(TAG, "Downloaded model ${message.label} (${message.buffer.capacity()} bytes)") clearStatusText() From b1491ae5b1ed968aab306355e3b810418586f6f4 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 26 Jul 2023 16:44:09 -0700 Subject: [PATCH 05/19] Disable timer queries on all Mali GPUs fixes b/233754398 --- NEW_RELEASE_NOTES.md | 1 + filament/backend/src/opengl/OpenGLContext.cpp | 24 +++++-------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 4a1a9c7fa7e..3e0b96ec755 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,3 +7,4 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut +- backend: Disable timer queries on all Mali GPUs (fixes b/233754398) diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp index 07a6d7ce8cd..f3d9300eb06 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -268,25 +268,13 @@ OpenGLContext::OpenGLContext() noexcept { bugs.dont_use_timer_query = true; } if (strstr(state.renderer, "Mali-G")) { - // assume we don't have working timer queries + // We have run into several problems with timer queries on Mali-Gxx: + // - timer queries seem to cause memory corruptions in some cases on some devices + // (see b/233754398) + // - appeared at least in: "OpenGL ES 3.2 v1.r26p0-01eac0" + // - wasn't present in: "OpenGL ES 3.2 v1.r32p1-00pxl1" + // - timer queries sometime crash with an NPE (see b/273759031) bugs.dont_use_timer_query = true; - - int maj, min, driverVersion, driverRevision, driverPatch; - int const c = sscanf(state.version, "OpenGL ES %d.%d v%d.r%dp%d", // NOLINT(cert-err34-c) - &maj, &min, &driverVersion, &driverRevision, &driverPatch); - if (c == 5) { - // Workarounds based on version here. - // notes: - // bugs.dont_use_timer_query : on some Mali-Gxx drivers timer query seems - // to cause memory corruptions in some cases on some devices (see b/233754398). - // - appeared at least in - // "OpenGL ES 3.2 v1.r26p0-01eac0" - // - wasn't present in - // "OpenGL ES 3.2 v1.r32p1-00pxl1" - if (driverVersion >= 2 || (driverVersion == 1 && driverRevision >= 32)) { - bugs.dont_use_timer_query = false; - } - } } // Mali seems to have no problem with this (which is good for us) bugs.allow_read_only_ancillary_feedback_loop = true; From 0ed71ab53bf93d0fd86cc8eb1f22f5a3fc8a2d6a Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 24 Jul 2023 10:46:21 -0700 Subject: [PATCH 06/19] fix an issue causing callbacks to be called too late We were waiting for programs from both queues to be compiled before calling the callback associated with one queue. In practice this caused the callback associated with high priority programs to be called only after low priority programs were ready. Also cleanup-up "token" so that it doesn't store the priority. Update the documentation and sample to better reflect what the implementation does. --- .../src/opengl/ShaderCompilerService.cpp | 39 ++++++++++++------- .../src/opengl/ShaderCompilerService.h | 8 ++-- .../src/opengl/platforms/PlatformCocoaGL.mm | 3 ++ filament/include/filament/Material.h | 8 ++++ libs/gltfio/src/ArchiveCache.cpp | 30 +++++++------- 5 files changed, 57 insertions(+), 31 deletions(-) diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp index 5e301d83cb4..4542f252d4e 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.cpp +++ b/filament/backend/src/opengl/ShaderCompilerService.cpp @@ -87,7 +87,6 @@ struct ShaderCompilerService::ProgramToken { BlobCacheKey key; std::future binary; - CompilerPriorityQueue priorityQueue = CompilerPriorityQueue::HIGH; bool canceled = false; }; @@ -148,12 +147,23 @@ void ShaderCompilerService::CompilerThreadPool::init( } } +auto ShaderCompilerService::CompilerThreadPool::find( + program_token_t const& token) -> std::pair { + for (auto&& q: mQueues) { + auto pos = std::find_if(q.begin(), q.end(), [&token](auto&& item) { + return item.first == token; + }); + if (pos != q.end()) { + return { q, pos }; + } + } + // this can happen if the program is being processed right now + return { mQueues[0], mQueues[0].end() }; +} + auto ShaderCompilerService::CompilerThreadPool::dequeue(program_token_t const& token) -> Job { - auto& q = mQueues[size_t(token->priorityQueue)]; - auto pos = std::find_if(q.begin(), q.end(), [&token](auto&& item) { - return item.first == token; - }); Job job; + auto&& [q, pos] = find(token); if (pos != q.end()) { std::swap(job, pos->second); q.erase(pos); @@ -169,9 +179,10 @@ void ShaderCompilerService::CompilerThreadPool::makeUrgent(program_token_t const mQueueCondition.notify_one(); } -void ShaderCompilerService::CompilerThreadPool::queue(program_token_t const& token, Job&& job) { +void ShaderCompilerService::CompilerThreadPool::queue(CompilerPriorityQueue priorityQueue, + program_token_t const& token, Job&& job) { std::unique_lock const lock(mQueueLock); - mQueues[size_t(token->priorityQueue)].emplace_back(token, std::move(job)); + mQueues[size_t(priorityQueue)].emplace_back(token, std::move(job)); mQueueCondition.notify_one(); } @@ -204,7 +215,9 @@ void ShaderCompilerService::init() noexcept { // also glProgramBinary blocks if other threads are compiling. // - on Mali shader compilation can be multithreaded, but program linking happens on // a single service thread, so we don't bother using more than one thread either. - // - on desktop we could use more threads, tbd. + // - on macOS (M1 MacBook Pro/Ventura) there is global lock around all GL APIs when using + // a shared context, so parallel shader compilation yields no benefit. + // - on windows/linux we could use more threads, tbd. if (mDriver.mPlatform.isExtraContextSupported()) { mShaderCompilerThreadCount = 1; mCompilerThreadPool.init(mUseSharedContext, @@ -230,13 +243,13 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( token->gl.program = OpenGLBlobCache::retrieve(&token->key, mDriver.mPlatform, program); 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 promise; token->binary = promise.get_future(); - token->priorityQueue = program.getPriorityQueue(); // queue a compile job - mCompilerThreadPool.queue(token, + mCompilerThreadPool.queue(priorityQueue, token, [this, &gl, promise = std::move(promise), program = std::move(program), token]() mutable { @@ -304,7 +317,7 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( } - runAtNextTick(token->priorityQueue, token, [this, token]() { + runAtNextTick(priorityQueue, token, [this, token]() { if (mShaderCompilerThreadCount) { if (!token->gl.program) { // TODO: see if we could completely eliminate this callback here @@ -431,8 +444,8 @@ void ShaderCompilerService::notifyWhenAllProgramsAreReady(CompilerPriorityQueue // list all programs up to this point, both low and high priority utils::FixedCapacityVector, false> tokens; tokens.reserve(mRunAtNextTickOps.size()); - for (auto& [priority_, token, fn_] : mRunAtNextTickOps) { - if (token) { + for (auto& [itemPriority, token, fn] : mRunAtNextTickOps) { + if (token && fn && itemPriority == priority) { tokens.push_back(token); } } diff --git a/filament/backend/src/opengl/ShaderCompilerService.h b/filament/backend/src/opengl/ShaderCompilerService.h index 833636965cc..3fc37abf5b5 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.h +++ b/filament/backend/src/opengl/ShaderCompilerService.h @@ -96,17 +96,19 @@ class ShaderCompilerService { using Job = utils::Invocable; void init(bool useSharedContexts, uint32_t threadCount, OpenGLPlatform& platform) noexcept; void exit() noexcept; - void queue(program_token_t const& token, Job&& job); + void queue(CompilerPriorityQueue priorityQueue, program_token_t const& token, Job&& job); void makeUrgent(program_token_t const& token); private: + using Queue = std::deque>; std::vector mCompilerThreads; std::atomic_bool mExitRequested{ false }; std::mutex mQueueLock; std::condition_variable mQueueCondition; - std::array>, 2> mQueues; - Job mUrgentJob; + std::array mQueues; + Job mUrgentJob; // needs mQueueLock as well Job dequeue(program_token_t const& token); // lock must be held + std::pair find(program_token_t const& token); }; OpenGLDriver& mDriver; diff --git a/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm b/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm index 195d8d17e70..b19ec4cbbf4 100644 --- a/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm +++ b/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm @@ -173,6 +173,9 @@ } bool PlatformCocoaGL::isExtraContextSupported() const noexcept { + // macOS supports shared contexts however, it looks like the implementation uses a global + // lock around all GL APIs. It's a problem for API calls that take a long time to execute, + // one such call is e.g.: glCompileProgram. return true; } diff --git a/filament/include/filament/Material.h b/filament/include/filament/Material.h index 5b9314a042d..04d4039cb0c 100644 --- a/filament/include/filament/Material.h +++ b/filament/include/filament/Material.h @@ -166,6 +166,14 @@ class UTILS_PUBLIC Material : public FilamentAPI { * many previous frames are enqueued in the backend. This also varies by backend. Therefore, * it is recommended to only call this method once per material shortly after creation. * + * If the same variant is scheduled for compilation multiple times, the first scheduling + * takes precedence; later scheduling are ignored. + * + * caveat: A consequence is that if a variant is scheduled on the low priority queue and later + * scheduled again on the high priority queue, the later scheduling is ignored. + * Therefore, the second callback could be called before the variant is compiled. + * However, the first callback, if specified, will trigger as expected. + * * @param priority Which priority queue to use, LOW or HIGH. * @param variants Variants to include to the compile command. * @param handler Handler to dispatch the callback or nullptr for the default handler diff --git a/libs/gltfio/src/ArchiveCache.cpp b/libs/gltfio/src/ArchiveCache.cpp index 118beec0fdc..47798741fa3 100644 --- a/libs/gltfio/src/ArchiveCache.cpp +++ b/libs/gltfio/src/ArchiveCache.cpp @@ -118,22 +118,22 @@ Material* ArchiveCache::getMaterial(const ArchiveRequirements& reqs) { mMaterials[i] = Material::Builder() .package(spec.package, spec.packageByteCount) .build(mEngine); - } - // Don't attempt to precompile shaders on WebGL. - // Chrome already suffers from slow shader compilation: - // https://github.com/google/filament/issues/6615 - // Precompiling shaders exacerbates the problem. -#if !defined(__EMSCRIPTEN__) - // compile everything at low priority - mMaterials[i]->compile(Material::CompilerPriorityQueue::LOW); - - // promote variants we care about to high priority - mMaterials[i]->compile(Material::CompilerPriorityQueue::HIGH, - UserVariantFilterBit::DIRECTIONAL_LIGHTING | - UserVariantFilterBit::DYNAMIC_LIGHTING | - UserVariantFilterBit::SHADOW_RECEIVER); -#endif + // Don't attempt to precompile shaders on WebGL. + // Chrome already suffers from slow shader compilation: + // https://github.com/google/filament/issues/6615 + // Precompiling shaders exacerbates the problem. + #if !defined(__EMSCRIPTEN__) + // First compile high priority variants + mMaterials[i]->compile(Material::CompilerPriorityQueue::HIGH, + UserVariantFilterBit::DIRECTIONAL_LIGHTING | + UserVariantFilterBit::DYNAMIC_LIGHTING | + UserVariantFilterBit::SHADOW_RECEIVER); + + // and then, everything else at low priority + mMaterials[i]->compile(Material::CompilerPriorityQueue::LOW); + #endif + } return mMaterials[i]; } From 3dbb7298f8ae23c1b6c18cbea9d9dbffda3f7027 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 24 Jul 2023 15:03:10 -0700 Subject: [PATCH 07/19] fix a cleanup of material parallel compilation When the engine is shut down, it's possible for some parallel compilation jobs (and callbacks) to be queued. We need to make sure to clear the queues and call the callbacks before destroying the parallel compilation service. Fixes b/290388359 --- filament/backend/src/Driver.cpp | 2 + filament/backend/src/opengl/OpenGLDriver.cpp | 4 +- .../src/opengl/ShaderCompilerService.cpp | 66 +++++++++++++------ .../src/opengl/ShaderCompilerService.h | 23 ++++++- filament/include/filament/Material.h | 6 ++ filament/src/details/Engine.cpp | 2 +- 6 files changed, 77 insertions(+), 26 deletions(-) diff --git a/filament/backend/src/Driver.cpp b/filament/backend/src/Driver.cpp index 7501ae48d26..b3bbab4a6ca 100644 --- a/filament/backend/src/Driver.cpp +++ b/filament/backend/src/Driver.cpp @@ -63,6 +63,8 @@ DriverBase::DriverBase() noexcept { } DriverBase::~DriverBase() noexcept { + assert_invariant(mCallbacks.empty()); + assert_invariant(mServiceThreadCallbackQueue.empty()); if constexpr (UTILS_HAS_THREADING) { // quit our service thread std::unique_lock lock(mServiceThreadLock); diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index fac8bc7d4a9..5b1ff835688 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -231,6 +231,8 @@ void OpenGLDriver::terminate() { // wait for the GPU to finish executing all commands glFinish(); + mShaderCompilerService.terminate(); + // and make sure to execute all the GpuCommandCompleteOps callbacks executeGpuCommandsCompleteOps(); @@ -249,8 +251,6 @@ void OpenGLDriver::terminate() { delete mTimerQueryImpl; - mShaderCompilerService.terminate(); - mPlatform.terminate(); } diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp index 4542f252d4e..59b85d134db 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.cpp +++ b/filament/backend/src/opengl/ShaderCompilerService.cpp @@ -100,6 +100,14 @@ void* ShaderCompilerService::getUserData(const program_token_t& token) noexcept // ------------------------------------------------------------------------------------------------ +ShaderCompilerService::CompilerThreadPool::CompilerThreadPool() noexcept = default; + +ShaderCompilerService::CompilerThreadPool::~CompilerThreadPool() noexcept { + assert_invariant(mCompilerThreads.empty()); + assert_invariant(mQueues[0].empty()); + assert_invariant(mQueues[1].empty()); +} + void ShaderCompilerService::CompilerThreadPool::init( bool useSharedContexts, uint32_t threadCount, OpenGLPlatform& platform) noexcept { @@ -186,16 +194,23 @@ void ShaderCompilerService::CompilerThreadPool::queue(CompilerPriorityQueue prio mQueueCondition.notify_one(); } -void ShaderCompilerService::CompilerThreadPool::exit() noexcept { +void ShaderCompilerService::CompilerThreadPool::terminate() noexcept { std::unique_lock lock(mQueueLock); mExitRequested = true; mQueueCondition.notify_all(); lock.unlock(); + for (auto& thread: mCompilerThreads) { if (thread.joinable()) { thread.join(); } } + mCompilerThreads.clear(); + + // Clear all the queues, dropping the remaining jobs. This relies on the jobs being cancelable. + for (auto&& q : mQueues) { + q.clear(); + } } // ------------------------------------------------------------------------------------------------ @@ -227,8 +242,16 @@ void ShaderCompilerService::init() noexcept { } void ShaderCompilerService::terminate() noexcept { - // FIXME: could we have some user callbacks pending here? - mCompilerThreadPool.exit(); + mCompilerThreadPool.terminate(); + + // We could have some pending callbacks here, we need to execute them + for (auto&& op: mRunAtNextTickOps) { + Job const& job = std::get<2>(op); + if (job.callback) { + mDriver.scheduleCallback(job.handler, job.user, job.callback); + } + } + mRunAtNextTickOps.clear(); } ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( @@ -317,7 +340,7 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( } - runAtNextTick(priorityQueue, token, [this, token]() { + runAtNextTick(priorityQueue, token, [this, token](Job const&) { if (mShaderCompilerThreadCount) { if (!token->gl.program) { // TODO: see if we could completely eliminate this callback here @@ -442,16 +465,19 @@ void ShaderCompilerService::notifyWhenAllProgramsAreReady(CompilerPriorityQueue if (KHR_parallel_shader_compile || mShaderCompilerThreadCount) { // list all programs up to this point, both low and high priority - utils::FixedCapacityVector, false> tokens; - tokens.reserve(mRunAtNextTickOps.size()); - for (auto& [itemPriority, token, fn] : mRunAtNextTickOps) { - if (token && fn && itemPriority == priority) { + + using TokenVector = utils::FixedCapacityVector< + program_token_t, std::allocator, false>; + TokenVector tokens{ TokenVector::with_capacity(mRunAtNextTickOps.size()) }; + + for (auto& [itemPriority, token, job] : mRunAtNextTickOps) { + if (token && job.fn && itemPriority == priority) { tokens.push_back(token); } } - runAtNextTick(priority, nullptr, - [this, tokens = std::move(tokens), handler, user, callback]() { + runAtNextTick(priority, nullptr, { + [this, tokens = std::move(tokens)](Job const& job) { for (auto const& token : tokens) { assert_invariant(token); if (!isProgramReady(token)) { @@ -459,23 +485,23 @@ void ShaderCompilerService::notifyWhenAllProgramsAreReady(CompilerPriorityQueue return false; } } - if (callback) { + if (job.callback) { // all programs are ready, we can call the callbacks - mDriver.scheduleCallback(handler, user, callback); + mDriver.scheduleCallback(job.handler, job.user, job.callback); } // and we're done return true; - }); + }, handler, user, callback }); return; } // we don't have KHR_parallel_shader_compile - runAtNextTick(priority, nullptr, [this, handler, user, callback]() { - mDriver.scheduleCallback(handler, user, callback); + runAtNextTick(priority, nullptr, {[this](Job const& job) { + mDriver.scheduleCallback(job.handler, job.user, job.callback); return true; - }); + }, handler, user, callback }); // TODO: we could spread the compiles over several frames, the tick() below then is not // needed here. We keep it for now as to not change the current behavior too much. @@ -761,14 +787,14 @@ GLuint ShaderCompilerService::linkProgram(OpenGLContext& context, // ------------------------------------------------------------------------------------------------ void ShaderCompilerService::runAtNextTick(CompilerPriorityQueue priority, - const program_token_t& token, std::function fn) noexcept { + const program_token_t& token, Job job) noexcept { // insert items in order of priority and at the end of the range auto& ops = mRunAtNextTickOps; auto const pos = std::lower_bound(ops.begin(), ops.end(), priority, [](ContainerType const& lhs, CompilerPriorityQueue priorityQueue) { return std::get<0>(lhs) < priorityQueue; }); - ops.emplace(pos, priority, token, std::move(fn)); + ops.emplace(pos, priority, token, std::move(job)); SYSTRACE_CONTEXT(); SYSTRACE_VALUE32("ShaderCompilerService Jobs", mRunAtNextTickOps.size()); @@ -792,8 +818,8 @@ void ShaderCompilerService::executeTickOps() noexcept { auto& ops = mRunAtNextTickOps; auto it = ops.begin(); while (it != ops.end()) { - auto fn = std::get<2>(*it); - bool const remove = fn(); + Job const& job = std::get<2>(*it); + bool const remove = job.fn(job); if (remove) { it = ops.erase(it); } else { diff --git a/filament/backend/src/opengl/ShaderCompilerService.h b/filament/backend/src/opengl/ShaderCompilerService.h index 3fc37abf5b5..8f42dfed6b2 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.h +++ b/filament/backend/src/opengl/ShaderCompilerService.h @@ -33,6 +33,7 @@ #include #include #include +#include #include namespace filament::backend { @@ -93,9 +94,11 @@ class ShaderCompilerService { private: class CompilerThreadPool { public: + CompilerThreadPool() noexcept; + ~CompilerThreadPool() noexcept; using Job = utils::Invocable; void init(bool useSharedContexts, uint32_t threadCount, OpenGLPlatform& platform) noexcept; - void exit() noexcept; + void terminate() noexcept; void queue(CompilerPriorityQueue priorityQueue, program_token_t const& token, Job&& job); void makeUrgent(program_token_t const& token); @@ -145,12 +148,26 @@ class ShaderCompilerService { static bool checkProgramStatus(program_token_t const& token) noexcept; + struct Job { + template + Job(FUNC&& fn) : fn(std::forward(fn)) {} + Job(std::function fn, + CallbackHandler* handler, void* user, CallbackHandler::Callback callback) + : fn(std::move(fn)), handler(handler), user(user), callback(callback) { + } + std::function fn; + CallbackHandler* handler = nullptr; + void* user = nullptr; + CallbackHandler::Callback callback{}; + }; + void runAtNextTick(CompilerPriorityQueue priority, - const program_token_t& token, std::function fn) noexcept; + const program_token_t& token, Job job) noexcept; void executeTickOps() noexcept; void cancelTickOp(program_token_t token) noexcept; // order of insertion is important - using ContainerType = std::tuple>; + + using ContainerType = std::tuple; std::vector mRunAtNextTickOps; }; diff --git a/filament/include/filament/Material.h b/filament/include/filament/Material.h index 04d4039cb0c..06988983c4d 100644 --- a/filament/include/filament/Material.h +++ b/filament/include/filament/Material.h @@ -174,6 +174,12 @@ class UTILS_PUBLIC Material : public FilamentAPI { * Therefore, the second callback could be called before the variant is compiled. * However, the first callback, if specified, will trigger as expected. * + * The callback is guaranteed to be called. If the engine is destroyed while some material + * variants are still compiling or in the queue, these will be discarded and the corresponding + * callback will be called. In that case however the Material pointer passed to the callback + * is guaranteed to be invalid (either because it's been destroyed by the user already, or, + * because it's been cleaned-up by the Engine). + * * @param priority Which priority queue to use, LOW or HIGH. * @param variants Variants to include to the compile command. * @param handler Handler to dispatch the callback or nullptr for the default handler diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index 3206d1ec702..4ebb359ef01 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -465,7 +465,7 @@ void FEngine::shutdown() { getDriverApi().terminate(); } else { mDriverThread.join(); - + // Driver::terminate() has been called here. } // Finally, call user callbacks that might have been scheduled. From d3025256749501b6094693daecfa663e3b94bc73 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 24 Jul 2023 15:42:01 -0700 Subject: [PATCH 08/19] Add a way to query the validity of filament objects Engine::isValid() can be used to check the validity of most filament objects. --- NEW_RELEASE_NOTES.md | 2 + .../filament-android/src/main/cpp/Engine.cpp | 106 ++++++++++++ .../com/google/android/filament/Engine.java | 159 +++++++++++++++++- filament/include/filament/Engine.h | 19 +++ filament/src/Engine.cpp | 55 ++++++ filament/src/ResourceList.cpp | 5 +- filament/src/ResourceList.h | 5 + filament/src/details/Engine.cpp | 79 +++++++++ filament/src/details/Engine.h | 23 +++ web/filament-js/jsbindings.cpp | 48 +++++- 10 files changed, 495 insertions(+), 6 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 3e0b96ec755..77176462ef1 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -8,3 +8,5 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut - backend: Disable timer queries on all Mali GPUs (fixes b/233754398) +- engine: Add a way to query the validity of most filament objects (see `Engine::isValid`) +- opengl: fix b/290388359 : possible crash when shutting down the engine diff --git a/android/filament-android/src/main/cpp/Engine.cpp b/android/filament-android/src/main/cpp/Engine.cpp index 3d720a88a3f..e530e7bb55b 100644 --- a/android/filament-android/src/main/cpp/Engine.cpp +++ b/android/filament-android/src/main/cpp/Engine.cpp @@ -278,6 +278,112 @@ Java_com_google_android_filament_Engine_nDestroyEntity(JNIEnv*, jclass, engine->destroy(entity); } + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidRenderer(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeRenderer) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((Renderer*)nativeRenderer); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidView(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeView) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((View*)nativeView); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidScene(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeScene) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((Scene*)nativeScene); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidFence(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeFence) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((Fence*)nativeFence); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidStream(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeStream) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((Stream*)nativeStream); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidIndexBuffer(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeIndexBuffer) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((IndexBuffer*)nativeIndexBuffer); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidVertexBuffer(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeVertexBuffer) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((VertexBuffer*)nativeVertexBuffer); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidSkinningBuffer(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeSkinningBuffer) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((SkinningBuffer*)nativeSkinningBuffer); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidIndirectLight(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeIndirectLight) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((IndirectLight*)nativeIndirectLight); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidMaterial(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeMaterial) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((Material*)nativeMaterial); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidSkybox(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeSkybox) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((Skybox*)nativeSkybox); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidColorGrading(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeColorGrading) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((ColorGrading*)nativeColorGrading); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidTexture(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeTexture) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((Texture*)nativeTexture); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidRenderTarget(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeTarget) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((RenderTarget*)nativeTarget); +} + +extern "C" JNIEXPORT jboolean JNICALL +Java_com_google_android_filament_Engine_nIsValidSwapChain(JNIEnv*, jclass, + jlong nativeEngine, jlong nativeSwapChain) { + Engine* engine = (Engine *)nativeEngine; + return (jboolean)engine->isValid((SwapChain*)nativeSwapChain); +} + extern "C" JNIEXPORT void JNICALL Java_com_google_android_filament_Engine_nFlushAndWait(JNIEnv*, jclass, jlong nativeEngine) { diff --git a/android/filament-android/src/main/java/com/google/android/filament/Engine.java b/android/filament-android/src/main/java/com/google/android/filament/Engine.java index c047c96224a..e6bc4d92430 100644 --- a/android/filament-android/src/main/java/com/google/android/filament/Engine.java +++ b/android/filament-android/src/main/java/com/google/android/filament/Engine.java @@ -449,6 +449,141 @@ public void destroySwapChain(@NonNull SwapChain swapChain) { swapChain.clearNativeObject(); } + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidRenderer(@NonNull Renderer object) { + return nIsValidRenderer(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidView(@NonNull View object) { + return nIsValidView(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidScene(@NonNull Scene object) { + return nIsValidScene(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidFence(@NonNull Fence object) { + return nIsValidFence(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidStream(@NonNull Stream object) { + return nIsValidStream(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidIndexBuffer(@NonNull IndexBuffer object) { + return nIsValidIndexBuffer(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidVertexBuffer(@NonNull VertexBuffer object) { + return nIsValidVertexBuffer(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidSkinningBuffer(@NonNull SkinningBuffer object) { + return nIsValidSkinningBuffer(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidIndirectLight(@NonNull IndirectLight object) { + return nIsValidIndirectLight(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidMaterial(@NonNull Material object) { + return nIsValidMaterial(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidSkybox(@NonNull Skybox object) { + return nIsValidSkybox(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidColorGrading(@NonNull ColorGrading object) { + return nIsValidColorGrading(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidTexture(@NonNull Texture object) { + return nIsValidTexture(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidRenderTarget(@NonNull RenderTarget object) { + return nIsValidRenderTarget(getNativeObject(), object.getNativeObject()); + } + + /** + * Returns whether the object is valid. + * @param object Object to check for validity + * @return returns true if the specified object is valid. + */ + public boolean isValidSwapChain(@NonNull SwapChain object) { + return nIsValidSwapChain(getNativeObject(), object.getNativeObject()); + } + // View /** @@ -785,17 +920,17 @@ private static void assertDestroy(boolean success) { private static native long nCreateSwapChain(long nativeEngine, Object nativeWindow, long flags); private static native long nCreateSwapChainHeadless(long nativeEngine, int width, int height, long flags); private static native long nCreateSwapChainFromRawPointer(long nativeEngine, long pointer, long flags); - private static native boolean nDestroySwapChain(long nativeEngine, long nativeSwapChain); private static native long nCreateView(long nativeEngine); - private static native boolean nDestroyView(long nativeEngine, long nativeView); private static native long nCreateRenderer(long nativeEngine); - private static native boolean nDestroyRenderer(long nativeEngine, long nativeRenderer); private static native long nCreateCamera(long nativeEngine, int entity); private static native long nGetCameraComponent(long nativeEngine, int entity); private static native void nDestroyCameraComponent(long nativeEngine, int entity); private static native long nCreateScene(long nativeEngine); - private static native boolean nDestroyScene(long nativeEngine, long nativeScene); private static native long nCreateFence(long nativeEngine); + + private static native boolean nDestroyRenderer(long nativeEngine, long nativeRenderer); + private static native boolean nDestroyView(long nativeEngine, long nativeView); + private static native boolean nDestroyScene(long nativeEngine, long nativeScene); private static native boolean nDestroyFence(long nativeEngine, long nativeFence); private static native boolean nDestroyStream(long nativeEngine, long nativeStream); private static native boolean nDestroyIndexBuffer(long nativeEngine, long nativeIndexBuffer); @@ -808,6 +943,22 @@ private static void assertDestroy(boolean success) { private static native boolean nDestroyColorGrading(long nativeEngine, long nativeColorGrading); private static native boolean nDestroyTexture(long nativeEngine, long nativeTexture); private static native boolean nDestroyRenderTarget(long nativeEngine, long nativeTarget); + private static native boolean nDestroySwapChain(long nativeEngine, long nativeSwapChain); + private static native boolean nIsValidRenderer(long nativeEngine, long nativeRenderer); + private static native boolean nIsValidView(long nativeEngine, long nativeView); + private static native boolean nIsValidScene(long nativeEngine, long nativeScene); + private static native boolean nIsValidFence(long nativeEngine, long nativeFence); + private static native boolean nIsValidStream(long nativeEngine, long nativeStream); + private static native boolean nIsValidIndexBuffer(long nativeEngine, long nativeIndexBuffer); + private static native boolean nIsValidVertexBuffer(long nativeEngine, long nativeVertexBuffer); + private static native boolean nIsValidSkinningBuffer(long nativeEngine, long nativeSkinningBuffer); + private static native boolean nIsValidIndirectLight(long nativeEngine, long nativeIndirectLight); + private static native boolean nIsValidMaterial(long nativeEngine, long nativeMaterial); + private static native boolean nIsValidSkybox(long nativeEngine, long nativeSkybox); + private static native boolean nIsValidColorGrading(long nativeEngine, long nativeColorGrading); + private static native boolean nIsValidTexture(long nativeEngine, long nativeTexture); + private static native boolean nIsValidRenderTarget(long nativeEngine, long nativeTarget); + private static native boolean nIsValidSwapChain(long nativeEngine, long nativeSwapChain); private static native void nDestroyEntity(long nativeEngine, int entity); private static native void nFlushAndWait(long nativeEngine); private static native long nGetTransformManager(long nativeEngine); diff --git a/filament/include/filament/Engine.h b/filament/include/filament/Engine.h index 3795b89fb54..e4d601b3cde 100644 --- a/filament/include/filament/Engine.h +++ b/filament/include/filament/Engine.h @@ -676,6 +676,25 @@ class UTILS_PUBLIC Engine { bool destroy(const InstanceBuffer* p); //!< Destroys an InstanceBuffer object. void destroy(utils::Entity e); //!< Destroys all filament-known components from this entity + bool isValid(const BufferObject* p); //!< Tells whether a BufferObject object is valid + bool isValid(const VertexBuffer* p); //!< Tells whether an VertexBuffer object is valid + bool isValid(const Fence* p); //!< Tells whether a Fence object is valid + bool isValid(const IndexBuffer* p); //!< Tells whether an IndexBuffer object is valid + bool isValid(const SkinningBuffer* p); //!< Tells whether a SkinningBuffer object is valid + bool isValid(const MorphTargetBuffer* p); //!< Tells whether a MorphTargetBuffer object is valid + bool isValid(const IndirectLight* p); //!< Tells whether an IndirectLight object is valid + bool isValid(const Material* p); //!< Tells whether an IndirectLight object is valid + bool isValid(const Renderer* p); //!< Tells whether a Renderer object is valid + bool isValid(const Scene* p); //!< Tells whether a Scene object is valid + bool isValid(const Skybox* p); //!< Tells whether a SkyBox object is valid + bool isValid(const ColorGrading* p); //!< Tells whether a ColorGrading object is valid + bool isValid(const SwapChain* p); //!< Tells whether a SwapChain object is valid + bool isValid(const Stream* p); //!< Tells whether a Stream object is valid + bool isValid(const Texture* p); //!< Tells whether a Texture object is valid + bool isValid(const RenderTarget* p); //!< Tells whether a RenderTarget object is valid + bool isValid(const View* p); //!< Tells whether a View object is valid + bool isValid(const InstanceBuffer* p); //!< Tells whether an InstanceBuffer object is valid + /** * Kicks the hardware thread (e.g. the OpenGL, Vulkan or Metal thread) and blocks until * all commands to this point are executed. Note that does guarantee that the diff --git a/filament/src/Engine.cpp b/filament/src/Engine.cpp index ccb1748047e..ce97bdfd0ff 100644 --- a/filament/src/Engine.cpp +++ b/filament/src/Engine.cpp @@ -196,6 +196,61 @@ void Engine::destroy(Entity e) { downcast(this)->destroy(e); } +bool Engine::isValid(const BufferObject* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const VertexBuffer* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const Fence* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const IndexBuffer* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const SkinningBuffer* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const MorphTargetBuffer* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const IndirectLight* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const Material* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const Renderer* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const Scene* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const Skybox* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const ColorGrading* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const SwapChain* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const Stream* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const Texture* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const RenderTarget* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const View* p) { + return downcast(this)->isValid(downcast(p)); +} +bool Engine::isValid(const InstanceBuffer* p) { + return downcast(this)->isValid(downcast(p)); +} + void Engine::flushAndWait() { downcast(this)->flushAndWait(); } diff --git a/filament/src/ResourceList.cpp b/filament/src/ResourceList.cpp index b8e8c8f6618..4579ed8a320 100644 --- a/filament/src/ResourceList.cpp +++ b/filament/src/ResourceList.cpp @@ -45,12 +45,15 @@ bool ResourceListBase::remove(void const* item) { return mList.erase(const_cast(item)) > 0; } +auto ResourceListBase::find(void const* item) -> iterator { + return mList.find(const_cast(item)); +} void ResourceListBase::clear() noexcept { mList.clear(); } -// this is not inlined so we don't pay the code-size cost of iterating the list +// this is not inlined, so we don't pay the code-size cost of iterating the list void ResourceListBase::forEach(void (* f)(void*, void*), void* user) const noexcept { std::for_each(mList.begin(), mList.end(), [=](void* p) { f(user, p); diff --git a/filament/src/ResourceList.h b/filament/src/ResourceList.h index 188a5925fe4..1bdf188a563 100644 --- a/filament/src/ResourceList.h +++ b/filament/src/ResourceList.h @@ -38,6 +38,8 @@ class ResourceListBase { bool remove(void const* item); + iterator find(void const* item); + void clear() noexcept; bool empty() const noexcept { @@ -76,9 +78,12 @@ class ResourceList : private ResourceListBase { using ResourceListBase::forEach; using ResourceListBase::insert; using ResourceListBase::remove; + using ResourceListBase::find; using ResourceListBase::empty; using ResourceListBase::size; using ResourceListBase::clear; + using ResourceListBase::begin; + using ResourceListBase::end; explicit ResourceList(const char* name) noexcept: ResourceListBase(name) {} diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index 4ebb359ef01..76e2d3d6b17 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -852,6 +852,12 @@ void FEngine::cleanupResourceListLocked(Lock& lock, ResourceList&& list) { // ----------------------------------------------------------------------------------------------- +template +UTILS_ALWAYS_INLINE +inline bool FEngine::isValid(const T* ptr, ResourceList& list) { + return list.find(ptr) != list.end(); +} + template UTILS_ALWAYS_INLINE inline bool FEngine::terminateAndDestroy(const T* ptr, ResourceList& list) { @@ -1019,6 +1025,79 @@ void FEngine::destroy(Entity e) { mCameraManager.destroy(e); } +bool FEngine::isValid(const FBufferObject* p) { + return isValid(p, mBufferObjects); +} + +bool FEngine::isValid(const FVertexBuffer* p) { + return isValid(p, mVertexBuffers); +} + +bool FEngine::isValid(const FFence* p) { + return isValid(p, mFences); +} + +bool FEngine::isValid(const FIndexBuffer* p) { + return isValid(p, mIndexBuffers); +} + +bool FEngine::isValid(const FSkinningBuffer* p) { + return isValid(p, mSkinningBuffers); +} + +bool FEngine::isValid(const FMorphTargetBuffer* p) { + return isValid(p, mMorphTargetBuffers); +} + +bool FEngine::isValid(const FIndirectLight* p) { + return isValid(p, mIndirectLights); +} + +bool FEngine::isValid(const FMaterial* p) { + return isValid(p, mMaterials); +} + +bool FEngine::isValid(const FRenderer* p) { + return isValid(p, mRenderers); +} + +bool FEngine::isValid(const FScene* p) { + return isValid(p, mScenes); +} + +bool FEngine::isValid(const FSkybox* p) { + return isValid(p, mSkyboxes); +} + +bool FEngine::isValid(const FColorGrading* p) { + return isValid(p, mColorGradings); +} + +bool FEngine::isValid(const FSwapChain* p) { + return isValid(p, mSwapChains); +} + +bool FEngine::isValid(const FStream* p) { + return isValid(p, mStreams); +} + +bool FEngine::isValid(const FTexture* p) { + return isValid(p, mTextures); +} + +bool FEngine::isValid(const FRenderTarget* p) { + return isValid(p, mRenderTargets); +} + +bool FEngine::isValid(const FView* p) { + return isValid(p, mViews); +} + +bool FEngine::isValid(const FInstanceBuffer* p) { + return isValid(p, mInstanceBuffers); +} + + void* FEngine::streamAlloc(size_t size, size_t alignment) noexcept { // we allow this only for small allocations if (size > 65536) { diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index fdb4d3a3a7c..c9c10471fa0 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -293,6 +293,26 @@ class FEngine : public Engine { bool destroy(const FView* p); bool destroy(const FInstanceBuffer* p); + bool isValid(const FBufferObject* p); + bool isValid(const FVertexBuffer* p); + bool isValid(const FFence* p); + bool isValid(const FIndexBuffer* p); + bool isValid(const FSkinningBuffer* p); + bool isValid(const FMorphTargetBuffer* p); + bool isValid(const FIndirectLight* p); + bool isValid(const FMaterial* p); + bool isValid(const FMaterialInstance* p); + bool isValid(const FRenderer* p); + bool isValid(const FScene* p); + bool isValid(const FSkybox* p); + bool isValid(const FColorGrading* p); + bool isValid(const FSwapChain* p); + bool isValid(const FStream* p); + bool isValid(const FTexture* p); + bool isValid(const FRenderTarget* p); + bool isValid(const FView* p); + bool isValid(const FInstanceBuffer* p); + void destroy(utils::Entity e); void flushAndWait(); @@ -392,6 +412,9 @@ class FEngine : public Engine { backend::Driver& getDriver() const noexcept { return *mDriver; } + template + bool isValid(const T* ptr, ResourceList& list); + template bool terminateAndDestroy(const T* p, ResourceList& list); diff --git a/web/filament-js/jsbindings.cpp b/web/filament-js/jsbindings.cpp index 11634105458..ce4401e9763 100644 --- a/web/filament-js/jsbindings.cpp +++ b/web/filament-js/jsbindings.cpp @@ -540,7 +540,53 @@ class_("Engine") /// vb ::argument:: the [VertexBuffer] to destroy .function("destroyVertexBuffer", (void (*)(Engine*, VertexBuffer*)) [] (Engine* engine, VertexBuffer* vb) { engine->destroy(vb); }, - allow_raw_pointers()); + allow_raw_pointers()) + + .function("isValidRenderer", EMBIND_LAMBDA(bool, (Engine* engine, Renderer* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidView", EMBIND_LAMBDA(bool, (Engine* engine, View* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidScene", EMBIND_LAMBDA(bool, (Engine* engine, Scene* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidFence", EMBIND_LAMBDA(bool, (Engine* engine, Fence* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidStream", EMBIND_LAMBDA(bool, (Engine* engine, Stream* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidIndexBuffer", EMBIND_LAMBDA(bool, (Engine* engine, IndexBuffer* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidVertexBuffer", EMBIND_LAMBDA(bool, (Engine* engine, VertexBuffer* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidSkinningBuffer", EMBIND_LAMBDA(bool, (Engine* engine, SkinningBuffer* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidIndirectLight", EMBIND_LAMBDA(bool, (Engine* engine, IndirectLight* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidMaterial", EMBIND_LAMBDA(bool, (Engine* engine, Material* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidSkybox", EMBIND_LAMBDA(bool, (Engine* engine, Skybox* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidColorGrading", EMBIND_LAMBDA(bool, (Engine* engine, ColorGrading* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidTexture", EMBIND_LAMBDA(bool, (Engine* engine, Texture* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidRenderTarget", EMBIND_LAMBDA(bool, (Engine* engine, RenderTarget* object), { + return engine->isValid(object); + }), allow_raw_pointers()) + .function("isValidSwapChain", EMBIND_LAMBDA(bool, (Engine* engine, SwapChain* object), { + return engine->isValid(object); + }), allow_raw_pointers()); /// SwapChain ::core class:: Represents the platform's native rendering surface. /// See also the [Engine] methods `createSwapChain` and `destroySwapChain`. From 95c7e4d02bcb1310f5154db81332d816c14d9aae Mon Sep 17 00:00:00 2001 From: mackong Date: Thu, 27 Jul 2023 22:07:20 +0800 Subject: [PATCH 09/19] sample: fix typo --- samples/hellotriangle.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/hellotriangle.cpp b/samples/hellotriangle.cpp index a36df692201..c6469fc1b37 100644 --- a/samples/hellotriangle.cpp +++ b/samples/hellotriangle.cpp @@ -72,7 +72,7 @@ static void printUsage(char* name) { std::string usage( "HELLOTRIANGLE renders a spinning colored triangle\n" "Usage:\n" - " SHOWCASE [options]\n" + " HELLOTRIANGLE [options]\n" "Options:\n" " --help, -h\n" " Prints this message\n\n" From 042cd670aa1d6f398d1a6ec1ee6ca144d2608385 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 27 Jul 2023 09:59:47 -0700 Subject: [PATCH 10/19] Improve fence-based timer query emulation It now uses the fence for the start time and end time, leading to much more accurate timings. We also use a single atomic variable instead of two. --- filament/backend/src/opengl/OpenGLDriver.cpp | 4 +- filament/backend/src/opengl/OpenGLDriver.h | 3 +- .../backend/src/opengl/OpenGLTimerQuery.cpp | 84 +++++++++---------- .../backend/src/opengl/OpenGLTimerQuery.h | 5 -- filament/src/FrameInfo.cpp | 10 ++- 5 files changed, 48 insertions(+), 58 deletions(-) diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 5b1ff835688..f9c2871923b 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -3296,19 +3296,17 @@ void OpenGLDriver::flush(int) { if (!gl.bugs.disable_glFlush) { glFlush(); } - mTimerQueryImpl->flush(); } void OpenGLDriver::finish(int) { DEBUG_MARKER() glFinish(); - mTimerQueryImpl->flush(); executeGpuCommandsCompleteOps(); executeEveryNowAndThenOps(); // Note: since we executed a glFinish(), all pending tasks should be done assert_invariant(mGpuCommandCompleteOps.empty()); - // however, some tasks rely on a separated thread to publish their result (e.g. + // However, some tasks rely on a separated thread to publish their result (e.g. // endTimerQuery), so the result could very well not be ready, and the task will // linger a bit longer, this is only true for mEveryNowAndThenOps tasks. // The fallout of this is that we can't assert that mEveryNowAndThenOps is empty. diff --git a/filament/backend/src/opengl/OpenGLDriver.h b/filament/backend/src/opengl/OpenGLDriver.h index efb512b5265..49bbaca975b 100644 --- a/filament/backend/src/opengl/OpenGLDriver.h +++ b/filament/backend/src/opengl/OpenGLDriver.h @@ -151,8 +151,7 @@ class OpenGLDriver final : public DriverBase { struct GLTimerQuery : public HwTimerQuery { struct State { - std::atomic elapsed{}; - std::atomic_bool available{}; + std::atomic elapsed{}; }; struct { GLuint query = 0; diff --git a/filament/backend/src/opengl/OpenGLTimerQuery.cpp b/filament/backend/src/opengl/OpenGLTimerQuery.cpp index 9b7d836540e..f7e537383de 100644 --- a/filament/backend/src/opengl/OpenGLTimerQuery.cpp +++ b/filament/backend/src/opengl/OpenGLTimerQuery.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -41,9 +42,6 @@ TimerQueryNative::TimerQueryNative(OpenGLContext& context) : mContext(context) { TimerQueryNative::~TimerQueryNative() = default; -void TimerQueryNative::flush() { -} - void TimerQueryNative::beginTimeElapsedQuery(GLTimerQuery* query) { mContext.procs.beginQuery(GL_TIME_ELAPSED, query->gl.query); CHECK_GL_ERROR(utils::slog.e) @@ -77,6 +75,8 @@ OpenGLTimerQueryFence::OpenGLTimerQueryFence(OpenGLPlatform& platform) : mPlatform(platform) { mQueue.reserve(2); mThread = std::thread([this]() { + utils::JobSystem::setThreadName("OpenGLTimerQueryFence"); + utils::JobSystem::setThreadPriority(utils::JobSystem::Priority::URGENT_DISPLAY); auto& queue = mQueue; bool exitRequested; do { @@ -111,59 +111,55 @@ void OpenGLTimerQueryFence::enqueue(OpenGLTimerQueryFence::Job&& job) { mCondition.notify_one(); } -void OpenGLTimerQueryFence::flush() { - // Use calls to flush() as a proxy for when the GPU work started. - GLTimerQuery* query = mActiveQuery; - if (query) { - uint64_t const elapsed = query->gl.emulation->elapsed.load(std::memory_order_relaxed); - if (!elapsed) { - uint64_t const now = clock::now().time_since_epoch().count(); - query->gl.emulation->elapsed.store(now, std::memory_order_relaxed); - //SYSTRACE_CONTEXT(); - //SYSTRACE_ASYNC_BEGIN("gpu", query->gl.query); - } - } -} - void OpenGLTimerQueryFence::beginTimeElapsedQuery(GLTimerQuery* query) { - assert_invariant(!mActiveQuery); - // We can't use a fence to figure out when a GPU operation starts (only when it finishes) - // so instead, we use when glFlush() was issued as a proxy. if (UTILS_UNLIKELY(!query->gl.emulation)) { query->gl.emulation = std::make_shared(); } - query->gl.emulation->elapsed.store(0, std::memory_order_relaxed); - query->gl.emulation->available.store(false); - mActiveQuery = query; + query->gl.emulation->elapsed.store(0); + Platform::Fence* fence = mPlatform.createFence(); + std::weak_ptr const weak = query->gl.emulation; + uint32_t const cookie = query->gl.query; + push([&platform = mPlatform, fence, weak, cookie]() { + auto emulation = weak.lock(); + if (emulation) { + platform.waitFence(fence, FENCE_WAIT_FOR_EVER); + int64_t const then = clock::now().time_since_epoch().count(); + emulation->elapsed.store(-then, std::memory_order_relaxed); + SYSTRACE_CONTEXT(); + SYSTRACE_ASYNC_BEGIN("OpenGLTimerQueryFence", cookie); + (void)cookie; + } + platform.destroyFence(fence); + }); } void OpenGLTimerQueryFence::endTimeElapsedQuery(GLTimerQuery* query) { - assert_invariant(mActiveQuery); Platform::Fence* fence = mPlatform.createFence(); std::weak_ptr const weak = query->gl.emulation; - mActiveQuery = nullptr; - //uint32_t cookie = cookie = query->gl.query; - push([&platform = mPlatform, fence, weak]() { + uint32_t const cookie = query->gl.query; + push([&platform = mPlatform, fence, weak, cookie]() { auto emulation = weak.lock(); if (emulation) { platform.waitFence(fence, FENCE_WAIT_FOR_EVER); - auto now = clock::now().time_since_epoch().count(); - auto then = emulation->elapsed.load(std::memory_order_relaxed); - emulation->elapsed.store(now - then, std::memory_order_relaxed); - emulation->available.store(true); - //SYSTRACE_CONTEXT(); - //SYSTRACE_ASYNC_END("gpu", cookie); + int64_t const now = clock::now().time_since_epoch().count(); + int64_t const then = emulation->elapsed.load(std::memory_order_relaxed); + assert_invariant(then < 0); + emulation->elapsed.store(now + then, std::memory_order_relaxed); + SYSTRACE_CONTEXT(); + SYSTRACE_ASYNC_END("OpenGLTimerQueryFence", cookie); + (void)cookie; } platform.destroyFence(fence); }); } bool OpenGLTimerQueryFence::queryResultAvailable(GLTimerQuery* query) { - return query->gl.emulation->available.load(); + return query->gl.emulation->elapsed.load(std::memory_order_relaxed) > 0; } uint64_t OpenGLTimerQueryFence::queryResult(GLTimerQuery* query) { - return query->gl.emulation->elapsed; + int64_t const result = query->gl.emulation->elapsed.load(std::memory_order_relaxed); + return result > 0 ? result : 0; } // ------------------------------------------------------------------------------------------------ @@ -172,30 +168,30 @@ TimerQueryFallback::TimerQueryFallback() = default; TimerQueryFallback::~TimerQueryFallback() = default; -void TimerQueryFallback::flush() { -} - void TimerQueryFallback::beginTimeElapsedQuery(OpenGLTimerQueryInterface::GLTimerQuery* query) { if (!query->gl.emulation) { query->gl.emulation = std::make_shared(); } // this implementation clearly doesn't work at all, but we have no h/w support - query->gl.emulation->available.store(false, std::memory_order_relaxed); - query->gl.emulation->elapsed = clock::now().time_since_epoch().count(); + int64_t const then = clock::now().time_since_epoch().count(); + query->gl.emulation->elapsed.store(-then, std::memory_order_relaxed); } void TimerQueryFallback::endTimeElapsedQuery(OpenGLTimerQueryInterface::GLTimerQuery* query) { // this implementation clearly doesn't work at all, but we have no h/w support - query->gl.emulation->elapsed = clock::now().time_since_epoch().count() - query->gl.emulation->elapsed; - query->gl.emulation->available.store(true, std::memory_order_relaxed); + int64_t const now = clock::now().time_since_epoch().count(); + int64_t const then = query->gl.emulation->elapsed.load(std::memory_order_relaxed); + assert_invariant(then < 0); + query->gl.emulation->elapsed.store(now + then, std::memory_order_relaxed); } bool TimerQueryFallback::queryResultAvailable(OpenGLTimerQueryInterface::GLTimerQuery* query) { - return query->gl.emulation->available.load(std::memory_order_relaxed); + return query->gl.emulation->elapsed.load(std::memory_order_relaxed) > 0; } uint64_t TimerQueryFallback::queryResult(OpenGLTimerQueryInterface::GLTimerQuery* query) { - return query->gl.emulation->elapsed; + int64_t const result = query->gl.emulation->elapsed.load(std::memory_order_relaxed); + return result > 0 ? result : 0; } } // namespace filament::backend diff --git a/filament/backend/src/opengl/OpenGLTimerQuery.h b/filament/backend/src/opengl/OpenGLTimerQuery.h index 7df3f937a8f..1edee992f8a 100644 --- a/filament/backend/src/opengl/OpenGLTimerQuery.h +++ b/filament/backend/src/opengl/OpenGLTimerQuery.h @@ -41,7 +41,6 @@ class OpenGLTimerQueryInterface { public: virtual ~OpenGLTimerQueryInterface(); - virtual void flush() = 0; virtual void beginTimeElapsedQuery(GLTimerQuery* query) = 0; virtual void endTimeElapsedQuery(GLTimerQuery* query) = 0; virtual bool queryResultAvailable(GLTimerQuery* query) = 0; @@ -55,7 +54,6 @@ class TimerQueryNative : public OpenGLTimerQueryInterface { explicit TimerQueryNative(OpenGLContext& context); ~TimerQueryNative() override; private: - void flush() override; void beginTimeElapsedQuery(GLTimerQuery* query) override; void endTimeElapsedQuery(GLTimerQuery* query) override; bool queryResultAvailable(GLTimerQuery* query) override; @@ -71,7 +69,6 @@ class OpenGLTimerQueryFence : public OpenGLTimerQueryInterface { ~OpenGLTimerQueryFence() override; private: using Job = std::function; - void flush() override; void beginTimeElapsedQuery(GLTimerQuery* query) override; void endTimeElapsedQuery(GLTimerQuery* query) override; bool queryResultAvailable(GLTimerQuery* query) override; @@ -89,7 +86,6 @@ class OpenGLTimerQueryFence : public OpenGLTimerQueryInterface { mutable utils::Condition mCondition; std::vector mQueue; bool mExitRequested = false; - GLTimerQuery* mActiveQuery = nullptr; }; class TimerQueryFallback : public OpenGLTimerQueryInterface { @@ -97,7 +93,6 @@ class TimerQueryFallback : public OpenGLTimerQueryInterface { explicit TimerQueryFallback(); ~TimerQueryFallback() override; private: - void flush() override; void beginTimeElapsedQuery(GLTimerQuery* query) override; void endTimeElapsedQuery(GLTimerQuery* query) override; bool queryResultAvailable(GLTimerQuery* query) override; diff --git a/filament/src/FrameInfo.cpp b/filament/src/FrameInfo.cpp index d6f9653ae28..12923e69d37 100644 --- a/filament/src/FrameInfo.cpp +++ b/filament/src/FrameInfo.cpp @@ -51,7 +51,7 @@ void FrameInfoManager::terminate(DriverApi& driver) noexcept { } } -void FrameInfoManager::beginFrame(DriverApi& driver,Config const& config, uint32_t frameId) noexcept { +void FrameInfoManager::beginFrame(DriverApi& driver,Config const& config, uint32_t) noexcept { driver.beginTimerQuery(mQueries[mIndex]); uint64_t elapsed = 0; if (driver.getTimerQueryValue(mQueries[mLast], &elapsed)) { @@ -67,7 +67,8 @@ void FrameInfoManager::endFrame(DriverApi& driver) noexcept { mIndex = (mIndex + 1) % POOL_COUNT; } -void FrameInfoManager::update(Config const& config, FrameInfoManager::duration lastFrameTime) noexcept { +void FrameInfoManager::update(Config const& config, + FrameInfoManager::duration lastFrameTime) noexcept { // keep an history of frame times auto& history = mFrameTimeHistory; @@ -85,12 +86,13 @@ void FrameInfoManager::update(Config const& config, FrameInfoManager::duration l // apply a median filter to get a good representation of the frame time of the last // N frames. std::array median; // NOLINT -- it's initialized below - size_t size = std::min(mFrameTimeHistorySize, std::min(config.historySize, (uint32_t)median.size())); + size_t const size = std::min(mFrameTimeHistorySize, + std::min(config.historySize, (uint32_t)median.size())); for (size_t i = 0; i < size; ++i) { median[i] = history[i].frameTime; } std::sort(median.begin(), median.begin() + size); - duration denoisedFrameTime = median[size / 2]; + duration const denoisedFrameTime = median[size / 2]; history[0].denoisedFrameTime = denoisedFrameTime; history[0].valid = true; From 0ba891fb14a62d402d818766687cd6f010569f5c Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 28 Jul 2023 14:59:01 -0700 Subject: [PATCH 11/19] Fix off-by-one in FrameSkipper The frame latency specified was off-by-one, i.e. a value of 1 meant a latency of 2. The default was 2, which meant 3. Also it wasn't possible to specify the max latency of 4, which would OOB. --- filament/src/FrameSkipper.cpp | 2 +- filament/src/FrameSkipper.h | 26 ++++++++++++++++++++++---- filament/src/details/Engine.cpp | 3 ++- filament/src/details/Renderer.cpp | 8 ++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/filament/src/FrameSkipper.cpp b/filament/src/FrameSkipper.cpp index ff0a185f27a..46e35f8fe6c 100644 --- a/filament/src/FrameSkipper.cpp +++ b/filament/src/FrameSkipper.cpp @@ -27,7 +27,7 @@ using namespace utils; using namespace backend; FrameSkipper::FrameSkipper(size_t latency) noexcept - : mLast(latency) { + : mLast(latency - 1) { assert_invariant(latency <= MAX_FRAME_LATENCY); } diff --git a/filament/src/FrameSkipper.h b/filament/src/FrameSkipper.h index 434c61f6e26..bc3d073d003 100644 --- a/filament/src/FrameSkipper.h +++ b/filament/src/FrameSkipper.h @@ -24,17 +24,35 @@ namespace filament { +/* + * FrameSkipper is used to determine if the current frame needs to be skipped so that we don't + * outrun the GPU. + */ class FrameSkipper { - static constexpr size_t MAX_FRAME_LATENCY = 4; + static constexpr size_t MAX_FRAME_LATENCY = 3; public: + /* + * The latency parameter defines how many unfinished frames we want to accept before we start + * dropping frames. This affects frame latency. + * + * A latency of 1 means that the GPU must be finished with the previous frame so that + * we don't drop the current frame. While this provides the best latency this doesn't allow + * much overlap between the main thread, the back thread and the GPU. + * + * A latency of 2 (default) allows full overlap between the CPU And GPU, but the main and driver + * thread can't fully overlap. + * + * A latency 3 allows the main thread, driver thread and GPU to overlap, each being able to + * use up to 16ms (or whatever the refresh rate is). + */ explicit FrameSkipper(size_t latency = 2) noexcept; ~FrameSkipper() noexcept; void terminate(backend::DriverApi& driver) noexcept; - // returns false if we need to skip this frame, because the gpu is running behind the cpu. - // in that case, don't call endFrame(). - // returns true if rendering can proceed. Always call endFrame() when done. + // Returns false if we need to skip this frame, because the GPU is running behind the CPU; + // In that case, don't call render endFrame() + // Returns true if rendering can proceed. Always call endFrame() when done. bool beginFrame(backend::DriverApi& driver) noexcept; void endFrame(backend::DriverApi& driver) noexcept; diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index 76e2d3d6b17..6816396aeb1 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -1114,9 +1114,10 @@ bool FEngine::execute() { } // execute all command buffers + auto& driver = getDriverApi(); for (auto& item : buffers) { if (UTILS_LIKELY(item.begin)) { - getDriverApi().execute(item.begin); + driver.execute(item.begin); mCommandBufferQueue.releaseBuffer(item); } } diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index 3b01c13420d..b17a6104758 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -55,7 +55,7 @@ using namespace backend; FRenderer::FRenderer(FEngine& engine) : mEngine(engine), - mFrameSkipper(1u), + mFrameSkipper(), mRenderTargetHandle(engine.getDefaultRenderTarget()), mFrameInfoManager(engine.getDriverApi()), mHdrTranslucent(TextureFormat::RGBA16F), @@ -287,14 +287,14 @@ void FRenderer::endFrame() { driver.debugThreading(); } - mFrameInfoManager.endFrame(driver); - mFrameSkipper.endFrame(driver); - if (mSwapChain) { mSwapChain->commit(driver); mSwapChain = nullptr; } + mFrameInfoManager.endFrame(driver); + mFrameSkipper.endFrame(driver); + driver.endFrame(mFrameId); // gives the backend a chance to execute periodic tasks From 98a2b8f159dca73a1efcb8b273e140385d9538b9 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 28 Jul 2023 16:05:39 -0700 Subject: [PATCH 12/19] Improve FrameSkipper performance on GLES/Android We get rid of the backend's HwSync object because on all platforms but GL it was implemented just like a HwFence. We now use HwFence instead. On GL platforms though, HwFence doesn't exist natively it is instead provided by the Platform. In that case, we emulate it as with GLSync objects -- the emulation incurs some latency that can cause frames to be skipped. On Android and platforms that provide the Fence functionality, there is no such issue. This change improves significantly frame pacing on Android. --- NEW_RELEASE_NOTES.md | 2 + filament/backend/include/backend/Handle.h | 2 - .../include/private/backend/DriverAPI.inc | 4 - filament/backend/src/DriverBase.h | 3 - filament/backend/src/Handle.cpp | 1 - filament/backend/src/metal/MetalDriver.mm | 29 --- filament/backend/src/metal/MetalHandles.h | 4 +- filament/backend/src/noop/NoopDriver.cpp | 7 - filament/backend/src/opengl/OpenGLContext.cpp | 59 ------ filament/backend/src/opengl/OpenGLContext.h | 20 -- filament/backend/src/opengl/OpenGLDriver.cpp | 199 ++++++++++-------- filament/backend/src/opengl/OpenGLDriver.h | 19 +- filament/backend/src/vulkan/VulkanDriver.cpp | 33 --- filament/backend/src/vulkan/VulkanHandles.h | 6 - filament/src/FrameSkipper.cpp | 35 ++- filament/src/FrameSkipper.h | 4 +- 16 files changed, 142 insertions(+), 285 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 77176462ef1..141b196b233 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -10,3 +10,5 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). - backend: Disable timer queries on all Mali GPUs (fixes b/233754398) - engine: Add a way to query the validity of most filament objects (see `Engine::isValid`) - opengl: fix b/290388359 : possible crash when shutting down the engine +- engine: Improve precision of frame time measurement when using emulated TimerQueries +- backend: Improve frame pacing on Android and Vulkan. diff --git a/filament/backend/include/backend/Handle.h b/filament/backend/include/backend/Handle.h index 7317e2c59b1..04e83809800 100644 --- a/filament/backend/include/backend/Handle.h +++ b/filament/backend/include/backend/Handle.h @@ -39,7 +39,6 @@ struct HwRenderTarget; struct HwSamplerGroup; struct HwStream; struct HwSwapChain; -struct HwSync; struct HwTexture; struct HwTimerQuery; struct HwVertexBuffer; @@ -126,7 +125,6 @@ using RenderTargetHandle = Handle; using SamplerGroupHandle = Handle; using StreamHandle = Handle; using SwapChainHandle = Handle; -using SyncHandle = Handle; using TextureHandle = Handle; using TimerQueryHandle = Handle; using VertexBufferHandle = Handle; diff --git a/filament/backend/include/private/backend/DriverAPI.inc b/filament/backend/include/private/backend/DriverAPI.inc index 84ac2f24049..150615767fb 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -245,8 +245,6 @@ DECL_DRIVER_API_R_N(backend::RenderTargetHandle, createRenderTarget, DECL_DRIVER_API_R_0(backend::FenceHandle, createFence) -DECL_DRIVER_API_R_0(backend::SyncHandle, createSync) - DECL_DRIVER_API_R_N(backend::SwapChainHandle, createSwapChain, void*, nativeWindow, uint64_t, flags) @@ -275,7 +273,6 @@ DECL_DRIVER_API_N(destroyRenderTarget, backend::RenderTargetHandle, rth) DECL_DRIVER_API_N(destroySwapChain, backend::SwapChainHandle, sch) DECL_DRIVER_API_N(destroyStream, backend::StreamHandle, sh) DECL_DRIVER_API_N(destroyTimerQuery, backend::TimerQueryHandle, sh) -DECL_DRIVER_API_N(destroySync, backend::SyncHandle, sh) /* * Synchronous APIs @@ -306,7 +303,6 @@ DECL_DRIVER_API_SYNCHRONOUS_0(math::float2, getClipSpaceParams) DECL_DRIVER_API_SYNCHRONOUS_0(bool, canGenerateMipmaps) DECL_DRIVER_API_SYNCHRONOUS_N(void, setupExternalImage, void*, image) DECL_DRIVER_API_SYNCHRONOUS_N(bool, getTimerQueryValue, backend::TimerQueryHandle, query, uint64_t*, elapsedTime) -DECL_DRIVER_API_SYNCHRONOUS_N(backend::SyncStatus, getSyncStatus, backend::SyncHandle, sh) DECL_DRIVER_API_SYNCHRONOUS_N(bool, isWorkaroundNeeded, backend::Workaround, workaround) DECL_DRIVER_API_SYNCHRONOUS_0(backend::FeatureLevel, getFeatureLevel) diff --git a/filament/backend/src/DriverBase.h b/filament/backend/src/DriverBase.h index 3de365245b9..3e7f2647d2f 100644 --- a/filament/backend/src/DriverBase.h +++ b/filament/backend/src/DriverBase.h @@ -135,9 +135,6 @@ struct HwFence : public HwBase { Platform::Fence* fence = nullptr; }; -struct HwSync : public HwBase { -}; - struct HwSwapChain : public HwBase { Platform::SwapChain* swapChain = nullptr; }; diff --git a/filament/backend/src/Handle.cpp b/filament/backend/src/Handle.cpp index c715a5ee67e..a32b2252680 100644 --- a/filament/backend/src/Handle.cpp +++ b/filament/backend/src/Handle.cpp @@ -67,7 +67,6 @@ template io::ostream& operator<<(io::ostream& out, const Handle& h) noe template io::ostream& operator<<(io::ostream& out, const Handle& h) noexcept; template io::ostream& operator<<(io::ostream& out, const Handle& h) noexcept; template io::ostream& operator<<(io::ostream& out, const Handle& h) noexcept; -template io::ostream& operator<<(io::ostream& out, const Handle& h) noexcept; template io::ostream& operator<<(io::ostream& out, const Handle& h) noexcept; #endif diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 61f42b1143a..a849180ea9b 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -380,11 +380,6 @@ fence->encode(); } -void MetalDriver::createSyncR(Handle sh, int) { - auto* fence = handle_cast(sh); - fence->encode(); -} - void MetalDriver::createSwapChainR(Handle sch, void* nativeWindow, uint64_t flags) { if (UTILS_UNLIKELY(flags & SWAP_CHAIN_CONFIG_APPLE_CVPIXELBUFFER)) { CVPixelBufferRef pixelBuffer = (CVPixelBufferRef) nativeWindow; @@ -454,12 +449,6 @@ return alloc_and_construct_handle(*mContext); } -Handle MetalDriver::createSyncS() noexcept { - // The handle must be constructed here, as a synchronous call to getSyncStatus might happen - // before createSyncR is executed. - return alloc_and_construct_handle(*mContext); -} - Handle MetalDriver::createSwapChainS() noexcept { return alloc_handle(); } @@ -567,13 +556,6 @@ } } -void MetalDriver::destroySync(Handle sh) { - if (sh) { - destruct_handle(sh); - } -} - - void MetalDriver::terminate() { // finish() will flush the pending command buffer and will ensure all GPU work has finished. // This must be done before calling bufferPool->reset() to ensure no buffers are in flight. @@ -841,17 +823,6 @@ return mContext->timerQueryImpl->getQueryResult(tq, elapsedTime); } -SyncStatus MetalDriver::getSyncStatus(Handle sh) { - auto* fence = handle_cast(sh); - FenceStatus status = fence->wait(0); - if (status == FenceStatus::TIMEOUT_EXPIRED) { - return SyncStatus::NOT_SIGNALED; - } else if (status == FenceStatus::CONDITION_SATISFIED) { - return SyncStatus::SIGNALED; - } - return SyncStatus::ERROR; -} - void MetalDriver::generateMipmaps(Handle th) { ASSERT_PRECONDITION(!isInRenderPass(mContext), "generateMipmaps must be called outside of a render pass."); diff --git a/filament/backend/src/metal/MetalHandles.h b/filament/backend/src/metal/MetalHandles.h index b1f59d59e83..9ffa1a0bda8 100644 --- a/filament/backend/src/metal/MetalHandles.h +++ b/filament/backend/src/metal/MetalHandles.h @@ -446,9 +446,7 @@ class MetalRenderTarget : public HwRenderTarget { }; // MetalFence is used to implement both Fences and Syncs. -// There's no diamond problem, because HwBase (superclass of HwFence and HwSync) is empty. -static_assert(std::is_empty_v); -class MetalFence : public HwFence, public HwSync { +class MetalFence : public HwFence { public: // MetalFence is special, as it gets constructed on the Filament thread. We must delay inserting diff --git a/filament/backend/src/noop/NoopDriver.cpp b/filament/backend/src/noop/NoopDriver.cpp index 61b80df465c..40a617f4a17 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -107,9 +107,6 @@ void NoopDriver::destroyStream(Handle sh) { void NoopDriver::destroyTimerQuery(Handle tqh) { } -void NoopDriver::destroySync(Handle fh) { -} - Handle NoopDriver::createStreamNative(void* nativeStream) { return {}; } @@ -236,10 +233,6 @@ bool NoopDriver::getTimerQueryValue(Handle tqh, uint64_t* elapsedT return false; } -SyncStatus NoopDriver::getSyncStatus(Handle sh) { - return SyncStatus::SIGNALED; -} - void NoopDriver::setExternalImage(Handle th, void* image) { } diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp index f3d9300eb06..14d7c679c7f 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -882,63 +882,4 @@ void OpenGLContext::resetState() noexcept { } -OpenGLContext::FenceSync OpenGLContext::createFenceSync( - OpenGLPlatform& platform) noexcept { - - if (UTILS_UNLIKELY(isES2())) { - assert_invariant(platform.canCreateFence()); - return { .fence = platform.createFence() }; - } - -#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 - auto sync = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); - CHECK_GL_ERROR(utils::slog.e) - return { .sync = sync }; -#else - return {}; -#endif -} - -void OpenGLContext::destroyFenceSync( - OpenGLPlatform& platform, FenceSync sync) noexcept { - - if (UTILS_UNLIKELY(isES2())) { - platform.destroyFence(static_cast(sync.fence)); - return; - } - -#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 - glDeleteSync(sync.sync); - CHECK_GL_ERROR(utils::slog.e) -#endif -} - -OpenGLContext::FenceSync::Status OpenGLContext::clientWaitSync( - OpenGLPlatform& platform, FenceSync sync) const noexcept { - - if (UTILS_UNLIKELY(isES2())) { - using Status = OpenGLContext::FenceSync::Status; - auto const status = platform.waitFence(static_cast(sync.fence), 0u); - switch (status) { - case FenceStatus::ERROR: return Status::FAILURE; - case FenceStatus::CONDITION_SATISFIED: return Status::CONDITION_SATISFIED; - case FenceStatus::TIMEOUT_EXPIRED: return Status ::TIMEOUT_EXPIRED; - } - } - -#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 - GLenum const status = glClientWaitSync(sync.sync, 0, 0u); - CHECK_GL_ERROR(utils::slog.e) - using Status = OpenGLContext::FenceSync::Status; - switch (status) { - case GL_ALREADY_SIGNALED: return Status::ALREADY_SIGNALED; - case GL_TIMEOUT_EXPIRED: return Status::TIMEOUT_EXPIRED; - case GL_CONDITION_SATISFIED: return Status::CONDITION_SATISFIED; - default: return Status::FAILURE; - } -#else - return FenceSync::Status::FAILURE; -#endif -} - } // namesapce filament diff --git a/filament/backend/src/opengl/OpenGLContext.h b/filament/backend/src/opengl/OpenGLContext.h index 60930213719..cdad65b8a7d 100644 --- a/filament/backend/src/opengl/OpenGLContext.h +++ b/filament/backend/src/opengl/OpenGLContext.h @@ -150,26 +150,6 @@ class OpenGLContext { void deleteBuffers(GLsizei n, const GLuint* buffers, GLenum target) noexcept; void deleteVertexArrays(GLsizei n, const GLuint* arrays) noexcept; - // we abstract GL's sync because it's not available in ES2, but we can use EGL's sync - // instead, if available. - struct FenceSync { - enum class Status { - ALREADY_SIGNALED, - TIMEOUT_EXPIRED, - CONDITION_SATISFIED, - FAILURE - }; - union { - void* fence; - GLsync sync; - }; - }; - - FenceSync createFenceSync(OpenGLPlatform& platform) noexcept; - void destroyFenceSync(OpenGLPlatform& platform, FenceSync sync) noexcept; - FenceSync::Status clientWaitSync(OpenGLPlatform& platform, FenceSync sync) const noexcept; - - // glGet*() values struct { GLfloat max_anisotropy; diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index f9c2871923b..98f50859e83 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -233,13 +233,13 @@ void OpenGLDriver::terminate() { mShaderCompilerService.terminate(); +#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 // and make sure to execute all the GpuCommandCompleteOps callbacks executeGpuCommandsCompleteOps(); // because we called glFinish(), all callbacks should have been executed assert_invariant(mGpuCommandCompleteOps.empty()); -#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 if (!getContext().isES2()) { for (auto& item: mSamplerMap) { mContext.unbindSampler(item.second); @@ -436,11 +436,7 @@ Handle OpenGLDriver::createRenderTargetS() noexcept { } Handle OpenGLDriver::createFenceS() noexcept { - return initHandle(); -} - -Handle OpenGLDriver::createSyncS() noexcept { - return initHandle(); + return initHandle(); } Handle OpenGLDriver::createSwapChainS() noexcept { @@ -1352,28 +1348,45 @@ void OpenGLDriver::createRenderTargetR(Handle rth, void OpenGLDriver::createFenceR(Handle fh, int) { DEBUG_MARKER() - HwFence* f = handle_cast(fh); - f->fence = mPlatform.createFence(); -} + GLFence* f = handle_cast(fh); -void OpenGLDriver::createSyncR(Handle fh, int) { - DEBUG_MARKER() - - GLSync* f = handle_cast(fh); - f->handle = mContext.createFenceSync(mPlatform); - - // check the status of the sync once a frame, since we must do this from our thread - std::weak_ptr const weak = f->result; - runEveryNowAndThen( - [&platform = mPlatform, context = mContext, handle = f->handle, weak]() -> bool { - auto result = weak.lock(); - if (result) { - auto const status = context.clientWaitSync(platform, handle); - result->status.store(status, std::memory_order_relaxed); - return (status != OpenGLContext::FenceSync::Status::TIMEOUT_EXPIRED); - } - return true; - }); + if (mPlatform.canCreateFence() || mContext.isES2()) { + assert_invariant(mPlatform.canCreateFence()); + f->fence = mPlatform.createFence(); + } +#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 + else { + f->sync = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); + std::weak_ptr const weak = f->state; + runEveryNowAndThen( + [sync = f->sync, weak]() -> bool { + auto state = weak.lock(); + if (state) { + FenceStatus fenceStatus; + GLenum const syncStatus = glClientWaitSync(sync, 0, 0u); + switch (syncStatus) { + case GL_TIMEOUT_EXPIRED: + fenceStatus = FenceStatus::TIMEOUT_EXPIRED; + break; + case GL_ALREADY_SIGNALED: + case GL_CONDITION_SATISFIED: + fenceStatus = FenceStatus::CONDITION_SATISFIED; + break; + default: + fenceStatus = FenceStatus::ERROR; + break; + } + if (fenceStatus != FenceStatus::TIMEOUT_EXPIRED) { + std::lock_guard const lock(state->lock); + state->status = fenceStatus; + state->cond.notify_all(); + } + return (fenceStatus != FenceStatus::TIMEOUT_EXPIRED); + } + return true; + }); + } +#endif } void OpenGLDriver::createSwapChainR(Handle sch, void* nativeWindow, uint64_t flags) { @@ -1572,15 +1585,6 @@ void OpenGLDriver::destroyTimerQuery(Handle tqh) { } } -void OpenGLDriver::destroySync(Handle sh) { - DEBUG_MARKER() - if (sh) { - GLSync* s = handle_cast(sh); - mContext.destroyFenceSync(mPlatform, s->handle); - destruct(sh, s); - } -} - // ------------------------------------------------------------------------------------------------ // Synchronous APIs // These are called on the application's thread @@ -1683,24 +1687,46 @@ int64_t OpenGLDriver::getStreamTimestamp(Handle sh) { void OpenGLDriver::destroyFence(Handle fh) { if (fh) { - HwFence* f = handle_cast(fh); - mPlatform.destroyFence(f->fence); + GLFence* f = handle_cast(fh); + if (mPlatform.canCreateFence() || mContext.isES2()) { + mPlatform.destroyFence(f->fence); + } +#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 + else { + glDeleteSync(f->sync); + CHECK_GL_ERROR(utils::slog.e) + } +#endif destruct(fh, f); } } FenceStatus OpenGLDriver::wait(Handle fh, uint64_t timeout) { if (fh) { - HwFence* f = handle_cast(fh); - if (f->fence == nullptr) { - // we can end-up here if: - // - the platform doesn't support h/w fences - // - wait() was called before the fence was asynchronously created. - // This case is not handled in OpenGLDriver but is handled by FFence. - // TODO: move FFence logic into the backend. - return FenceStatus::ERROR; + GLFence* f = handle_cast(fh); + if (mPlatform.canCreateFence() || mContext.isES2()) { + if (f->fence == nullptr) { + // we can end-up here if: + // - the platform doesn't support h/w fences + if (UTILS_UNLIKELY(!mPlatform.canCreateFence())) { + return FenceStatus::ERROR; + } + // - wait() was called before the fence was asynchronously created. + return FenceStatus::TIMEOUT_EXPIRED; + } + return mPlatform.waitFence(f->fence, timeout); + } +#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 + else { + assert_invariant(f->state); + std::unique_lock lock(f->state->lock); + f->state->cond.wait_for(lock, + std::chrono::nanoseconds(timeout), [&state = f->state]() { + return state->status != FenceStatus::TIMEOUT_EXPIRED; + }); + return f->state->status; } - return mPlatform.waitFence(f->fence, timeout); +#endif } return FenceStatus::ERROR; } @@ -2588,25 +2614,6 @@ bool OpenGLDriver::getTimerQueryValue(Handle tqh, uint64_t* elapse return true; } -SyncStatus OpenGLDriver::getSyncStatus(Handle sh) { - GLSync* s = handle_cast(sh); - if (!s->result) { - return SyncStatus::NOT_SIGNALED; - } - auto status = s->result->status.load(std::memory_order_relaxed); - using Status = OpenGLContext::FenceSync::Status; - switch (status) { - case Status::CONDITION_SATISFIED: - case Status::ALREADY_SIGNALED: - return SyncStatus::SIGNALED; - case Status::TIMEOUT_EXPIRED: - return SyncStatus::NOT_SIGNALED; - case Status::FAILURE: - default: - return SyncStatus::ERROR; - } -} - void OpenGLDriver::compilePrograms(CompilerPriorityQueue priority, CallbackHandler* handler, CallbackHandler::Callback callback, void* user) { if (callback) { @@ -3180,30 +3187,16 @@ void OpenGLDriver::readBufferSubData(backend::BufferObjectHandle boh, #endif } -void OpenGLDriver::whenGpuCommandsComplete(std::function fn) noexcept { - OpenGLContext::FenceSync sync = mContext.createFenceSync(mPlatform); - mGpuCommandCompleteOps.emplace_back(sync, std::move(fn)); - CHECK_GL_ERROR(utils::slog.e) -} void OpenGLDriver::runEveryNowAndThen(std::function fn) noexcept { mEveryNowAndThenOps.push_back(std::move(fn)); } -void OpenGLDriver::executeGpuCommandsCompleteOps() noexcept { - auto& v = mGpuCommandCompleteOps; +void OpenGLDriver::executeEveryNowAndThenOps() noexcept { + auto& v = mEveryNowAndThenOps; auto it = v.begin(); while (it != v.end()) { - using Status = OpenGLContext::FenceSync::Status; - auto const status = mContext.clientWaitSync(mPlatform, it->first); - if (status == Status::ALREADY_SIGNALED || status == Status::CONDITION_SATISFIED) { - it->second(); - mContext.destroyFenceSync(mPlatform, it->first); - it = v.erase(it); - } else if (UTILS_UNLIKELY(status == Status::FAILURE)) { - // This should never happen, but is very problematic if it does, as we might leak - // some data depending on what the callback does. However, we clean up our own state. - mContext.destroyFenceSync(mPlatform, it->first); + if ((*it)()) { it = v.erase(it); } else { ++it; @@ -3211,17 +3204,41 @@ void OpenGLDriver::executeGpuCommandsCompleteOps() noexcept { } } -void OpenGLDriver::executeEveryNowAndThenOps() noexcept { - auto& v = mEveryNowAndThenOps; +#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 +void OpenGLDriver::whenGpuCommandsComplete(const std::function& fn) noexcept { + GLsync sync = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); + mGpuCommandCompleteOps.emplace_back(sync, fn); + CHECK_GL_ERROR(utils::slog.e) +} + +void OpenGLDriver::executeGpuCommandsCompleteOps() noexcept { + auto& v = mGpuCommandCompleteOps; auto it = v.begin(); while (it != v.end()) { - if ((*it)()) { - it = v.erase(it); - } else { - ++it; + auto const& [sync, fn] = *it; + GLenum const syncStatus = glClientWaitSync(sync, 0, 0u); + switch (syncStatus) { + case GL_TIMEOUT_EXPIRED: + // not ready + ++it; + break; + case GL_ALREADY_SIGNALED: + case GL_CONDITION_SATISFIED: + // ready + it->second(); + glDeleteSync(sync); + it = v.erase(it); + break; + default: + // This should never happen, but is very problematic if it does, as we might leak + // some data depending on what the callback does. However, we clean up our own state. + glDeleteSync(sync); + it = v.erase(it); + break; } } } +#endif // ------------------------------------------------------------------------------------------------ // Rendering ops @@ -3301,10 +3318,12 @@ void OpenGLDriver::flush(int) { void OpenGLDriver::finish(int) { DEBUG_MARKER() glFinish(); +#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 executeGpuCommandsCompleteOps(); + assert_invariant(mGpuCommandCompleteOps.empty()); +#endif executeEveryNowAndThenOps(); // Note: since we executed a glFinish(), all pending tasks should be done - assert_invariant(mGpuCommandCompleteOps.empty()); // However, some tasks rely on a separated thread to publish their result (e.g. // endTimerQuery), so the result could very well not be ready, and the task will diff --git a/filament/backend/src/opengl/OpenGLDriver.h b/filament/backend/src/opengl/OpenGLDriver.h index 49bbaca975b..bc5830044e0 100644 --- a/filament/backend/src/opengl/OpenGLDriver.h +++ b/filament/backend/src/opengl/OpenGLDriver.h @@ -195,14 +195,15 @@ class OpenGLDriver final : public DriverBase { TargetBufferFlags targets = {}; }; - struct GLSync : public HwSync { - using HwSync::HwSync; + struct GLFence : public HwFence { + using HwFence::HwFence; struct State { - std::atomic status{ - OpenGLContext::FenceSync::Status::TIMEOUT_EXPIRED }; + std::mutex lock; + std::condition_variable cond; + FenceStatus status{ FenceStatus::TIMEOUT_EXPIRED }; }; - OpenGLContext::FenceSync handle{}; - std::shared_ptr result{ std::make_shared() }; + GLsync sync; + std::shared_ptr state{ std::make_shared() }; }; OpenGLDriver(OpenGLDriver const&) = delete; @@ -382,10 +383,12 @@ class OpenGLDriver final : public DriverBase { void updateTextureLodRange(GLTexture* texture, int8_t targetLevel) noexcept; +#ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 // tasks executed on the main thread after the fence signaled - void whenGpuCommandsComplete(std::function fn) noexcept; + void whenGpuCommandsComplete(const std::function& fn) noexcept; void executeGpuCommandsCompleteOps() noexcept; - std::vector>> mGpuCommandCompleteOps; + std::vector>> mGpuCommandCompleteOps; +#endif // tasks regularly executed on the main thread at until they return true void runEveryNowAndThen(std::function fn) noexcept; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 0e2c2e6d1e9..30573ddf194 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -506,11 +506,6 @@ void VulkanDriver::createFenceR(Handle fh, int) { construct(fh, commandBuffer); } -void VulkanDriver::createSyncR(Handle sh, int) { - VulkanCommandBuffer const& commandBuffer = mCommands->get(); - construct(sh, commandBuffer); -} - void VulkanDriver::createSwapChainR(Handle sch, void* nativeWindow, uint64_t flags) { if ((flags & backend::SWAP_CHAIN_CONFIG_SRGB_COLORSPACE) != 0 && !isSRGBSwapChainSupported()) { @@ -587,12 +582,6 @@ Handle VulkanDriver::createFenceS() noexcept { return allocHandle(); } -Handle VulkanDriver::createSyncS() noexcept { - Handle sh = allocHandle(); - construct(sh); - return sh; -} - Handle VulkanDriver::createSwapChainS() noexcept { return allocHandle(); } @@ -647,11 +636,6 @@ void VulkanDriver::destroyTimerQuery(Handle tqh) { } } -void VulkanDriver::destroySync(Handle sh) { - destruct(sh); -} - - Handle VulkanDriver::createStreamNative(void* nativeStream) { return {}; } @@ -931,23 +915,6 @@ bool VulkanDriver::getTimerQueryValue(Handle tqh, uint64_t* elapse return true; } -SyncStatus VulkanDriver::getSyncStatus(Handle sh) { - VulkanSync* sync = handle_cast(sh); - if (sync->fence == nullptr) { - return SyncStatus::NOT_SIGNALED; - } - VkResult status = sync->fence->status.load(std::memory_order_relaxed); - switch (status) { - case VK_SUCCESS: return SyncStatus::SIGNALED; - case VK_INCOMPLETE: return SyncStatus::NOT_SIGNALED; - case VK_NOT_READY: return SyncStatus::NOT_SIGNALED; - case VK_ERROR_DEVICE_LOST: return SyncStatus::ERROR; - default: - // NOTE: In theory, the fence status must be one of the above values. - return SyncStatus::ERROR; - } -} - void VulkanDriver::setExternalImage(Handle th, void* image) { } diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index c234cf53944..fb82e78f738 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -133,12 +133,6 @@ struct VulkanFence : public HwFence { std::shared_ptr fence; }; -struct VulkanSync : public HwSync { - VulkanSync() = default; - explicit VulkanSync(const VulkanCommandBuffer& commands) : fence(commands.fence) {} - std::shared_ptr fence; -}; - struct VulkanTimerQuery : public HwTimerQuery { explicit VulkanTimerQuery(std::tuple indices); ~VulkanTimerQuery(); diff --git a/filament/src/FrameSkipper.cpp b/filament/src/FrameSkipper.cpp index 46e35f8fe6c..acb27958cf5 100644 --- a/filament/src/FrameSkipper.cpp +++ b/filament/src/FrameSkipper.cpp @@ -16,8 +16,6 @@ #include "FrameSkipper.h" -#include "details/Engine.h" - #include #include @@ -34,39 +32,40 @@ FrameSkipper::FrameSkipper(size_t latency) noexcept FrameSkipper::~FrameSkipper() noexcept = default; void FrameSkipper::terminate(DriverApi& driver) noexcept { - for (auto sync : mDelayedSyncs) { - if (sync) { - driver.destroySync(sync); + for (auto fence : mDelayedFences) { + if (fence) { + driver.destroyFence(fence); } } } bool FrameSkipper::beginFrame(DriverApi& driver) noexcept { - auto& syncs = mDelayedSyncs; - auto sync = syncs.front(); - if (sync) { - auto status = driver.getSyncStatus(sync); - if (status == SyncStatus::NOT_SIGNALED) { + auto& fences = mDelayedFences; + auto fence = fences.front(); + if (fence) { + auto status = driver.wait(fence, 0); + if (status == FenceStatus::TIMEOUT_EXPIRED) { // Sync not ready, skip frame return false; } - driver.destroySync(sync); + assert_invariant(status == FenceStatus::CONDITION_SATISFIED); + driver.destroyFence(fence); } // shift all fences down by 1 - std::move(syncs.begin() + 1, syncs.end(), syncs.begin()); - syncs.back() = {}; + std::move(fences.begin() + 1, fences.end(), fences.begin()); + fences.back() = {}; return true; } void FrameSkipper::endFrame(DriverApi& driver) noexcept { - // if the user produced a new frame despite the fact that the previous one wasn't finished + // If the user produced a new frame despite the fact that the previous one wasn't finished // (i.e. FrameSkipper::beginFrame() returned false), we need to make sure to replace // a fence that might be here already) - auto& sync = mDelayedSyncs[mLast]; - if (sync) { - driver.destroySync(sync); + auto& fence = mDelayedFences[mLast]; + if (fence) { + driver.destroyFence(fence); } - sync = driver.createSync(); + fence = driver.createFence(); } } // namespace filament diff --git a/filament/src/FrameSkipper.h b/filament/src/FrameSkipper.h index bc3d073d003..3b7cedbc25c 100644 --- a/filament/src/FrameSkipper.h +++ b/filament/src/FrameSkipper.h @@ -58,8 +58,8 @@ class FrameSkipper { void endFrame(backend::DriverApi& driver) noexcept; private: - using Container = std::array, MAX_FRAME_LATENCY>; - mutable Container mDelayedSyncs{}; + using Container = std::array, MAX_FRAME_LATENCY>; + mutable Container mDelayedFences{}; size_t mLast; }; From b2278986dd5eb3acaaec74daa093be35abe414eb Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 27 Jul 2023 14:24:30 -0700 Subject: [PATCH 13/19] Update bug_report.md --- .github/ISSUE_TEMPLATE/bug_report.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 7faddd90696..69bc29cc63b 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -4,6 +4,8 @@ about: Create a report to help us improve --- +⚠️ **Issues not using this template will be systematically closed.** + **Describe the bug** A clear and concise description of what the bug is. @@ -18,8 +20,8 @@ A clear and concise description of what you expected to happen. If applicable, add screenshots to help explain your problem. **Logs** -If applicable, copy logs from your console here. Please *do not* -use screenshots of logs, copy them as text. +If applicable, copy **full** logs from your console here. Please *do not* +use screenshots of logs, copy them as text, use gist or attach an *uncompressed* file. **Desktop (please complete the following information):** - OS: [e.g. iOS] From 6c05029a9f4011fc8e1f778ad5f38b891b4d35e0 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 31 Jul 2023 14:41:22 -0700 Subject: [PATCH 14/19] workaround a crash with some adreno drivers The crashes are triggered by spirv-opt's MergeReturnPass, so we just disable it. This pass also caused issues with AMD drivers on macOS. fixes b/291140208 --- NEW_RELEASE_NOTES.md | 1 + libs/filamat/src/GLSLPostProcessor.cpp | 120 ++++++++++++------------- 2 files changed, 60 insertions(+), 61 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 141b196b233..828fc267167 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -12,3 +12,4 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). - opengl: fix b/290388359 : possible crash when shutting down the engine - engine: Improve precision of frame time measurement when using emulated TimerQueries - backend: Improve frame pacing on Android and Vulkan. +- backend: workaround b/291140208 (gltf_viewer crashes on Nexus 6P) \ No newline at end of file diff --git a/libs/filamat/src/GLSLPostProcessor.cpp b/libs/filamat/src/GLSLPostProcessor.cpp index d8cb02e11bb..0206aea23f6 100644 --- a/libs/filamat/src/GLSLPostProcessor.cpp +++ b/libs/filamat/src/GLSLPostProcessor.cpp @@ -628,36 +628,33 @@ void GLSLPostProcessor::fixupClipDistance( } } -void GLSLPostProcessor::registerPerformancePasses(Optimizer& optimizer, Config const& config) { - optimizer - .RegisterPass(CreateWrapOpKillPass()) - .RegisterPass(CreateDeadBranchElimPass()); - - if (config.shaderModel != ShaderModel::DESKTOP || - config.targetApi != MaterialBuilder::TargetApi::OPENGL) { - // this triggers a segfault with AMD OpenGL drivers on MacOS - // note that Metal also requires this pass in order to correctly generate half-precision MSL - optimizer.RegisterPass(CreateMergeReturnPass()); - } +// CreateMergeReturnPass() causes these issues: +// - triggers a segfault with AMD OpenGL drivers on macOS +// - triggers a crash on some Adreno drivers (b/291140208, b/289401984, b/289393290) +// However Metal requires this pass in order to correctly generate half-precision MSL +// +// CreateSimplificationPass() creates a lot of problems: +// - Adreno GPU show artifacts after running simplification passes (Vulkan) +// - spirv-cross fails generating working glsl +// (https://github.com/KhronosGroup/SPIRV-Cross/issues/2162) +// - generally it makes the code more complicated, e.g.: replacing for loops with +// while-if-break, unclear if it helps for anything. +// However, the simplification passes below are necessary when targeting Metal, otherwise the +// result is mismatched half / float assignments in MSL. - // CreateSimplificationPass() creates a lot of problems: - // - Adreno GPU show artifacts after running simplication passes (Vulkan) - // - spirv-cross fails generating working glsl - // (https://github.com/KhronosGroup/SPIRV-Cross/issues/2162) - // - generally it makes the code more complicated, e.g.: replacing for loops with - // while-if-break, unclear if it helps for anything. - // However, the simplification passes below are necessary when targeting Metal, otherwise the - // result is mismatched half / float assignments in MSL. +void GLSLPostProcessor::registerPerformancePasses(Optimizer& optimizer, Config const& config) { auto RegisterPass = [&](spvtools::Optimizer::PassToken&& pass, - MaterialBuilder::TargetApi apiFilter = - MaterialBuilder::TargetApi::ALL) { + MaterialBuilder::TargetApi apiFilter = MaterialBuilder::TargetApi::ALL) { if (!(config.targetApi & apiFilter)) { return; } optimizer.RegisterPass(std::move(pass)); }; + RegisterPass(CreateWrapOpKillPass()); + RegisterPass(CreateDeadBranchElimPass()); + RegisterPass(CreateMergeReturnPass(), MaterialBuilder::TargetApi::METAL); RegisterPass(CreateInlineExhaustivePass()); RegisterPass(CreateAggressiveDCEPass()); RegisterPass(CreatePrivateToLocalPass()); @@ -692,47 +689,48 @@ void GLSLPostProcessor::registerPerformancePasses(Optimizer& optimizer, Config c } void GLSLPostProcessor::registerSizePasses(Optimizer& optimizer, Config const& config) { - optimizer - .RegisterPass(CreateWrapOpKillPass()) - .RegisterPass(CreateDeadBranchElimPass()); - - if (config.shaderModel != ShaderModel::DESKTOP) { - // this triggers a segfault with AMD drivers on MacOS - optimizer.RegisterPass(CreateMergeReturnPass()); - } + auto RegisterPass = [&](spvtools::Optimizer::PassToken&& pass, + MaterialBuilder::TargetApi apiFilter = MaterialBuilder::TargetApi::ALL) { + if (!(config.targetApi & apiFilter)) { + return; + } + optimizer.RegisterPass(std::move(pass)); + }; - optimizer - .RegisterPass(CreateInlineExhaustivePass()) - .RegisterPass(CreateEliminateDeadFunctionsPass()) - .RegisterPass(CreatePrivateToLocalPass()) - .RegisterPass(CreateScalarReplacementPass(0)) - .RegisterPass(CreateLocalMultiStoreElimPass()) - .RegisterPass(CreateCCPPass()) - .RegisterPass(CreateLoopUnrollPass(true)) - .RegisterPass(CreateDeadBranchElimPass()) - .RegisterPass(CreateSimplificationPass()) - .RegisterPass(CreateScalarReplacementPass(0)) - .RegisterPass(CreateLocalSingleStoreElimPass()) - .RegisterPass(CreateIfConversionPass()) - .RegisterPass(CreateSimplificationPass()) - .RegisterPass(CreateAggressiveDCEPass()) - .RegisterPass(CreateDeadBranchElimPass()) - .RegisterPass(CreateBlockMergePass()) - .RegisterPass(CreateLocalAccessChainConvertPass()) - .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass()) - .RegisterPass(CreateAggressiveDCEPass()) - .RegisterPass(CreateCopyPropagateArraysPass()) - .RegisterPass(CreateVectorDCEPass()) - .RegisterPass(CreateDeadInsertElimPass()) - // this breaks UBO layout - //.RegisterPass(CreateEliminateDeadMembersPass()) - .RegisterPass(CreateLocalSingleStoreElimPass()) - .RegisterPass(CreateBlockMergePass()) - .RegisterPass(CreateLocalMultiStoreElimPass()) - .RegisterPass(CreateRedundancyEliminationPass()) - .RegisterPass(CreateSimplificationPass()) - .RegisterPass(CreateAggressiveDCEPass()) - .RegisterPass(CreateCFGCleanupPass()); + RegisterPass(CreateWrapOpKillPass()); + RegisterPass(CreateDeadBranchElimPass()); + RegisterPass(CreateMergeReturnPass(), MaterialBuilder::TargetApi::METAL); + RegisterPass(CreateInlineExhaustivePass()); + RegisterPass(CreateEliminateDeadFunctionsPass()); + RegisterPass(CreatePrivateToLocalPass()); + RegisterPass(CreateScalarReplacementPass(0)); + RegisterPass(CreateLocalMultiStoreElimPass()); + RegisterPass(CreateCCPPass()); + RegisterPass(CreateLoopUnrollPass(true)); + RegisterPass(CreateDeadBranchElimPass()); + RegisterPass(CreateSimplificationPass(), MaterialBuilder::TargetApi::METAL); + RegisterPass(CreateScalarReplacementPass(0)); + RegisterPass(CreateLocalSingleStoreElimPass()); + RegisterPass(CreateIfConversionPass()); + RegisterPass(CreateSimplificationPass(), MaterialBuilder::TargetApi::METAL); + RegisterPass(CreateAggressiveDCEPass()); + RegisterPass(CreateDeadBranchElimPass()); + RegisterPass(CreateBlockMergePass()); + RegisterPass(CreateLocalAccessChainConvertPass()); + RegisterPass(CreateLocalSingleBlockLoadStoreElimPass()); + RegisterPass(CreateAggressiveDCEPass()); + RegisterPass(CreateCopyPropagateArraysPass()); + RegisterPass(CreateVectorDCEPass()); + RegisterPass(CreateDeadInsertElimPass()); + // this breaks UBO layout + //RegisterPass(CreateEliminateDeadMembersPass()); + RegisterPass(CreateLocalSingleStoreElimPass()); + RegisterPass(CreateBlockMergePass()); + RegisterPass(CreateLocalMultiStoreElimPass()); + RegisterPass(CreateRedundancyEliminationPass()); + RegisterPass(CreateSimplificationPass(), MaterialBuilder::TargetApi::METAL); + RegisterPass(CreateAggressiveDCEPass()); + RegisterPass(CreateCFGCleanupPass()); } } // namespace filamat From 549c58228734deeb5f5ac5a9171eaafb93304c63 Mon Sep 17 00:00:00 2001 From: mackong Date: Tue, 1 Aug 2023 06:49:48 +0800 Subject: [PATCH 15/19] engine: support setDepthFunc for MaterialInstance (#7004) Co-authored-by: Mathias Agopian --- NEW_RELEASE_NOTES.md | 5 ++++- .../src/main/cpp/MaterialInstance.cpp | 16 ++++++++++++++++ .../android/filament/MaterialInstance.java | 18 ++++++++++++++++++ filament/include/filament/MaterialInstance.h | 11 +++++++++++ filament/src/MaterialInstance.cpp | 8 ++++++++ filament/src/details/MaterialInstance.h | 4 ++++ samples/depthtesting.cpp | 19 ++++++++++++++++++- web/filament-js/filament.d.ts | 1 + web/filament-js/jsbindings.cpp | 2 ++ 9 files changed, 82 insertions(+), 2 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 828fc267167..9dda512353d 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -12,4 +12,7 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). - opengl: fix b/290388359 : possible crash when shutting down the engine - engine: Improve precision of frame time measurement when using emulated TimerQueries - backend: Improve frame pacing on Android and Vulkan. -- backend: workaround b/291140208 (gltf_viewer crashes on Nexus 6P) \ No newline at end of file +- backend: workaround b/291140208 (gltf_viewer crashes on Nexus 6P) +- engine: support `setDepthFunc` for `MaterialInstance` +- web: Added setDepthFunc()/getDepthFunc() to MaterialInstance +- android: Added setDepthFunc()/getDepthFunc() to MaterialInstance diff --git a/android/filament-android/src/main/cpp/MaterialInstance.cpp b/android/filament-android/src/main/cpp/MaterialInstance.cpp index 5273dc2c808..d692689e94c 100644 --- a/android/filament-android/src/main/cpp/MaterialInstance.cpp +++ b/android/filament-android/src/main/cpp/MaterialInstance.cpp @@ -357,6 +357,14 @@ Java_com_google_android_filament_MaterialInstance_nSetDepthCulling(JNIEnv*, instance->setDepthCulling(enable); } +extern "C" +JNIEXPORT void JNICALL +Java_com_google_android_filament_MaterialInstance_nSetDepthFunc(JNIEnv*, + jclass, jlong nativeMaterialInstance, jlong function) { + MaterialInstance* instance = (MaterialInstance*) nativeMaterialInstance; + instance->setDepthFunc(static_cast(function)); +} + extern "C" JNIEXPORT void JNICALL Java_com_google_android_filament_MaterialInstance_nSetStencilCompareFunction(JNIEnv*, jclass, @@ -524,3 +532,11 @@ Java_com_google_android_filament_MaterialInstance_nIsDepthCullingEnabled(JNIEnv* MaterialInstance* instance = (MaterialInstance*)nativeMaterialInstance; return instance->isDepthCullingEnabled(); } + +extern "C" +JNIEXPORT jint JNICALL +Java_com_google_android_filament_MaterialInstance_nGetDepthFunc(JNIEnv* env, jclass clazz, + jlong nativeMaterialInstance) { + MaterialInstance* instance = (MaterialInstance*)nativeMaterialInstance; + return (jint)instance->getDepthFunc(); +} diff --git a/android/filament-android/src/main/java/com/google/android/filament/MaterialInstance.java b/android/filament-android/src/main/java/com/google/android/filament/MaterialInstance.java index bde26df5ac1..9f30568f52b 100644 --- a/android/filament-android/src/main/java/com/google/android/filament/MaterialInstance.java +++ b/android/filament-android/src/main/java/com/google/android/filament/MaterialInstance.java @@ -625,6 +625,15 @@ public void setDepthCulling(boolean enable) { nSetDepthCulling(getNativeObject(), enable); } + /** + * Sets the depth comparison function (default is {@link TextureSampler.CompareFunction#GE}). + * + * @param func the depth comparison function + */ + public void setDepthFunc(TextureSampler.CompareFunction func) { + nSetDepthFunc(getNativeObject(), func.ordinal()); + } + /** * Returns whether depth culling is enabled. */ @@ -632,6 +641,13 @@ public boolean isDepthCullingEnabled() { return nIsDepthCullingEnabled(getNativeObject()); } + /** + * Returns the depth comparison function. + */ + public TextureSampler.CompareFunction getDepthFunc() { + return TextureSampler.EnumCache.sCompareFunctionValues[nGetDepthFunc(getNativeObject())]; + } + /** * Sets the stencil comparison function (default is {@link TextureSampler.CompareFunction#ALWAYS}). * @@ -908,6 +924,7 @@ private static native void nSetSpecularAntiAliasingThreshold(long nativeMaterial private static native void nSetDepthWrite(long nativeMaterialInstance, boolean enable); private static native void nSetStencilWrite(long nativeMaterialInstance, boolean enable); private static native void nSetDepthCulling(long nativeMaterialInstance, boolean enable); + private static native void nSetDepthFunc(long nativeMaterialInstance, long function); private static native void nSetStencilCompareFunction(long nativeMaterialInstance, long function, long face); @@ -939,4 +956,5 @@ private static native void nSetStencilWriteMask(long nativeMaterialInstance, int private static native boolean nIsDepthWriteEnabled(long nativeMaterialInstance); private static native boolean nIsStencilWriteEnabled(long nativeMaterialInstance); private static native boolean nIsDepthCullingEnabled(long nativeMaterialInstance); + private static native int nGetDepthFunc(long nativeMaterialInstance); } diff --git a/filament/include/filament/MaterialInstance.h b/filament/include/filament/MaterialInstance.h index c2095e1c477..ee7a8e252ff 100644 --- a/filament/include/filament/MaterialInstance.h +++ b/filament/include/filament/MaterialInstance.h @@ -52,6 +52,7 @@ class UTILS_PUBLIC MaterialInstance : public FilamentAPI { public: using CullingMode = filament::backend::CullingMode; using TransparencyMode = filament::TransparencyMode; + using DepthFunc = filament::backend::SamplerCompareFunc; using StencilCompareFunc = filament::backend::SamplerCompareFunc; using StencilOperation = filament::backend::StencilOperation; using StencilFace = filament::backend::StencilFace; @@ -367,6 +368,16 @@ class UTILS_PUBLIC MaterialInstance : public FilamentAPI { */ void setDepthCulling(bool enable) noexcept; + /** + * Overrides the default depth function state that was set on the material. + */ + void setDepthFunc(DepthFunc depthFunc) noexcept; + + /** + * Returns the depth function state. + */ + DepthFunc getDepthFunc() const noexcept; + /** * Returns whether depth culling is enabled. */ diff --git a/filament/src/MaterialInstance.cpp b/filament/src/MaterialInstance.cpp index ec3e3ea931c..33f3ab764b1 100644 --- a/filament/src/MaterialInstance.cpp +++ b/filament/src/MaterialInstance.cpp @@ -255,6 +255,14 @@ void MaterialInstance::setDepthCulling(bool enable) noexcept { downcast(this)->setDepthCulling(enable); } +void MaterialInstance::setDepthFunc(DepthFunc depthFunc) noexcept { + downcast(this)->setDepthFunc(depthFunc); +} + +MaterialInstance::DepthFunc MaterialInstance::getDepthFunc() const noexcept { + return downcast(this)->getDepthFunc(); +} + void MaterialInstance::setStencilWrite(bool enable) noexcept { downcast(this)->setStencilWrite(enable); } diff --git a/filament/src/details/MaterialInstance.h b/filament/src/details/MaterialInstance.h index c98c8f3115e..6be23b7e06c 100644 --- a/filament/src/details/MaterialInstance.h +++ b/filament/src/details/MaterialInstance.h @@ -101,6 +101,10 @@ class FMaterialInstance : public MaterialInstance { backend::RasterState::DepthFunc getDepthFunc() const noexcept { return mDepthFunc; } + void setDepthFunc(backend::RasterState::DepthFunc depthFunc) noexcept { + mDepthFunc = depthFunc; + } + void setPolygonOffset(float scale, float constant) noexcept { // handle reversed Z mPolygonOffset = { -scale, -constant }; diff --git a/samples/depthtesting.cpp b/samples/depthtesting.cpp index 8b678466761..77fae2233f1 100644 --- a/samples/depthtesting.cpp +++ b/samples/depthtesting.cpp @@ -26,6 +26,8 @@ #include #include +#include + #include #include @@ -48,6 +50,7 @@ struct App { Skybox* skybox; Entity whiteTriangle; Entity colorTriangle; + MaterialInstance::DepthFunc depthFunc; }; struct Vertex { @@ -115,8 +118,11 @@ int main(int argc, char** argv) { .culling(false) .receiveShadows(false) .castShadows(false) + .priority(5) // draw after whiteTriangle. .build(*engine, app.colorTriangle); scene->addEntity(app.colorTriangle); + + app.depthFunc = MaterialInstance::DepthFunc::GE; }; auto cleanup = [&app](Engine* engine, View*, Scene*) { @@ -131,6 +137,17 @@ int main(int argc, char** argv) { utils::EntityManager::get().destroy(app.camera); }; + auto gui = [&app](Engine* engine, View* view) { + int depthFuncSelection = (int) app.depthFunc; + ImGui::Combo("Depth Function", &depthFuncSelection, + "Less or equal\0Greater or equal\0Strictly less than\0" + "Strictly greater than\0Equal\0Not equal\0Always\0Never\0\0"); + if (depthFuncSelection != (int) app.depthFunc) { + app.depthFunc = (MaterialInstance::DepthFunc) depthFuncSelection; + app.mat->getDefaultInstance()->setDepthFunc(app.depthFunc); + } + }; + FilamentApp::get().animate([&app](Engine* engine, View* view, double now) { constexpr float ZOOM = 1.5f; const uint32_t w = view->getViewport().width; @@ -144,7 +161,7 @@ int main(int argc, char** argv) { filament::math::mat4f::rotation(now, filament::math::float3{ 0, 1, 0 })); }); - FilamentApp::get().run(config, setup, cleanup); + FilamentApp::get().run(config, setup, cleanup, gui); return 0; } diff --git a/web/filament-js/filament.d.ts b/web/filament-js/filament.d.ts index d0bfaa0372a..16d6e2f3831 100644 --- a/web/filament-js/filament.d.ts +++ b/web/filament-js/filament.d.ts @@ -183,6 +183,7 @@ export class MaterialInstance { public setDepthWrite(enable: boolean): void; public setStencilWrite(enable: boolean): void; public setDepthCulling(enable: boolean): void; + public setDepthFunc(func: CompareFunc): void; public setStencilCompareFunction(func: CompareFunc, face?: StencilFace): void; public setStencilOpStencilFail(op: StencilOperation, face?: StencilFace): void; public setStencilOpDepthFail(op: StencilOperation, face?: StencilFace): void; diff --git a/web/filament-js/jsbindings.cpp b/web/filament-js/jsbindings.cpp index ce4401e9763..201ca0c4f24 100644 --- a/web/filament-js/jsbindings.cpp +++ b/web/filament-js/jsbindings.cpp @@ -1371,6 +1371,8 @@ class_("MaterialInstance") .function("setStencilWrite", &MaterialInstance::setStencilWrite) .function("setDepthCulling", &MaterialInstance::setDepthCulling) .function("isDepthCullingEnabled", &MaterialInstance::isDepthCullingEnabled) + .function("setDepthFunc", &MaterialInstance::setDepthFunc) + .function("getDepthFunc", &MaterialInstance::getDepthFunc) .function("setStencilCompareFunction", &MaterialInstance::setStencilCompareFunction) .function("setStencilCompareFunction", EMBIND_LAMBDA(void, (MaterialInstance* self, MaterialInstance::StencilCompareFunc func), { From eb18d75b2e3b47e4d4cab31d6d0994e21c1d2d90 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Tue, 1 Aug 2023 15:36:13 -0700 Subject: [PATCH 16/19] Release Filament 1.40.4 --- NEW_RELEASE_NOTES.md | 9 --------- README.md | 4 ++-- RELEASE_NOTES.md | 12 ++++++++++++ android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 9dda512353d..4a1a9c7fa7e 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,12 +7,3 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut -- backend: Disable timer queries on all Mali GPUs (fixes b/233754398) -- engine: Add a way to query the validity of most filament objects (see `Engine::isValid`) -- opengl: fix b/290388359 : possible crash when shutting down the engine -- engine: Improve precision of frame time measurement when using emulated TimerQueries -- backend: Improve frame pacing on Android and Vulkan. -- backend: workaround b/291140208 (gltf_viewer crashes on Nexus 6P) -- engine: support `setDepthFunc` for `MaterialInstance` -- web: Added setDepthFunc()/getDepthFunc() to MaterialInstance -- android: Added setDepthFunc()/getDepthFunc() to MaterialInstance diff --git a/README.md b/README.md index 26d893bb43a..f1962fa0748 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.40.3' + implementation 'com.google.android.filament:filament-android:1.40.4' } ``` @@ -50,7 +50,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.3' +pod 'Filament', '~> 1.40.4' ``` ### Snapshots diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a7e0930c5ae..c1827db87a2 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,18 @@ 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.40.5 + +- backend: Disable timer queries on all Mali GPUs (fixes b/233754398) +- engine: Add a way to query the validity of most filament objects (see `Engine::isValid`) +- opengl: fix b/290388359 : possible crash when shutting down the engine +- engine: Improve precision of frame time measurement when using emulated TimerQueries +- backend: Improve frame pacing on Android and Vulkan. +- backend: workaround b/291140208 (gltf_viewer crashes on Nexus 6P) +- engine: support `setDepthFunc` for `MaterialInstance` +- web: Added setDepthFunc()/getDepthFunc() to MaterialInstance +- android: Added setDepthFunc()/getDepthFunc() to MaterialInstance + ## v1.40.4 - gltfio: fix crash when compute morph target without material diff --git a/android/gradle.properties b/android/gradle.properties index a734dc6aa09..879f309c2ca 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.40.3 +VERSION_NAME=1.40.4 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 0bddbf53351..86e39c076d6 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.40.3" + spec.version = "1.40.4" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.40.3/filament-v1.40.3-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.40.4/filament-v1.40.4-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/web/filament-js/package.json b/web/filament-js/package.json index 8fde612eaf4..4745c2a343f 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.40.3", + "version": "1.40.4", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 59063fb7b49ef3453ca86a7e598a1e7dd3615535 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Tue, 1 Aug 2023 15:39:35 -0700 Subject: [PATCH 17/19] Bump version to 1.40.5 --- README.md | 4 ++-- android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index f1962fa0748..dc21159e4d2 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.40.4' + implementation 'com.google.android.filament:filament-android:1.40.5' } ``` @@ -50,7 +50,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.4' +pod 'Filament', '~> 1.40.5' ``` ### Snapshots diff --git a/android/gradle.properties b/android/gradle.properties index 879f309c2ca..d017be996c6 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.40.4 +VERSION_NAME=1.40.5 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 86e39c076d6..8a31ccd6333 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.40.4" + spec.version = "1.40.5" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.40.4/filament-v1.40.4-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.40.5/filament-v1.40.5-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/web/filament-js/package.json b/web/filament-js/package.json index 4745c2a343f..b08a19a8b53 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.40.4", + "version": "1.40.5", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 25c08f19e3b174ca3a958a17f721a3e5a41671be Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 2 Aug 2023 11:00:48 -0700 Subject: [PATCH 18/19] vulkan: fix TSAN in readpixels (#7023) (#7028) --- filament/backend/src/vulkan/VulkanReadPixels.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/src/vulkan/VulkanReadPixels.cpp b/filament/backend/src/vulkan/VulkanReadPixels.cpp index 7940056964f..e299d206597 100644 --- a/filament/backend/src/vulkan/VulkanReadPixels.cpp +++ b/filament/backend/src/vulkan/VulkanReadPixels.cpp @@ -57,8 +57,8 @@ void TaskHandler::drain() { { std::unique_lock lock(syncPointMutex); done = true; + syncCondition.notify_one(); } - syncCondition.notify_one(); }); std::unique_lock lock(syncPointMutex); From bbad75a012dd622b23cfc41378c350126998de86 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Fri, 4 Aug 2023 11:13:57 -0700 Subject: [PATCH 19/19] vulkan: fix fence initialization (#7038) Previously, we have a VulkanSync with a default constructor that allows us to have sync objects that returns error when actual fences are not yet present. We need to replicate that with VulkanFence since sync objects have been removed from the API. Fixes #7034 --- filament/backend/src/vulkan/VulkanDriver.cpp | 10 ++++++++-- filament/backend/src/vulkan/VulkanHandles.h | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 30573ddf194..14215434975 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -503,7 +503,7 @@ void VulkanDriver::destroyRenderTarget(Handle rth) { void VulkanDriver::createFenceR(Handle fh, int) { VulkanCommandBuffer const& commandBuffer = mCommands->get(); - construct(fh, commandBuffer); + construct(fh, commandBuffer.fence); } void VulkanDriver::createSwapChainR(Handle sch, void* nativeWindow, uint64_t flags) { @@ -579,7 +579,7 @@ Handle VulkanDriver::createRenderTargetS() noexcept { } Handle VulkanDriver::createFenceS() noexcept { - return allocHandle(); + return initHandle(); } Handle VulkanDriver::createSwapChainS() noexcept { @@ -664,6 +664,12 @@ void VulkanDriver::destroyFence(Handle fh) { FenceStatus VulkanDriver::wait(Handle fh, uint64_t timeout) { auto& cmdfence = handle_cast(fh)->fence; + if (!cmdfence) { + // If wait is called before a fence actually exists, we return timeout. This matches the + // current behavior in OpenGLDriver, but we should eventually reconsider a different error + // code. + return FenceStatus::TIMEOUT_EXPIRED; + } // Internally we use the VK_INCOMPLETE status to mean "not yet submitted". // When this fence gets submitted, its status changes to VK_NOT_READY. diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index fb82e78f738..fe5fc2227e0 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -129,7 +129,9 @@ struct VulkanRenderPrimitive : public HwRenderPrimitive { }; struct VulkanFence : public HwFence { - explicit VulkanFence(const VulkanCommandBuffer& commands) : fence(commands.fence) {} + VulkanFence() = default; + explicit VulkanFence(std::shared_ptr fence) : fence(fence) {} + std::shared_ptr fence; };