From b48a1556b6cc16c8a8a48d048388a1148cb489ec Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 2 Dec 2024 23:37:19 -0800 Subject: [PATCH 01/12] Fix a couple depthClamp issues on metal - depthClamp is only supported from iOS 11.0 - reset the depthClamp state at the beginning of the renderpass BUGS=[379729888] --- filament/backend/src/metal/MetalContext.h | 1 + filament/backend/src/metal/MetalDriver.mm | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/filament/backend/src/metal/MetalContext.h b/filament/backend/src/metal/MetalContext.h index e8acf71fbc6b..158435150820 100644 --- a/filament/backend/src/metal/MetalContext.h +++ b/filament/backend/src/metal/MetalContext.h @@ -135,6 +135,7 @@ struct MetalContext { bool supportsTextureSwizzling = false; bool supportsAutoDepthResolve = false; bool supportsMemorylessRenderTargets = false; + bool supportsDepthClamp = false; uint8_t maxColorRenderTargets = 4; struct { uint8_t common; diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index bee3ba883553..d787c0cdb265 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -151,6 +151,11 @@ mContext->highestSupportedGpuFamily.mac >= 2; // newer macOS GPUs } + mContext->supportsDepthClamp = false; + if (@available(macOS 10.11, iOS 11.0, *)) { + mContext->supportsDepthClamp = true; + } + // In order to support resolve store action on depth attachment, the GPU needs to support it. // Note that support for depth resolve implies support for stencil resolve using .sample0 resolve filter. // (Other resolve filters are supported starting .apple5 and .mac2 families). @@ -1063,7 +1068,7 @@ } bool MetalDriver::isDepthClampSupported() { - return true; + return mContext->supportsDepthClamp; } bool MetalDriver::isWorkaroundNeeded(Workaround workaround) { @@ -1258,6 +1263,7 @@ mContext->cullModeState.invalidate(); mContext->windingState.invalidate(); mContext->scissorRectState.invalidate(); + mContext->depthClampState.invalidate(); mContext->currentPolygonOffset = {0.0f, 0.0f}; mContext->finalizedDescriptorSets.clear(); @@ -1729,10 +1735,12 @@ } // depth clip mode - MTLDepthClipMode depthClipMode = rs.depthClamp ? MTLDepthClipModeClamp : MTLDepthClipModeClip; - mContext->depthClampState.updateState(depthClipMode); - if (mContext->depthClampState.stateChanged()) { - [mContext->currentRenderPassEncoder setDepthClipMode:depthClipMode]; + if (mContext->supportsDepthClamp) { + MTLDepthClipMode depthClipMode = rs.depthClamp ? MTLDepthClipModeClamp : MTLDepthClipModeClip; + mContext->depthClampState.updateState(depthClipMode); + if (mContext->depthClampState.stateChanged()) { + [mContext->currentRenderPassEncoder setDepthClipMode:depthClipMode]; + } } // Set the depth-stencil state, if a state change is needed. From 4ea3f7476d4914503b057c4f3517be8b197a1710 Mon Sep 17 00:00:00 2001 From: feelamee <106318766+feelamee@users.noreply.github.com> Date: Wed, 4 Dec 2024 23:03:50 +0000 Subject: [PATCH 02/12] Fix typo in Camera doc comment --- filament/include/filament/Camera.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/include/filament/Camera.h b/filament/include/filament/Camera.h index 74f34af0fec6..91d51377f5c2 100644 --- a/filament/include/filament/Camera.h +++ b/filament/include/filament/Camera.h @@ -61,7 +61,7 @@ namespace filament { * filament::Camera* myCamera = engine->createCamera(myCameraEntity); * myCamera->setProjection(45, 16.0/9.0, 0.1, 1.0); * myCamera->lookAt({0, 1.60, 1}, {0, 0, 0}); - * engine->destroyCameraComponent(myCamera); + * engine->destroyCameraComponent(myCameraEntity); * ~~~~~~~~~~~ * * From 6bb7f890ef58da2484e8834cfbf7a9c89e7d7dc3 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 5 Dec 2024 13:18:20 -0800 Subject: [PATCH 03/12] pre-populate the default material's depth variants This restores the old behavior with depth variant caching. We pay the price at engine init time, instead of when the variant is needed the first time. We can revisit this later. Note that the default material doesn't have all possible depth variants (e.g. VSM), but that pre-caches the most popular ones. BUGS=[381946222] --- filament/src/details/Material.cpp | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index c5d6b83f42ec..5f641eb167f2 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -605,10 +605,13 @@ void FMaterial::createAndCacheProgram(Program&& p, Variant variant) const noexce FEngine const& engine = mEngine; DriverApi& driverApi = mEngine.getDriverApi(); - // Check if the default material has this program cached - if (mMaterialDomain == MaterialDomain::SURFACE && + bool const isSharedVariant = + (mMaterialDomain == MaterialDomain::SURFACE) && !mIsDefaultMaterial && !mHasCustomDepthShader && - Variant::isValidDepthVariant(variant)) { + Variant::isValidDepthVariant(variant); + + // Check if the default material has this program cached + if (isSharedVariant) { FMaterial const* const pDefaultMaterial = engine.getDefaultMaterial(); if (pDefaultMaterial) { auto program = pDefaultMaterial->mCachedPrograms[variant.key]; @@ -627,9 +630,7 @@ void FMaterial::createAndCacheProgram(Program&& p, Variant variant) const noexce // If the default material doesn't already have this program cached, and all caching conditions // are met (Surface Domain and no custom depth shader), cache it now. // New Materials will inherit these program automatically. - if (mMaterialDomain == MaterialDomain::SURFACE && - !mIsDefaultMaterial && !mHasCustomDepthShader && - Variant::isValidDepthVariant(variant)) { + if (isSharedVariant) { FMaterial const* const pDefaultMaterial = engine.getDefaultMaterial(); if (pDefaultMaterial && !pDefaultMaterial->mCachedPrograms[variant.key]) { // set the tag to the default material name @@ -1076,6 +1077,21 @@ void FMaterial::processPushConstants(FEngine& engine, MaterialParser const* pars } void FMaterial::precacheDepthVariants(FEngine& engine) { + // pre-cache all depth variants inside the default material. Note that this should be + // entirely optional; if we remove this pre-caching, these variants will be populated + // later, when/if needed by createAndCacheProgram(). Doing it now potentially uses more + // memory and increases init time, but reduces hiccups during the first frame. + if (UTILS_UNLIKELY(mIsDefaultMaterial)) { + auto const allDepthVariants = VariantUtils::getDepthVariants(); + for (auto const variant: allDepthVariants) { + assert_invariant(Variant::isValidDepthVariant(variant)); + if (hasVariant(variant)) { + prepareProgram(variant); + } + } + return; + } + // if possible pre-cache all depth variants from the default material if (mMaterialDomain == MaterialDomain::SURFACE && !mIsDefaultMaterial && From 6b7080e8b69f808df37eb6493cc73a2925e5f987 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 3 Dec 2024 23:02:40 -0800 Subject: [PATCH 04/12] fix a use-after-free of texture handles Because ColorPassDescriptorSet is used for both the color pass and the picking/ssr/structure passes, the texture it holds can be stale. In this CL we fix this by reseting the offending textures when preparing the picking/ssr/structure passes. This has a side effect of recreating the descriptor-set internally. A better fix would be to use separate descriptor-sets for these two cases and rely on the texture cache to avoid recreation from a frame to the next. BUGS=[376705346] --- filament/src/details/Renderer.cpp | 5 +++++ filament/src/details/View.cpp | 4 ++++ filament/src/details/View.h | 1 + filament/src/ds/ColorPassDescriptorSet.cpp | 11 +++++++++++ filament/src/ds/ColorPassDescriptorSet.h | 2 ++ 5 files changed, 23 insertions(+) diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index eec97677948a..6bdd39daa836 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -963,6 +963,11 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { uint32_t(float(xvp.width ) * aoOptions.resolution), uint32_t(float(xvp.height) * aoOptions.resolution)}); + // this needs to reset the sampler that are only set in RendererUtils::colorPass(), because + // this descriptor-set is also used for ssr/picking/structure and these could be stale + // it would be better to use a separate desriptor-set for those two cases so that we don't + // have to do this + view.unbindSamplers(driver); view.commitUniformsAndSamplers(driver); }); diff --git a/filament/src/details/View.cpp b/filament/src/details/View.cpp index e7414f2a1919..36e38f0153ef 100644 --- a/filament/src/details/View.cpp +++ b/filament/src/details/View.cpp @@ -912,6 +912,10 @@ void FView::commitUniformsAndSamplers(DriverApi& driver) const noexcept { mColorPassDescriptorSet.commit(driver); } +void FView::unbindSamplers(DriverApi& driver) noexcept { + mColorPassDescriptorSet.unbindSamplers(driver); +} + void FView::commitFroxels(DriverApi& driverApi) const noexcept { if (mHasDynamicLighting) { mFroxelizer.commit(driverApi); diff --git a/filament/src/details/View.h b/filament/src/details/View.h index 1f41bab82350..715ebfd1a554 100644 --- a/filament/src/details/View.h +++ b/filament/src/details/View.h @@ -174,6 +174,7 @@ class FView : public View { void commitFroxels(backend::DriverApi& driverApi) const noexcept; void commitUniformsAndSamplers(backend::DriverApi& driver) const noexcept; + void unbindSamplers(backend::DriverApi& driver) noexcept; utils::JobSystem::Job* getFroxelizerSync() const noexcept { return mFroxelizerSync; } void setFroxelizerSync(utils::JobSystem::Job* sync) noexcept { mFroxelizerSync = sync; } diff --git a/filament/src/ds/ColorPassDescriptorSet.cpp b/filament/src/ds/ColorPassDescriptorSet.cpp index 18d3f495d6c9..03f8924bb9cb 100644 --- a/filament/src/ds/ColorPassDescriptorSet.cpp +++ b/filament/src/ds/ColorPassDescriptorSet.cpp @@ -541,6 +541,17 @@ void ColorPassDescriptorSet::commit(backend::DriverApi& driver) noexcept { } } +void ColorPassDescriptorSet::unbindSamplers(DriverApi&) noexcept { + // this needs to reset the sampler that are only set in RendererUtils::colorPass(), because + // this descriptor-set is also used for ssr/picking/structure and these could be stale + // it would be better to use a separate descriptor-set for those two cases so that we don't + // have to do this + setSampler(+PerViewBindingPoints::STRUCTURE, {}, {}); + setSampler(+PerViewBindingPoints::SHADOW_MAP, {}, {}); + setSampler(+PerViewBindingPoints::SSAO, {}, {}); + setSampler(+PerViewBindingPoints::SSR, {}, {}); +} + void ColorPassDescriptorSet::setSampler(backend::descriptor_binding_t binding, TextureHandle th, SamplerParams params) noexcept { for (size_t i = 0; i < DESCRIPTOR_LAYOUT_COUNT; i++) { diff --git a/filament/src/ds/ColorPassDescriptorSet.h b/filament/src/ds/ColorPassDescriptorSet.h index 79086753d75f..f0c5478605e5 100644 --- a/filament/src/ds/ColorPassDescriptorSet.h +++ b/filament/src/ds/ColorPassDescriptorSet.h @@ -158,6 +158,8 @@ class ColorPassDescriptorSet { // update local data into GPU UBO void commit(backend::DriverApi& driver) noexcept; + void unbindSamplers(backend::DriverApi& driver) noexcept; + // bind this UBO void bind(backend::DriverApi& driver, uint8_t index) const noexcept { mDescriptorSet[index].bind(driver, DescriptorSetBindingPoints::PER_VIEW); From 03fdadf417982067423e1b30414123e461596bf8 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 9 Dec 2024 15:04:26 -0800 Subject: [PATCH 05/12] Fix ARC objects leak in MetalShaderCompiler thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using async shader compilation, the compiler thread pool is a standard pthread and not an NSThread. Therefore, it needs a manual @autorelease pool, otherwise ARC objects never get truly released. 
 BUGS=[383167935] --- filament/backend/src/metal/MetalShaderCompiler.mm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/filament/backend/src/metal/MetalShaderCompiler.mm b/filament/backend/src/metal/MetalShaderCompiler.mm index 90d6a47721cc..0a5f9e345cc0 100644 --- a/filament/backend/src/metal/MetalShaderCompiler.mm +++ b/filament/backend/src/metal/MetalShaderCompiler.mm @@ -226,9 +226,11 @@ bool isReady() const noexcept { CompilerPriorityQueue const priorityQueue = program.getPriorityQueue(); mCompilerThreadPool.queue(priorityQueue, token, [this, name, device = mDevice, program = std::move(program), token]() { - MetalFunctionBundle compiledProgram = compileProgram(program, device); - token->set(compiledProgram); - mCallbackManager.put(token->handle); + @autoreleasepool { + MetalFunctionBundle compiledProgram = compileProgram(program, device); + token->set(compiledProgram); + mCallbackManager.put(token->handle); + } }); break; From 776a748e051503e40d250e5cfe4ac508b61613cf Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 9 Dec 2024 14:51:52 -0800 Subject: [PATCH 06/12] fix a ResourceList<> "soft" leak when destroying a material A ResourceList<> object was leaked when destroying a material until the Engine was shut down. This could however grow unbounded if churning through Materials. What was actually leaked was entries in the hashmap linking a material to its material instance list. BUGS=[383158161] --- filament/src/details/Engine.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index a9c39de272df..576e0e081a8e 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -1159,8 +1159,13 @@ bool FEngine::destroy(const FMaterial* ptr) { if (!ASSERT_PRECONDITION_NON_FATAL(pos->second.empty(), "destroying material \"%s\" but %u instances still alive", ptr->getName().c_str(), (*pos).second.size())) { - return false; + // We return true here, because the material has been destroyed. However, this + // leaks this material's ResourceList until the engine + // is shut down. Note that it's still possible for the MaterialInstances to be + // destroyed manually. + return true; } + mMaterialInstances.erase(pos); } } return success; From 94c1657ca05994200b66a095015ce661a0322597 Mon Sep 17 00:00:00 2001 From: Sungun Park Date: Wed, 11 Dec 2024 02:17:39 +0000 Subject: [PATCH 07/12] Release Filament 1.56.3 --- README.md | 4 ++-- RELEASE_NOTES.md | 3 +++ android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 0907d9794c27..5c04ba0bba93 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.56.2' + implementation 'com.google.android.filament:filament-android:1.56.3' } ``` @@ -51,7 +51,7 @@ Here are all the libraries available in the group `com.google.android.filament`: iOS projects can use CocoaPods to install the latest release: ```shell -pod 'Filament', '~> 1.56.2' +pod 'Filament', '~> 1.56.3' ``` ## Documentation diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 56727aa38e87..2500ee1c2122 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,9 @@ 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.56.4 + + ## v1.56.3 diff --git a/android/gradle.properties b/android/gradle.properties index a478149a0ea0..ad8a89ce2cb7 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.56.2 +VERSION_NAME=1.56.3 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 0d57d4405a5f..33fe0bf83649 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.56.2" + spec.version = "1.56.3" 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.56.2/filament-v1.56.2-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.56.3/filament-v1.56.3-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 8022284f92ca..dd5c85b6d95e 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.56.2", + "version": "1.56.3", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 93790cd06c8deb9b430b80872c59afe40d75ea7f Mon Sep 17 00:00:00 2001 From: Serge Metral Date: Wed, 11 Dec 2024 11:00:42 -0800 Subject: [PATCH 08/12] Android hardware buffer import in Vulkan (#8295) - Platform changes for the extensions - Platform changes for the processing of the AHB --- filament/backend/CMakeLists.txt | 6 +- .../backend/platforms/VulkanPlatform.h | 56 ++++ filament/backend/src/vulkan/VulkanDriver.cpp | 21 +- .../backend/src/vulkan/VulkanSwapChain.cpp | 11 +- filament/backend/src/vulkan/VulkanTexture.cpp | 5 +- filament/backend/src/vulkan/VulkanTexture.h | 4 +- .../src/vulkan/platform/VulkanPlatform.cpp | 19 ++ .../vulkan/platform/VulkanPlatformAndroid.cpp | 240 ++++++++++++++++++ .../vulkan/platform/VulkanPlatformApple.mm | 11 + ...ows.cpp => VulkanPlatformLinuxWindows.cpp} | 29 +-- 10 files changed, 375 insertions(+), 27 deletions(-) create mode 100644 filament/backend/src/vulkan/platform/VulkanPlatformAndroid.cpp rename filament/backend/src/vulkan/platform/{VulkanPlatformAndroidLinuxWindows.cpp => VulkanPlatformLinuxWindows.cpp} (91%) diff --git a/filament/backend/CMakeLists.txt b/filament/backend/CMakeLists.txt index c44643a3f566..2fcdfc6190d7 100644 --- a/filament/backend/CMakeLists.txt +++ b/filament/backend/CMakeLists.txt @@ -226,10 +226,12 @@ if (FILAMENT_SUPPORTS_VULKAN) src/vulkan/VulkanUtility.cpp src/vulkan/VulkanUtility.h ) - if (ANDROID OR LINUX OR WIN32) - list(APPEND SRCS src/vulkan/platform/VulkanPlatformAndroidLinuxWindows.cpp) + if (LINUX OR WIN32) + list(APPEND SRCS src/vulkan/platform/VulkanPlatformLinuxWindows.cpp) elseif (APPLE OR IOS) list(APPEND SRCS src/vulkan/platform/VulkanPlatformApple.mm) + elseif (ANDROID) + list(APPEND SRCS src/vulkan/platform/VulkanPlatformAndroid.cpp) endif() endif() diff --git a/filament/backend/include/backend/platforms/VulkanPlatform.h b/filament/backend/include/backend/platforms/VulkanPlatform.h index 91189310d573..d61818a8831d 100644 --- a/filament/backend/include/backend/platforms/VulkanPlatform.h +++ b/filament/backend/include/backend/platforms/VulkanPlatform.h @@ -292,8 +292,64 @@ class VulkanPlatform : public Platform, utils::PrivateImplementation; + virtual ImageData createExternalImage(void* externalImage, + const ExternalImageMetadata& metadata); + private: static ExtensionSet getSwapchainInstanceExtensions(); + static ExternalImageMetadata getExternalImageMetadataImpl(void* externalImage, + VkDevice device); + static ImageData createExternalImageImpl(void* externalImage, VkDevice device, + const VkAllocationCallbacks* allocator, const ExternalImageMetadata& metadata); // Platform dependent helper methods using SurfaceBundle = std::tuple; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 9bfac220f4eb..25a2448b2918 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -544,7 +544,25 @@ void VulkanDriver::createTextureViewSwizzleR(Handle th, Handle th, backend::TextureFormat format, - uint32_t width, uint32_t height, backend::TextureUsage usage, void* image) { + uint32_t width, uint32_t height, backend::TextureUsage usage, void* externalImage) { + FVK_SYSTRACE_SCOPE(); + + const auto& metadata = mPlatform->getExternalImageMetadata(externalImage); + if (metadata.isProtected) { + usage |= backend::TextureUsage::PROTECTED; + } + + assert_invariant(width == metadata.width); + assert_invariant(height == metadata.height); + assert_invariant(getVkFormat(format) == metadata.format); + + const auto& data = mPlatform->createExternalImage(externalImage, metadata); + + auto texture = resource_ptr::make(&mResourceManager, th, mPlatform->getDevice(), + mAllocator, &mResourceManager, &mCommands, data.first, data.second, metadata.format, + 1, metadata.width, metadata.height, usage, mStagePool); + + texture.inc(); } void VulkanDriver::createTextureExternalImagePlaneR(Handle th, @@ -1172,6 +1190,7 @@ TimerQueryResult VulkanDriver::getTimerQueryValue(Handle tqh, uint } void VulkanDriver::setExternalImage(Handle th, void* image) { + } void VulkanDriver::setExternalImagePlane(Handle th, void* image, uint32_t plane) { diff --git a/filament/backend/src/vulkan/VulkanSwapChain.cpp b/filament/backend/src/vulkan/VulkanSwapChain.cpp index c1df76e76db8..ec015f1498e7 100644 --- a/filament/backend/src/vulkan/VulkanSwapChain.cpp +++ b/filament/backend/src/vulkan/VulkanSwapChain.cpp @@ -75,15 +75,16 @@ void VulkanSwapChain::update() { } for (auto const color: bundle.colors) { auto colorTexture = fvkmemory::resource_ptr::construct(mResourceManager, - device, mAllocator, mResourceManager, mCommands, color, bundle.colorFormat, 1, - bundle.extent.width, bundle.extent.height, TextureUsage::COLOR_ATTACHMENT, + device, mAllocator, mResourceManager, mCommands, color, VK_NULL_HANDLE, + bundle.colorFormat, 1, bundle.extent.width, bundle.extent.height, colorUsage, mStagePool); mColors.push_back(colorTexture); } - mDepth = fvkmemory::resource_ptr::construct(mResourceManager, device, mAllocator, - mResourceManager, mCommands, bundle.depth, bundle.depthFormat, 1, bundle.extent.width, - bundle.extent.height, TextureUsage::DEPTH_ATTACHMENT, mStagePool); + mDepth = fvkmemory::resource_ptr::construct(mResourceManager, device, + mAllocator, mResourceManager, mCommands, bundle.depth, VK_NULL_HANDLE, + bundle.depthFormat, 1, bundle.extent.width, bundle.extent.height, depthUsage, + mStagePool); mExtent = bundle.extent; } diff --git a/filament/backend/src/vulkan/VulkanTexture.cpp b/filament/backend/src/vulkan/VulkanTexture.cpp index 105ae8072d49..22139b0454c7 100644 --- a/filament/backend/src/vulkan/VulkanTexture.cpp +++ b/filament/backend/src/vulkan/VulkanTexture.cpp @@ -164,14 +164,15 @@ VulkanTextureState::VulkanTextureState(VkDevice device, VmaAllocator allocator, // Constructor for internally passed VkImage VulkanTexture::VulkanTexture(VkDevice device, VmaAllocator allocator, fvkmemory::ResourceManager* resourceManager, VulkanCommands* commands, VkImage image, - VkFormat format, uint8_t samples, uint32_t width, uint32_t height, TextureUsage tusage, - VulkanStagePool& stagePool) + VkDeviceMemory memory, VkFormat format, uint8_t samples, uint32_t width, + uint32_t height, TextureUsage tusage, VulkanStagePool& stagePool) : HwTexture(SamplerType::SAMPLER_2D, 1, samples, width, height, 1, TextureFormat::UNUSED, tusage), mState(fvkmemory::resource_ptr::construct(resourceManager, device, allocator, commands, stagePool, format, imgutil::getViewType(SamplerType::SAMPLER_2D), 1, 1, getDefaultLayoutImpl(tusage), any(usage & TextureUsage::PROTECTED))) { mState->mTextureImage = image; + mState->mTextureImageMemory = memory; mPrimaryViewRange = mState->mFullViewRange; } diff --git a/filament/backend/src/vulkan/VulkanTexture.h b/filament/backend/src/vulkan/VulkanTexture.h index e6f9e38ad400..1889adf1212a 100644 --- a/filament/backend/src/vulkan/VulkanTexture.h +++ b/filament/backend/src/vulkan/VulkanTexture.h @@ -94,8 +94,8 @@ struct VulkanTexture : public HwTexture, fvkmemory::Resource { // The texture will never destroy the given VkImage, but it does manages its subresources. VulkanTexture(VkDevice device, VmaAllocator allocator, fvkmemory::ResourceManager* resourceManager, VulkanCommands* commands, VkImage image, - VkFormat format, uint8_t samples, uint32_t width, uint32_t height, TextureUsage tusage, - VulkanStagePool& stagePool); + VkDeviceMemory memory, VkFormat format, uint8_t samples, uint32_t width, uint32_t height, + TextureUsage tusage, VulkanStagePool& stagePool); // Constructor for creating a texture view for wrt specific mip range VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, diff --git a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp index c894e18bff9f..33de86245a01 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp @@ -194,6 +194,15 @@ ExtensionSet getDeviceExtensions(VkPhysicalDevice device) { #if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) VK_EXT_DEBUG_MARKER_EXTENSION_NAME, #endif + // We only support external image for Android for now, but nothing bars us from + // supporting other platforms. +#if defined(__ANDROID__) + VK_KHR_EXTERNAL_MEMORY_EXTENSION_NAME, + VK_KHR_DEDICATED_ALLOCATION_EXTENSION_NAME, + VK_KHR_SAMPLER_YCBCR_CONVERSION_EXTENSION_NAME, + VK_ANDROID_EXTERNAL_MEMORY_ANDROID_HARDWARE_BUFFER_EXTENSION_NAME, + #endif + VK_KHR_PORTABILITY_SUBSET_EXTENSION_NAME, VK_KHR_MAINTENANCE1_EXTENSION_NAME, VK_KHR_MAINTENANCE2_EXTENSION_NAME, @@ -945,6 +954,16 @@ VkQueue VulkanPlatform::getProtectedGraphicsQueue() const noexcept { return mImpl->mProtectedGraphicsQueue; } +VulkanPlatform::ExternalImageMetadata VulkanPlatform::getExternalImageMetadata( + void* externalImage) { + return getExternalImageMetadataImpl(externalImage, mImpl->mDevice); +} + +VulkanPlatform::ImageData VulkanPlatform::createExternalImage(void* externalImage, + const ExternalImageMetadata& metadata) { + return createExternalImageImpl(externalImage, mImpl->mDevice, nullptr, metadata); +} + #undef SWAPCHAIN_RET_FUNC }// namespace filament::backend diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformAndroid.cpp b/filament/backend/src/vulkan/platform/VulkanPlatformAndroid.cpp new file mode 100644 index 000000000000..1675b9eaa1ba --- /dev/null +++ b/filament/backend/src/vulkan/platform/VulkanPlatformAndroid.cpp @@ -0,0 +1,240 @@ +/* + * Copyright (C) 2024 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 "vulkan/VulkanConstants.h" + +#include + +#include + +#include +#include + +using namespace bluevk; + +namespace filament::backend { + +namespace { +void getVKFormatAndUsage(const AHardwareBuffer_Desc& desc, VkFormat& format, + VkImageUsageFlags& usage, bool& isProtected) { + // Refer to "11.2.17. External Memory Handle Types" in the spec, and + // Tables 13/14 for how the following derivation works. + bool isDepthFormat = false; + isProtected = false; + switch (desc.format) { + case AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM: + format = VK_FORMAT_R8G8B8A8_UNORM; + break; + case AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM: + format = VK_FORMAT_R8G8B8A8_UNORM; + break; + case AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM: + format = VK_FORMAT_R8G8B8_UNORM; + break; + case AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM: + format = VK_FORMAT_R5G6B5_UNORM_PACK16; + break; + case AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT: + format = VK_FORMAT_R16G16B16A16_SFLOAT; + break; + case AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM: + format = VK_FORMAT_A2B10G10R10_UNORM_PACK32; + break; + case AHARDWAREBUFFER_FORMAT_D16_UNORM: + isDepthFormat = true; + format = VK_FORMAT_D16_UNORM; + break; + case AHARDWAREBUFFER_FORMAT_D24_UNORM: + isDepthFormat = true; + format = VK_FORMAT_X8_D24_UNORM_PACK32; + break; + case AHARDWAREBUFFER_FORMAT_D24_UNORM_S8_UINT: + isDepthFormat = true; + format = VK_FORMAT_D24_UNORM_S8_UINT; + break; + case AHARDWAREBUFFER_FORMAT_D32_FLOAT: + isDepthFormat = true; + format = VK_FORMAT_D32_SFLOAT; + break; + case AHARDWAREBUFFER_FORMAT_D32_FLOAT_S8_UINT: + isDepthFormat = true; + format = VK_FORMAT_D32_SFLOAT_S8_UINT; + break; + case AHARDWAREBUFFER_FORMAT_S8_UINT: + isDepthFormat = true; + format = VK_FORMAT_S8_UINT; + break; + default: + format = VK_FORMAT_UNDEFINED; + } + + // The following only concern usage flags derived from Table 14. + usage = 0; + if (desc.usage & AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE) { + usage |= VK_IMAGE_USAGE_SAMPLED_BIT; + usage |= VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT; + } + if (desc.usage & AHARDWAREBUFFER_USAGE_GPU_FRAMEBUFFER) { + if (isDepthFormat) { + usage |= VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT; + } else { + usage |= VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; + } + } + if (desc.usage & AHARDWAREBUFFER_USAGE_GPU_DATA_BUFFER) { + usage = VK_IMAGE_USAGE_STORAGE_BIT; + } + if (desc.usage & AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT) { + isProtected = true; + } +} + +VulkanPlatform::ImageData allocateExternalImage(void* externalBuffer, + VkDevice device, const VkAllocationCallbacks* allocator, + VulkanPlatform::ExternalImageMetadata const& metadata) { + VulkanPlatform::ImageData data; + AHardwareBuffer* buffer = static_cast(externalBuffer); + + // if external format we need to specifiy it in the allocation + const bool useExternalFormat = metadata.format == VK_FORMAT_UNDEFINED; + + const VkExternalFormatANDROID externalFormat = { + .sType = VK_STRUCTURE_TYPE_EXTERNAL_FORMAT_ANDROID, + .pNext = nullptr, + .externalFormat = metadata + .externalFormat,// pass down the format (external means we don't have it VK defined) + }; + const VkExternalMemoryImageCreateInfo externalCreateInfo = { + .sType = VK_STRUCTURE_TYPE_EXTERNAL_MEMORY_IMAGE_CREATE_INFO, + .pNext = useExternalFormat ? &externalFormat : nullptr, + .handleTypes = VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID, + }; + + VkImageCreateInfo imageInfo{.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO}; + imageInfo.pNext = &externalCreateInfo; + imageInfo.format = metadata.format; + imageInfo.imageType = VK_IMAGE_TYPE_2D; + imageInfo.extent = { + metadata.width, + metadata.height, + 1u, + }; + imageInfo.mipLevels = 1; + imageInfo.arrayLayers = metadata.layers; + imageInfo.usage = metadata.usage; + // In the unprotected case add R/W capabilities + if (metadata.isProtected == false) + imageInfo.usage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT; + + VkResult result = vkCreateImage(device, &imageInfo, allocator, &data.first); + FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) + << "vkCreateImage failed with error=" << result; + + // Allocate the memory + VkImportAndroidHardwareBufferInfoANDROID androidHardwareBufferInfo = { + .sType = VK_STRUCTURE_TYPE_IMPORT_ANDROID_HARDWARE_BUFFER_INFO_ANDROID, + .pNext = nullptr, + .buffer = buffer, + }; + VkMemoryDedicatedAllocateInfo memoryDedicatedAllocateInfo = { + .sType = VK_STRUCTURE_TYPE_MEMORY_DEDICATED_ALLOCATE_INFO, + .pNext = &androidHardwareBufferInfo, + .image = data.first, + .buffer = VK_NULL_HANDLE, + }; + VkMemoryAllocateInfo allocInfo = {.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, + .pNext = &memoryDedicatedAllocateInfo, + .allocationSize = metadata.allocationSize, + .memoryTypeIndex = metadata.memoryTypeBits}; + result = vkAllocateMemory(device, &allocInfo, allocator, &data.second); + FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) + << "vkAllocateMemory failed with error=" << result; + + return data; +} + +}// namespace + +VulkanPlatform::ExternalImageMetadata VulkanPlatform::getExternalImageMetadataImpl(void* externalImage, + VkDevice device) { + ExternalImageMetadata metadata; + AHardwareBuffer* buffer = static_cast(externalImage); + if (__builtin_available(android 26, *)) { + AHardwareBuffer_Desc bufferDesc; + AHardwareBuffer_describe(buffer, &bufferDesc); + metadata.width = bufferDesc.width; + metadata.height = bufferDesc.height; + metadata.layers = bufferDesc.layers; + + getVKFormatAndUsage(bufferDesc, metadata.format, metadata.usage, metadata.isProtected); + } + // In the unprotected case add R/W capabilities + if (metadata.isProtected == false) { + metadata.usage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT; + } + + VkAndroidHardwareBufferFormatPropertiesANDROID formatInfo = { + .sType = VK_STRUCTURE_TYPE_ANDROID_HARDWARE_BUFFER_FORMAT_PROPERTIES_ANDROID, + .pNext = nullptr, + }; + VkAndroidHardwareBufferPropertiesANDROID properties = { + .sType = VK_STRUCTURE_TYPE_ANDROID_HARDWARE_BUFFER_PROPERTIES_ANDROID, + .pNext = &formatInfo, + }; + VkResult result = vkGetAndroidHardwareBufferPropertiesANDROID(device, buffer, &properties); + FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) + << "vkGetAndroidHardwareBufferProperties failed with error=" << result; + + FILAMENT_CHECK_POSTCONDITION(metadata.format == formatInfo.format); + metadata.externalFormat = formatInfo.externalFormat; + metadata.allocationSize = properties.allocationSize; + metadata.memoryTypeBits = properties.memoryTypeBits; + return metadata; +} + +VulkanPlatform::ImageData VulkanPlatform::createExternalImageImpl(void* externalImage, + VkDevice device, const VkAllocationCallbacks* allocator, + const ExternalImageMetadata& metadata) { + ImageData data = allocateExternalImage(externalImage, device, allocator, metadata); + VkResult result = vkBindImageMemory(device, data.first, data.second, 0); + FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) + << "vkBindImageMemory error=" << result << "."; + return data; +} + +VulkanPlatform::ExtensionSet VulkanPlatform::getSwapchainInstanceExtensions() { + return { + VK_KHR_ANDROID_SURFACE_EXTENSION_NAME, + }; +} + +VulkanPlatform::SurfaceBundle VulkanPlatform::createVkSurfaceKHR(void* nativeWindow, + VkInstance instance, uint64_t flags) noexcept { + VkSurfaceKHR surface; + VkExtent2D extent; + + VkAndroidSurfaceCreateInfoKHR const createInfo{ + .sType = VK_STRUCTURE_TYPE_ANDROID_SURFACE_CREATE_INFO_KHR, + .window = (ANativeWindow*) nativeWindow, + }; + VkResult const result = + vkCreateAndroidSurfaceKHR(instance, &createInfo, VKALLOC, (VkSurfaceKHR*) &surface); + FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) + << "vkCreateAndroidSurfaceKHR with error=" << result; + return {surface, extent}; +} +} diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformApple.mm b/filament/backend/src/vulkan/platform/VulkanPlatformApple.mm index a540f2f7d751..6504ae3a7c47 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatformApple.mm +++ b/filament/backend/src/vulkan/platform/VulkanPlatformApple.mm @@ -63,6 +63,17 @@ return ret; } +VulkanPlatform::ExternalImageMetadata VulkanPlatform::getExternalImageMetadataImpl( + void* externalImage, VkDevice device) { + return {}; +} + +VulkanPlatform::ImageData VulkanPlatform::createExternalImageImpl(void* externalImage, + VkDevice device, const VkAllocationCallbacks* allocator, + const ExternalImageMetadata& metadata) { + return {}; +} + VulkanPlatform::SurfaceBundle VulkanPlatform::createVkSurfaceKHR(void* nativeWindow, VkInstance instance, uint64_t flags) noexcept { VkSurfaceKHR surface; diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformAndroidLinuxWindows.cpp b/filament/backend/src/vulkan/platform/VulkanPlatformLinuxWindows.cpp similarity index 91% rename from filament/backend/src/vulkan/platform/VulkanPlatformAndroidLinuxWindows.cpp rename to filament/backend/src/vulkan/platform/VulkanPlatformLinuxWindows.cpp index e7e12ac6759b..e198f1685b81 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatformAndroidLinuxWindows.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatformLinuxWindows.cpp @@ -34,9 +34,7 @@ #endif // Platform specific includes and defines -#if defined(__ANDROID__) - #include -#elif defined(__linux__) && defined(FILAMENT_SUPPORTS_WAYLAND) +#if defined(__linux__) && defined(FILAMENT_SUPPORTS_WAYLAND) #include namespace { typedef struct _wl { @@ -86,11 +84,20 @@ using namespace bluevk; namespace filament::backend { +VulkanPlatform::ExternalImageMetadata VulkanPlatform::getExternalImageMetadataImpl( + void* externalImage, VkDevice device) { + return {}; +} + +VulkanPlatform::ImageData VulkanPlatform::createExternalImageImpl(void* externalImage, + VkDevice device, const VkAllocationCallbacks* allocator, + const ExternalImageMetadata& metadata) { + return {}; +} + VulkanPlatform::ExtensionSet VulkanPlatform::getSwapchainInstanceExtensions() { VulkanPlatform::ExtensionSet const ret = { -#if defined(__ANDROID__) - VK_KHR_ANDROID_SURFACE_EXTENSION_NAME, -#elif defined(__linux__) && defined(FILAMENT_SUPPORTS_WAYLAND) +#if defined(__linux__) && defined(FILAMENT_SUPPORTS_WAYLAND) VK_KHR_WAYLAND_SURFACE_EXTENSION_NAME, #elif defined(LINUX_OR_FREEBSD) && defined(FILAMENT_SUPPORTS_X11) #if defined(FILAMENT_SUPPORTS_XCB) @@ -116,15 +123,7 @@ VulkanPlatform::SurfaceBundle VulkanPlatform::createVkSurfaceKHR(void* nativeWin // swap chain extent. VkExtent2D extent; - #if defined(__ANDROID__) - VkAndroidSurfaceCreateInfoKHR const createInfo{ - .sType = VK_STRUCTURE_TYPE_ANDROID_SURFACE_CREATE_INFO_KHR, - .window = (ANativeWindow*) nativeWindow, - }; - VkResult const result = vkCreateAndroidSurfaceKHR(instance, &createInfo, VKALLOC, - (VkSurfaceKHR*) &surface); - FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) << "vkCreateAndroidSurfaceKHR error."; - #elif defined(__linux__) && defined(FILAMENT_SUPPORTS_WAYLAND) + #if defined(__linux__) && defined(FILAMENT_SUPPORTS_WAYLAND) wl* ptrval = reinterpret_cast(nativeWindow); extent.width = ptrval->width; extent.height = ptrval->height; From ba6bee10f76759493d8b1a0036513088daf6da0a Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 10 Dec 2024 14:21:26 -0800 Subject: [PATCH 09/12] matc: remove access to public functions that should be undefined When tangents are not supplied in a material, all "normals" related public methods become undefined; remove access to them so we get compile time errors instead of garbage values in the shader. --- libs/filamat/src/shaders/ShaderGenerator.cpp | 12 +++++++----- shaders/src/common_shading.fs | 2 ++ shaders/src/getters.fs | 4 ++++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/libs/filamat/src/shaders/ShaderGenerator.cpp b/libs/filamat/src/shaders/ShaderGenerator.cpp index 45b78cc04394..63273b952b6f 100644 --- a/libs/filamat/src/shaders/ShaderGenerator.cpp +++ b/libs/filamat/src/shaders/ShaderGenerator.cpp @@ -660,11 +660,13 @@ std::string ShaderGenerator::createFragmentProgram(ShaderModel shaderModel, CodeGenerator::generateDepthShaderMain(fs, ShaderStage::FRAGMENT); } else { appendShader(fs, mMaterialFragmentCode, mMaterialLineOffset); - if (filament::Variant::isSSRVariant(variant)) { - CodeGenerator::generateShaderReflections(fs, ShaderStage::FRAGMENT); - } else if (material.isLit) { - CodeGenerator::generateShaderLit(fs, ShaderStage::FRAGMENT, variant, - material.shading,material.hasCustomSurfaceShading); + if (material.isLit) { + if (filament::Variant::isSSRVariant(variant)) { + CodeGenerator::generateShaderReflections(fs, ShaderStage::FRAGMENT); + } else { + CodeGenerator::generateShaderLit(fs, ShaderStage::FRAGMENT, variant, + material.shading,material.hasCustomSurfaceShading); + } } else { CodeGenerator::generateShaderUnlit(fs, ShaderStage::FRAGMENT, variant, material.hasShadowMultiplier); diff --git a/shaders/src/common_shading.fs b/shaders/src/common_shading.fs index ae1c4ab6f946..c95a99e7b691 100644 --- a/shaders/src/common_shading.fs +++ b/shaders/src/common_shading.fs @@ -3,6 +3,7 @@ highp mat3 shading_tangentToWorld; // TBN matrix highp vec3 shading_position; // position of the fragment in world space vec3 shading_view; // normalized vector from the fragment to the eye +#if defined(HAS_ATTRIBUTE_TANGENTS) vec3 shading_normal; // normalized transformed normal, in world space vec3 shading_geometricNormal; // normalized geometric normal, in world space vec3 shading_reflected; // reflection of view about normal @@ -15,5 +16,6 @@ highp vec3 shading_position; // position of the fragment in world space #if defined(MATERIAL_HAS_CLEAR_COAT) vec3 shading_clearCoatNormal; // normalized clear coat layer normal, in world space #endif +#endif highp vec2 shading_normalizedViewportCoord; diff --git a/shaders/src/getters.fs b/shaders/src/getters.fs index 2f2cc4231aa7..a6254764eb9a 100644 --- a/shaders/src/getters.fs +++ b/shaders/src/getters.fs @@ -63,6 +63,8 @@ bool isPerspectiveProjection() { return frameUniforms.clipFromViewMatrix[2].w != 0.0; } +#if defined(HAS_ATTRIBUTE_TANGENTS) + /** @public-api */ vec3 getWorldNormalVector() { return shading_normal; @@ -83,6 +85,8 @@ float getNdotV() { return shading_NoV; } +#endif + highp vec3 getNormalizedPhysicalViewportCoord() { // make sure to handle our reversed-z return vec3(shading_normalizedViewportCoord, gl_FragCoord.z); From 96278f79998d08ce603ab4c1d3c5be13a8008d80 Mon Sep 17 00:00:00 2001 From: Thoren Paulson <3453006+thoren-d@users.noreply.github.com> Date: Mon, 9 Dec 2024 09:26:16 -0800 Subject: [PATCH 10/12] Fix shadow cascade calculatioins The `ShadowCascade` functions for computing splits all attempted to clamp `cascades` to [0,4] (evidently to avoid out-of-bounds access), but used `max`, instead giving a range of [4, x]. For applications using less than 4 cascades, this results in incorrect split locations and lower quality (much more obvious seam between cascades). This change just switches to use `min` to ensure `cascades` is in the range [0,4]. --- filament/src/components/LightManager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/filament/src/components/LightManager.cpp b/filament/src/components/LightManager.cpp index ad642f5942e3..1b173ef1ffa5 100644 --- a/filament/src/components/LightManager.cpp +++ b/filament/src/components/LightManager.cpp @@ -427,7 +427,7 @@ float FLightManager::getSpotLightInnerCone(Instance i) const noexcept { void LightManager::ShadowCascades::computeUniformSplits(float splitPositions[3], uint8_t cascades) { size_t s = 0; - cascades = max(cascades, (uint8_t) 4u); + cascades = min(cascades, (uint8_t) 4u); for (size_t c = 1; c < cascades; c++) { splitPositions[s++] = float(c) / float(cascades); } @@ -436,7 +436,7 @@ void LightManager::ShadowCascades::computeUniformSplits(float splitPositions[3], void LightManager::ShadowCascades::computeLogSplits(float splitPositions[3], uint8_t cascades, float near, float far) { size_t s = 0; - cascades = max(cascades, (uint8_t) 4u); + cascades = min(cascades, (uint8_t) 4u); for (size_t c = 1; c < cascades; c++) { splitPositions[s++] = (near * std::pow(far / near, float(c) / float(cascades)) - near) / (far - near); @@ -447,7 +447,7 @@ void LightManager::ShadowCascades::computePracticalSplits(float splitPositions[3 float near, float far, float lambda) { float uniformSplits[3]; float logSplits[3]; - cascades = max(cascades, (uint8_t) 4u); + cascades = min(cascades, (uint8_t) 4u); computeUniformSplits(uniformSplits, cascades); computeLogSplits(logSplits, cascades, near, far); size_t s = 0; From 23847c4314a5ebcead2b24f18e6945c93247301f Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 11 Dec 2024 16:53:00 -0800 Subject: [PATCH 11/12] vk: Use-after-free coverage (#8273) To allow for use-after-free discovery with ref-counting, we keep a bit to indicate whether the handle is considered destroyed by Filament. We assert against "cast"-ing a destroyed handle. --- filament/backend/src/vulkan/memory/Resource.h | 33 ++++++++++++++++--- .../src/vulkan/memory/ResourcePointer.h | 7 ++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/filament/backend/src/vulkan/memory/Resource.h b/filament/backend/src/vulkan/memory/Resource.h index 7db90e4af0f3..c856e62879ef 100644 --- a/filament/backend/src/vulkan/memory/Resource.h +++ b/filament/backend/src/vulkan/memory/Resource.h @@ -66,7 +66,8 @@ struct Resource { : resManager(nullptr), id(HandleBase::nullid), mCount(0), - restype(ResourceType::UNDEFINED_TYPE) {} + restype(ResourceType::UNDEFINED_TYPE), + mHandleConsideredDestroyed(false) {} private: inline void inc() noexcept { @@ -80,6 +81,16 @@ struct Resource { } } + // To be able to detect use-after-free, we need a bit to signify if the handle should be + // consider destroyed (from Filament's perspective). + inline void setHandleConsiderDestroyed() noexcept { + mHandleConsideredDestroyed = true; + } + + inline bool isHandleConsideredDestroyed() const { + return mHandleConsideredDestroyed; + } + template inline void init(HandleId id, ResourceManager* resManager) { this->id = id; @@ -92,7 +103,9 @@ struct Resource { ResourceManager* resManager; // 8 HandleId id; // 4 uint32_t mCount : 24; - ResourceType restype : 6; // restype + mCount is 4 bytes. + ResourceType restype : 7; + bool mHandleConsideredDestroyed : 1; // restype + mCount + mHandleConsideredDestroyed + // is 4 bytes. friend class ResourceManager; @@ -105,7 +118,8 @@ struct ThreadSafeResource { : resManager(nullptr), id(HandleBase::nullid), mCount(0), - restype(ResourceType::UNDEFINED_TYPE) {} + restype(ResourceType::UNDEFINED_TYPE), + mHandleConsideredDestroyed(false) {} private: inline void inc() noexcept { @@ -118,6 +132,16 @@ struct ThreadSafeResource { } } + // To be able to detect use-after-free, we need a bit to signify if the handle should be + // consider destroyed (from Filament's perspective). + inline void setHandleConsiderDestroyed() noexcept { + mHandleConsideredDestroyed = true; + } + + inline bool isHandleConsideredDestroyed() const { + return mHandleConsideredDestroyed; + } + template inline void init(HandleId id, ResourceManager* resManager) { this->id = id; @@ -130,7 +154,8 @@ struct ThreadSafeResource { ResourceManager* resManager; // 8 HandleId id; // 4 std::atomic mCount; // 4 - ResourceType restype; // 1 + ResourceType restype : 7; + bool mHandleConsideredDestroyed : 1; // restype + mHandleConsideredDestroyed is 1 byte friend class ResourceManager; diff --git a/filament/backend/src/vulkan/memory/ResourcePointer.h b/filament/backend/src/vulkan/memory/ResourcePointer.h index 6f34b9252ab1..9affcdae56aa 100644 --- a/filament/backend/src/vulkan/memory/ResourcePointer.h +++ b/filament/backend/src/vulkan/memory/ResourcePointer.h @@ -21,6 +21,7 @@ #include "vulkan/memory/ResourceManager.h" #include +#include #include @@ -59,6 +60,10 @@ struct resource_ptr { static enabled_resource_ptr cast(ResourceManager* resManager, Handle const& handle) noexcept { D* ptr = resManager->handle_cast(handle); + FILAMENT_CHECK_PRECONDITION(!ptr->isHandleConsideredDestroyed()) + << "Handle id=" << ptr->id << " (" << getTypeStr(ptr->restype) + << ") is being used after it has been freed"; + return {ptr}; } @@ -156,6 +161,8 @@ struct resource_ptr { // only be used from VulkanDriver. inline void dec() { assert_invariant(mRef); + assert_invariant(!mRef->isHandleConsiderDestroyed()); + mRef->setHandleConsiderDestroyed(); mRef->dec(); } From 6a772bf7152972693efbd1176eb9c624c55b57b4 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 11 Dec 2024 19:39:04 -0800 Subject: [PATCH 12/12] vk: fix typo that caused debug builds to fail (#8309) --- filament/backend/src/vulkan/memory/ResourcePointer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/src/vulkan/memory/ResourcePointer.h b/filament/backend/src/vulkan/memory/ResourcePointer.h index 9affcdae56aa..25b9737bf710 100644 --- a/filament/backend/src/vulkan/memory/ResourcePointer.h +++ b/filament/backend/src/vulkan/memory/ResourcePointer.h @@ -161,7 +161,7 @@ struct resource_ptr { // only be used from VulkanDriver. inline void dec() { assert_invariant(mRef); - assert_invariant(!mRef->isHandleConsiderDestroyed()); + assert_invariant(!mRef->isHandleConsideredDestroyed()); mRef->setHandleConsiderDestroyed(); mRef->dec(); }