From 283d240409825f5d6180714bde532f008fdfd6b6 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 27 Aug 2024 23:17:26 -0700 Subject: [PATCH 01/18] Re-enable DebugRegistry::setProperty for release build (#8086) `DebugRegistry::setProperty` is no longer just applicable to debug builds. This change was previously added in 6c0bd36 but then reverted in a7317e7. We re-enable it and separate it from the shadow changes in this commit. --- filament/src/details/DebugRegistry.cpp | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/filament/src/details/DebugRegistry.cpp b/filament/src/details/DebugRegistry.cpp index c2f5d3d3fb6..8915cf312e1 100644 --- a/filament/src/details/DebugRegistry.cpp +++ b/filament/src/details/DebugRegistry.cpp @@ -28,12 +28,6 @@ #include #include -#ifndef NDEBUG -# define DEBUG_PROPERTIES_WRITABLE true -#else -# define DEBUG_PROPERTIES_WRITABLE false -#endif - using namespace filament::math; using namespace utils; @@ -79,17 +73,15 @@ bool FDebugRegistry::hasProperty(const char* name) const noexcept { template bool FDebugRegistry::setProperty(const char* name, T v) noexcept { - if constexpr (DEBUG_PROPERTIES_WRITABLE) { - auto info = getPropertyInfo(name); - T* const addr = static_cast(info.first); - if (addr) { - auto old = *addr; - *addr = v; - if (info.second && old != v) { - info.second(); - } - return true; + auto info = getPropertyInfo(name); + T* const addr = static_cast(info.first); + if (addr) { + auto old = *addr; + *addr = v; + if (info.second && old != v) { + info.second(); } + return true; } return false; } From 777f664b1bb6aa2836c1db3e36585e54b4eed3f9 Mon Sep 17 00:00:00 2001 From: Maximilien Dagois Date: Thu, 22 Aug 2024 14:18:40 +0900 Subject: [PATCH 02/18] Refactored the handling of usage flags in the VulkanTexture constructor --- filament/backend/src/vulkan/VulkanHandles.cpp | 1 - filament/backend/src/vulkan/VulkanTexture.cpp | 17 +++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index 4222f361a6f..ae445de79a2 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -16,7 +16,6 @@ #include "VulkanHandles.h" -#include "VulkanDriver.h" #include "VulkanConstants.h" #include "VulkanDriver.h" #include "VulkanMemory.h" diff --git a/filament/backend/src/vulkan/VulkanTexture.cpp b/filament/backend/src/vulkan/VulkanTexture.cpp index 9ca88a2c78c..b154eec6532 100644 --- a/filament/backend/src/vulkan/VulkanTexture.cpp +++ b/filament/backend/src/vulkan/VulkanTexture.cpp @@ -97,13 +97,11 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, imageInfo.extent.depth = 1; } - // Filament expects blit() to work with any texture, so we almost always set these usage flags. - // TODO: investigate performance implications of setting these flags. - constexpr VkImageUsageFlags blittable = VK_IMAGE_USAGE_TRANSFER_DST_BIT | - VK_IMAGE_USAGE_TRANSFER_SRC_BIT; - - if (any(usage & (TextureUsage::BLIT_DST | TextureUsage::BLIT_SRC))) { - imageInfo.usage |= blittable; + if (any(usage & TextureUsage::BLIT_SRC)) { + imageInfo.usage |= VK_IMAGE_USAGE_TRANSFER_SRC_BIT; + } + if (any(usage & TextureUsage::BLIT_DST)) { + imageInfo.usage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT; } if (any(usage & TextureUsage::SAMPLEABLE)) { @@ -121,7 +119,7 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, imageInfo.usage |= VK_IMAGE_USAGE_SAMPLED_BIT; } if (any(usage & TextureUsage::COLOR_ATTACHMENT)) { - imageInfo.usage |= VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | blittable; + imageInfo.usage |= VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; if (any(usage & TextureUsage::SUBPASS_INPUT)) { imageInfo.usage |= VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT; } @@ -130,10 +128,9 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, imageInfo.usage |= VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT; } if (any(usage & TextureUsage::UPLOADABLE)) { - imageInfo.usage |= blittable; + imageInfo.usage |= VK_IMAGE_USAGE_TRANSFER_DST_BIT; } if (any(usage & TextureUsage::DEPTH_ATTACHMENT)) { - imageInfo.usage |= blittable; imageInfo.usage |= VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT; // Depth resolves uses a custom shader and therefore needs to be sampleable. From 6653c6c08b68de5737933ba9e01464900d0882ca Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 28 Aug 2024 14:28:01 -0700 Subject: [PATCH 03/18] fix a typo in RenderPassBuilder::renderFlags() --- filament/src/RenderPass.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/filament/src/RenderPass.h b/filament/src/RenderPass.h index e729d67dd7f..f88a5a84411 100644 --- a/filament/src/RenderPass.h +++ b/filament/src/RenderPass.h @@ -528,7 +528,9 @@ class RenderPassBuilder { // like above but allows to set specific flags RenderPassBuilder& renderFlags( RenderPass::RenderFlags mask, RenderPass::RenderFlags value) noexcept { - mFlags = (mFlags & mask) | (value & mask); + value &= mask; + mFlags &= ~mask; + mFlags |= value; return *this; } From 639b933fd65a25a7aeecaf630e11369726f8064e Mon Sep 17 00:00:00 2001 From: Sungun Park Date: Thu, 29 Aug 2024 18:47:24 +0000 Subject: [PATCH 04/18] Rename customRenderTarget (#8093) The currentRenderTarget is a misnomer as it's a custom RT. Rename it properly. --- filament/src/details/Renderer.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index d91a9c42d36..3cac2bafefb 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -818,14 +818,14 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { blackboard["shadows"] = shadows; } - // When we don't have a custom RenderTarget, currentRenderTarget below is nullptr and is + // When we don't have a custom RenderTarget, customRenderTarget below is nullptr and is // recorded in the list of targets already rendered into -- this ensures that // initializeClearFlags() is called only once for the default RenderTarget. auto& previousRenderTargets = mPreviousRenderTargets; - FRenderTarget* const currentRenderTarget = downcast(view.getRenderTarget()); + FRenderTarget* const customRenderTarget = downcast(view.getRenderTarget()); if (UTILS_LIKELY( - previousRenderTargets.find(currentRenderTarget) == previousRenderTargets.end())) { - previousRenderTargets.insert(currentRenderTarget); + previousRenderTargets.find(customRenderTarget) == previousRenderTargets.end())) { + previousRenderTargets.insert(customRenderTarget); initializeClearFlags(); } @@ -842,10 +842,10 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { const TargetBufferFlags keepOverrideStartFlags = TargetBufferFlags::ALL & ~discardStartFlags; TargetBufferFlags keepOverrideEndFlags = TargetBufferFlags::NONE; - if (currentRenderTarget) { + if (customRenderTarget) { // For custom RenderTarget, we look at each attachment flag and if they have their // SAMPLEABLE usage bit set, we assume they must not be discarded after the render pass. - keepOverrideEndFlags |= currentRenderTarget->getSampleableAttachmentsMask(); + keepOverrideEndFlags |= customRenderTarget->getSampleableAttachmentsMask(); } // Renderer's ClearOptions apply once at the beginning of the frame (not for each View), From 2202b5ab8c91d5251a50d50d780e3b10bceafee1 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 28 Aug 2024 13:21:35 -0700 Subject: [PATCH 05/18] Add support for depth clamp and use it for shadows vk, metal and desktop gl all support depth clamp, GLES/android also does with ANGLE. Add support for it in the backends. use depth clamp to improve directional shadow quality; this allows to render everything that's behind the camera at the same "zero" depth, so we can reduce the depth range we need. Fixes #6293 --- .../backend/include/backend/DriverEnums.h | 2 +- .../include/private/backend/DriverAPI.inc | 1 + filament/backend/src/metal/MetalContext.h | 1 + filament/backend/src/metal/MetalDriver.mm | 11 +++++++ filament/backend/src/metal/MetalState.h | 1 + filament/backend/src/noop/NoopDriver.cpp | 4 +++ filament/backend/src/opengl/OpenGLContext.cpp | 2 ++ filament/backend/src/opengl/OpenGLContext.h | 8 +++-- filament/backend/src/opengl/OpenGLDriver.cpp | 12 ++++++++ filament/backend/src/opengl/gl_headers.h | 6 ++++ filament/backend/src/vulkan/VulkanContext.h | 4 +++ filament/backend/src/vulkan/VulkanDriver.cpp | 5 ++++ .../src/vulkan/VulkanPipelineCache.cpp | 1 + .../backend/src/vulkan/VulkanPipelineCache.h | 3 +- .../src/vulkan/platform/VulkanPlatform.cpp | 1 + filament/src/RenderPass.cpp | 11 +++++-- filament/src/RenderPass.h | 3 +- filament/src/ShadowMap.cpp | 12 ++++---- filament/src/ShadowMapManager.cpp | 29 +++++++++++++++---- filament/src/ShadowMapManager.h | 1 + filament/src/details/Engine.h | 1 + samples/gltf_viewer.cpp | 2 ++ 22 files changed, 102 insertions(+), 19 deletions(-) diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index 8da0cfb02fc..35cd89a57e7 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -1058,7 +1058,7 @@ struct RasterState { bool inverseFrontFaces : 1; // 31 //! padding, must be 0 - uint8_t padding : 1; // 32 + bool depthClamp : 1; // 32 }; uint32_t u = 0; }; diff --git a/filament/backend/include/private/backend/DriverAPI.inc b/filament/backend/include/private/backend/DriverAPI.inc index 99ea2d528eb..283e3e0e5ca 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -309,6 +309,7 @@ DECL_DRIVER_API_SYNCHRONOUS_0(bool, isParallelShaderCompileSupported) DECL_DRIVER_API_SYNCHRONOUS_0(bool, isDepthStencilResolveSupported) DECL_DRIVER_API_SYNCHRONOUS_N(bool, isDepthStencilBlitSupported, backend::TextureFormat, format) DECL_DRIVER_API_SYNCHRONOUS_0(bool, isProtectedTexturesSupported) +DECL_DRIVER_API_SYNCHRONOUS_0(bool, isDepthClampSupported) DECL_DRIVER_API_SYNCHRONOUS_0(uint8_t, getMaxDrawBuffers) DECL_DRIVER_API_SYNCHRONOUS_0(size_t, getMaxUniformBufferSize) DECL_DRIVER_API_SYNCHRONOUS_0(math::float2, getClipSpaceParams) diff --git a/filament/backend/src/metal/MetalContext.h b/filament/backend/src/metal/MetalContext.h index f771ca5b417..fc8cfc12b46 100644 --- a/filament/backend/src/metal/MetalContext.h +++ b/filament/backend/src/metal/MetalContext.h @@ -112,6 +112,7 @@ struct MetalContext { std::array ssboState; CullModeStateTracker cullModeState; WindingStateTracker windingState; + DepthClampStateTracker depthClampState; Handle currentRenderPrimitive; // State caches. diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index b85d616c773..5c12736b914 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -857,6 +857,10 @@ return false; } +bool MetalDriver::isDepthClampSupported() { + return true; +} + bool MetalDriver::isWorkaroundNeeded(Workaround workaround) { switch (workaround) { case Workaround::SPLIT_EASU: @@ -1774,6 +1778,13 @@ [mContext->currentRenderPassEncoder setFrontFacingWinding:winding]; } + // depth clip mode + 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. DepthStencilState depthState; if (depthAttachment) { diff --git a/filament/backend/src/metal/MetalState.h b/filament/backend/src/metal/MetalState.h index 83579cb5d50..1d541cc0065 100644 --- a/filament/backend/src/metal/MetalState.h +++ b/filament/backend/src/metal/MetalState.h @@ -382,6 +382,7 @@ using SamplerStateCache = StateCache, SamplerS using CullModeStateTracker = StateTracker; using WindingStateTracker = StateTracker; +using DepthClampStateTracker = StateTracker; // Argument encoder diff --git a/filament/backend/src/noop/NoopDriver.cpp b/filament/backend/src/noop/NoopDriver.cpp index adc9c1bc55f..096f57c3989 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -202,6 +202,10 @@ bool NoopDriver::isProtectedTexturesSupported() { return true; } +bool NoopDriver::isDepthClampSupported() { + return false; +} + bool NoopDriver::isWorkaroundNeeded(Workaround) { return false; } diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp index 66327633f17..a7933407bb4 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -676,6 +676,7 @@ void OpenGLContext::initExtensionsGLES(Extensions* ext, GLint major, GLint minor #ifndef __EMSCRIPTEN__ ext->EXT_debug_marker = exts.has("GL_EXT_debug_marker"sv); #endif + ext->EXT_depth_clamp = exts.has("GL_EXT_depth_clamp"sv); ext->EXT_discard_framebuffer = exts.has("GL_EXT_discard_framebuffer"sv); #ifndef __EMSCRIPTEN__ ext->EXT_disjoint_timer_query = exts.has("GL_EXT_disjoint_timer_query"sv); @@ -746,6 +747,7 @@ void OpenGLContext::initExtensionsGL(Extensions* ext, GLint major, GLint minor) ext->EXT_color_buffer_half_float = true; // Assumes core profile. ext->EXT_clip_cull_distance = true; ext->EXT_debug_marker = exts.has("GL_EXT_debug_marker"sv); + ext->EXT_depth_clamp = true; ext->EXT_discard_framebuffer = false; ext->EXT_disjoint_timer_query = true; ext->EXT_multisampled_render_to_texture = false; diff --git a/filament/backend/src/opengl/OpenGLContext.h b/filament/backend/src/opengl/OpenGLContext.h index 902fcc9f094..2972480e71f 100644 --- a/filament/backend/src/opengl/OpenGLContext.h +++ b/filament/backend/src/opengl/OpenGLContext.h @@ -220,8 +220,9 @@ class OpenGLContext final : public TimerQueryFactoryInterface { bool EXT_color_buffer_float; bool EXT_color_buffer_half_float; bool EXT_debug_marker; - bool EXT_disjoint_timer_query; + bool EXT_depth_clamp; bool EXT_discard_framebuffer; + bool EXT_disjoint_timer_query; bool EXT_multisampled_render_to_texture2; bool EXT_multisampled_render_to_texture; bool EXT_protected_textures; @@ -239,10 +240,10 @@ class OpenGLContext final : public TimerQueryFactoryInterface { bool KHR_parallel_shader_compile; bool KHR_texture_compression_astc_hdr; bool KHR_texture_compression_astc_ldr; - bool OES_depth_texture; + bool OES_EGL_image_external_essl3; bool OES_depth24; + bool OES_depth_texture; bool OES_packed_depth_stencil; - bool OES_EGL_image_external_essl3; bool OES_rgb8_rgba8; bool OES_standard_derivatives; bool OES_texture_npot; @@ -627,6 +628,7 @@ constexpr size_t OpenGLContext::getIndexForCap(GLenum cap) noexcept { //NOLINT #ifdef BACKEND_OPENGL_VERSION_GL case GL_PROGRAM_POINT_SIZE: index = 10; break; #endif + case GL_DEPTH_CLAMP: index = 11; break; default: break; } assert_invariant(index < state.enables.caps.size()); diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index bb9d2a63a57..ed23067bb87 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -451,6 +451,14 @@ void OpenGLDriver::setRasterState(RasterState rs) noexcept { } else { gl.disable(GL_SAMPLE_ALPHA_TO_COVERAGE); } + + if (gl.ext.EXT_depth_clamp) { + if (rs.depthClamp) { + gl.enable(GL_DEPTH_CLAMP); + } else { + gl.disable(GL_DEPTH_CLAMP); + } + } } void OpenGLDriver::setStencilState(StencilState ss) noexcept { @@ -2119,6 +2127,10 @@ bool OpenGLDriver::isProtectedTexturesSupported() { return getContext().ext.EXT_protected_textures; } +bool OpenGLDriver::isDepthClampSupported() { + return getContext().ext.EXT_depth_clamp; +} + bool OpenGLDriver::isWorkaroundNeeded(Workaround workaround) { switch (workaround) { case Workaround::SPLIT_EASU: diff --git a/filament/backend/src/opengl/gl_headers.h b/filament/backend/src/opengl/gl_headers.h index 569c8ed2859..04ad63ff6ac 100644 --- a/filament/backend/src/opengl/gl_headers.h +++ b/filament/backend/src/opengl/gl_headers.h @@ -201,6 +201,12 @@ using namespace glext; # define GL_CLIP_DISTANCE1 0x3001 #endif +#if defined(GL_EXT_depth_clamp) +# define GL_DEPTH_CLAMP GL_DEPTH_CLAMP_EXT +#else +# define GL_DEPTH_CLAMP 0x864F +#endif + #if defined(GL_KHR_debug) # define GL_DEBUG_OUTPUT GL_DEBUG_OUTPUT_KHR # define GL_DEBUG_OUTPUT_SYNCHRONOUS GL_DEBUG_OUTPUT_SYNCHRONOUS_KHR diff --git a/filament/backend/src/vulkan/VulkanContext.h b/filament/backend/src/vulkan/VulkanContext.h index 9f506e03f76..46fe475612a 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/filament/backend/src/vulkan/VulkanContext.h @@ -125,6 +125,10 @@ struct VulkanContext { return mPhysicalDeviceFeatures.imageCubeArray == VK_TRUE; } + inline bool isDepthClampSupported() const noexcept { + return mPhysicalDeviceFeatures.depthClamp == VK_TRUE; + } + inline bool isDebugMarkersSupported() const noexcept { return mDebugMarkersSupported; } diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 0855102dd91..41924b96e03 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -944,6 +944,10 @@ bool VulkanDriver::isProtectedTexturesSupported() { return false; } +bool VulkanDriver::isDepthClampSupported() { + return mContext.isDepthClampSupported(); +} + bool VulkanDriver::isWorkaroundNeeded(Workaround workaround) { switch (workaround) { case Workaround::SPLIT_EASU: { @@ -1816,6 +1820,7 @@ void VulkanDriver::bindPipeline(PipelineState const& pipelineState) { .dstAlphaBlendFactor = getBlendFactor(rasterState.blendFunctionDstAlpha), .colorWriteMask = (VkColorComponentFlags) (rasterState.colorWrite ? 0xf : 0x0), .rasterizationSamples = rt->getSamples(), + .depthClamp = rasterState.depthClamp, .colorTargetCount = rt->getColorTargetCount(mCurrentRenderPass), .colorBlendOp = rasterState.blendEquationRGB, .alphaBlendOp = rasterState.blendEquationAlpha, diff --git a/filament/backend/src/vulkan/VulkanPipelineCache.cpp b/filament/backend/src/vulkan/VulkanPipelineCache.cpp index 4c77525bd1b..b3d09da86ed 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.cpp +++ b/filament/backend/src/vulkan/VulkanPipelineCache.cpp @@ -184,6 +184,7 @@ VulkanPipelineCache::PipelineCacheEntry* VulkanPipelineCache::createPipeline() n vkRaster.polygonMode = VK_POLYGON_MODE_FILL; vkRaster.cullMode = raster.cullMode; vkRaster.frontFace = raster.frontFace; + vkRaster.depthClampEnable = raster.depthClamp; vkRaster.depthBiasEnable = raster.depthBiasEnable; vkRaster.depthBiasConstantFactor = raster.depthBiasConstantFactor; vkRaster.depthBiasClamp = 0.0f; diff --git a/filament/backend/src/vulkan/VulkanPipelineCache.h b/filament/backend/src/vulkan/VulkanPipelineCache.h index e6816497c05..702d27f245f 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.h +++ b/filament/backend/src/vulkan/VulkanPipelineCache.h @@ -90,7 +90,8 @@ class VulkanPipelineCache { VkBlendFactor srcAlphaBlendFactor : 5; VkBlendFactor dstAlphaBlendFactor : 5; VkColorComponentFlags colorWriteMask : 4; - uint8_t rasterizationSamples; // offset = 4 bytes + uint8_t rasterizationSamples : 4;// offset = 4 bytes + uint8_t depthClamp : 4; uint8_t colorTargetCount; // offset = 5 bytes BlendEquation colorBlendOp : 4; // offset = 6 bytes BlendEquation alphaBlendOp : 4; diff --git a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp index b4152807ccd..2e88542080f 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp @@ -325,6 +325,7 @@ VkDevice createLogicalDevice(VkPhysicalDevice physicalDevice, // We could simply enable all supported features, but since that may have performance // consequences let's just enable the features we need. VkPhysicalDeviceFeatures enabledFeatures{ + .depthClamp = features.depthClamp, .samplerAnisotropy = features.samplerAnisotropy, .textureCompressionETC2 = features.textureCompressionETC2, .textureCompressionBC = features.textureCompressionBC, diff --git a/filament/src/RenderPass.cpp b/filament/src/RenderPass.cpp index b931419775c..55ec3264b2d 100644 --- a/filament/src/RenderPass.cpp +++ b/filament/src/RenderPass.cpp @@ -419,7 +419,8 @@ RenderPass::Command* RenderPass::instanceify(FEngine& engine, UTILS_ALWAYS_INLINE // This function exists only to make the code more readable. we want it inlined. inline // and we don't need it in the compilation unit void RenderPass::setupColorCommand(Command& cmdDraw, Variant variant, - FMaterialInstance const* const UTILS_RESTRICT mi, bool inverseFrontFaces) noexcept { + FMaterialInstance const* const UTILS_RESTRICT mi, + bool inverseFrontFaces, bool hasDepthClamp) noexcept { FMaterial const * const UTILS_RESTRICT ma = mi->getMaterial(); variant = Variant::filterVariant(variant, ma->isVariantLit()); @@ -460,6 +461,7 @@ void RenderPass::setupColorCommand(Command& cmdDraw, Variant variant, cmdDraw.info.rasterState.colorWrite = mi->isColorWriteEnabled(); cmdDraw.info.rasterState.depthWrite = mi->isDepthWriteEnabled(); cmdDraw.info.rasterState.depthFunc = mi->getDepthFunc(); + cmdDraw.info.rasterState.depthClamp = hasDepthClamp; cmdDraw.info.materialVariant = variant; // we keep "RasterState::colorWrite" to the value set by material (could be disabled) } @@ -558,6 +560,9 @@ RenderPass::Command* RenderPass::generateCommandsImpl(RenderPass::CommandTypeFla bool const hasInstancedStereo = renderFlags & IS_INSTANCED_STEREOSCOPIC; + bool const hasDepthClamp = + renderFlags & HAS_DEPTH_CLAMP; + float const cameraPositionDotCameraForward = dot(cameraPosition, cameraForward); auto const* const UTILS_RESTRICT soaWorldAABBCenter = soa.data(); @@ -577,6 +582,7 @@ RenderPass::Command* RenderPass::generateCommandsImpl(RenderPass::CommandTypeFla cmd.info.rasterState.depthWrite = true; cmd.info.rasterState.depthFunc = RasterState::DepthFunc::GE; cmd.info.rasterState.alphaToCoverage = false; + cmd.info.rasterState.depthClamp = hasDepthClamp; } for (uint32_t i = range.first; i < range.last; ++i) { @@ -691,7 +697,8 @@ RenderPass::Command* RenderPass::generateCommandsImpl(RenderPass::CommandTypeFla cmd.info.morphingOffset = primitive.getMorphingBufferOffset(); if constexpr (isColorPass) { - RenderPass::setupColorCommand(cmd, renderableVariant, mi, inverseFrontFaces); + RenderPass::setupColorCommand(cmd, renderableVariant, mi, + inverseFrontFaces, hasDepthClamp); const bool blendPass = Pass(cmd.key & PASS_MASK) == Pass::BLENDED; if (blendPass) { // TODO: at least for transparent objects, AABB should be per primitive diff --git a/filament/src/RenderPass.h b/filament/src/RenderPass.h index f88a5a84411..485fd905894 100644 --- a/filament/src/RenderPass.h +++ b/filament/src/RenderPass.h @@ -284,6 +284,7 @@ class RenderPass { static constexpr RenderFlags HAS_SHADOWING = 0x01; static constexpr RenderFlags HAS_INVERSE_FRONT_FACES = 0x02; static constexpr RenderFlags IS_INSTANCED_STEREOSCOPIC = 0x04; + static constexpr RenderFlags HAS_DEPTH_CLAMP = 0x08; // Arena used for commands using Arena = utils::Arena< @@ -444,7 +445,7 @@ class RenderPass { uint8_t instancedStereoEyeCount) noexcept; static void setupColorCommand(Command& cmdDraw, Variant variant, - FMaterialInstance const* mi, bool inverseFrontFaces) noexcept; + FMaterialInstance const* mi, bool inverseFrontFaces, bool hasDepthClamp) noexcept; static void updateSummedPrimitiveCounts( FScene::RenderableSoa& renderableData, utils::Range vr) noexcept; diff --git a/filament/src/ShadowMap.cpp b/filament/src/ShadowMap.cpp index 4c406680542..bbf008e4a04 100644 --- a/filament/src/ShadowMap.cpp +++ b/filament/src/ShadowMap.cpp @@ -829,13 +829,13 @@ ShadowMap::Corners ShadowMap::computeFrustumCorners( Corners const csViewFrustumCorners = { .vertices = { { -1, -1, far }, - { 1, -1, far }, - { -1, 1, far }, - { 1, 1, far }, + { 1, -1, far }, + { -1, 1, far }, + { 1, 1, far }, { -1, -1, near }, - { 1, -1, near }, - { -1, 1, near }, - { 1, 1, near }, + { 1, -1, near }, + { -1, 1, near }, + { 1, 1, near }, } }; diff --git a/filament/src/ShadowMapManager.cpp b/filament/src/ShadowMapManager.cpp index a6f4a9fa3c0..6997f56a791 100644 --- a/filament/src/ShadowMapManager.cpp +++ b/filament/src/ShadowMapManager.cpp @@ -66,15 +66,15 @@ namespace filament { using namespace backend; using namespace math; -// do this only if depth-clamp is available -static constexpr bool USE_DEPTH_CLAMP = false; - -ShadowMapManager::ShadowMapManager(FEngine& engine) { +ShadowMapManager::ShadowMapManager(FEngine& engine) + : mIsDepthClampSupported(engine.getDriverApi().isDepthClampSupported()) { FDebugRegistry& debugRegistry = engine.getDebugRegistry(); debugRegistry.registerProperty("d.shadowmap.visualize_cascades", &engine.debug.shadowmap.visualize_cascades); debugRegistry.registerProperty("d.shadowmap.disable_light_frustum_align", &engine.debug.shadowmap.disable_light_frustum_align); + debugRegistry.registerProperty("d.shadowmap.depth_clamp", + &engine.debug.shadowmap.depth_clamp); } ShadowMapManager::~ShadowMapManager() { @@ -367,7 +367,20 @@ FrameGraphId ShadowMapManager::render(FEngine& engine, FrameG // generate and sort the commands for rendering the shadow map + RenderPass::RenderFlags renderPassFlags{}; + + bool const canUseDepthClamp = + shadowMap.getShadowType() == ShadowType::DIRECTIONAL && + !view.hasVSM() && + mIsDepthClampSupported && + engine.debug.shadowmap.depth_clamp; + + if (canUseDepthClamp) { + renderPassFlags |= RenderPass::HAS_DEPTH_CLAMP; + } + RenderPass const pass = passBuilder + .renderFlags(RenderPass::HAS_DEPTH_CLAMP, renderPassFlags) .camera(cameraInfo) .visibilityMask(entry.visibilityMask) .geometry(scene->getRenderableData(), @@ -641,8 +654,14 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng cameraInfo.zf = -nearFarPlanes[i + 1]; updateNearFarPlanes(&cameraInfo.cullingProjection, cameraInfo.zn, cameraInfo.zf); + bool const canUseDepthClamp = + !view.hasVSM() && + mIsDepthClampSupported && + engine.debug.shadowmap.depth_clamp; + auto shaderParameters = shadowMap.updateDirectional(engine, - lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, USE_DEPTH_CLAMP); + lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, + canUseDepthClamp); if (shadowMap.hasVisibleShadows()) { const size_t shadowIndex = shadowMap.getShadowIndex(); diff --git a/filament/src/ShadowMapManager.h b/filament/src/ShadowMapManager.h index 18b4a24e191..95ad0127741 100644 --- a/filament/src/ShadowMapManager.h +++ b/filament/src/ShadowMapManager.h @@ -232,6 +232,7 @@ class ShadowMapManager { ShadowMapCacheContainer mShadowMapCache; uint32_t mDirectionalShadowMapCount = 0; uint32_t mSpotShadowMapCount = 0; + bool const mIsDepthClampSupported; bool mInitialized = false; ShadowMap& getShadowMap(size_t index) noexcept { diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index d456a7b042c..fa963704177 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -600,6 +600,7 @@ class FEngine : public Engine { bool focus_shadowcasters = true; bool visualize_cascades = false; bool disable_light_frustum_align = false; + bool depth_clamp = true; float dzn = -1.0f; float dzf = 1.0f; float display_shadow_texture_scale = 0.25f; diff --git a/samples/gltf_viewer.cpp b/samples/gltf_viewer.cpp index 6206c0c9a46..e3dc4363a38 100644 --- a/samples/gltf_viewer.cpp +++ b/samples/gltf_viewer.cpp @@ -905,6 +905,8 @@ int main(int argc, char** argv) { debug.getPropertyAddress("d.shadowmap.focus_shadowcasters")); ImGui::Checkbox("Disable light frustum alignment", debug.getPropertyAddress("d.shadowmap.disable_light_frustum_align")); + ImGui::Checkbox("Depth clamp", + debug.getPropertyAddress("d.shadowmap.depth_clamp")); bool debugDirectionalShadowmap; if (debug.getProperty("d.shadowmap.debug_directional_shadowmap", From 3857e3789c9ed2e1c2fc82046487c225dce3be6e Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 13:43:26 -0700 Subject: [PATCH 06/18] a handle with a tag would sometime return "no tag" this is because the key used to retrieve the tag was not "truncated" like it was when inserting the tag in the hash-map. --- .../include/private/backend/HandleAllocator.h | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/filament/backend/include/private/backend/HandleAllocator.h b/filament/backend/include/private/backend/HandleAllocator.h index b61f8e376f0..6a13b75e43b 100644 --- a/filament/backend/include/private/backend/HandleAllocator.h +++ b/filament/backend/include/private/backend/HandleAllocator.h @@ -208,14 +208,8 @@ class HandleAllocator { // TODO: for now, only pool handles check for use-after-free, so we only keep tags for // those if (isPoolHandle(id)) { - // Truncate the tag's age to N bits. - constexpr uint8_t N = 2; // support a history of 4 tags - constexpr uint8_t mask = (1 << N) - 1; - - uint8_t const age = (id & HANDLE_AGE_MASK) >> HANDLE_AGE_SHIFT; - uint8_t const newAge = age & mask; - uint32_t const key = (id & ~HANDLE_AGE_MASK) | (newAge << HANDLE_AGE_SHIFT); - + // Truncate the age to get the debug tag + uint32_t const key = id & ~(HANDLE_DEBUG_TAG_MASK ^ HANDLE_AGE_MASK); // This line is the costly part. In the future, we could potentially use a custom // allocator. mDebugTags[key] = std::move(tag); @@ -226,7 +220,8 @@ class HandleAllocator { if (!isPoolHandle(id)) { return "(no tag)"; } - if (auto pos = mDebugTags.find(id); pos != mDebugTags.end()) { + uint32_t const key = id & ~(HANDLE_DEBUG_TAG_MASK ^ HANDLE_AGE_MASK); + if (auto pos = mDebugTags.find(key); pos != mDebugTags.end()) { return pos->second; } return "(no tag)"; @@ -349,12 +344,24 @@ class HandleAllocator { } } - // we handle a 4 bits age per address - static constexpr uint32_t HANDLE_HEAP_FLAG = 0x80000000u; // pool vs heap handle - static constexpr uint32_t HANDLE_AGE_MASK = 0x78000000u; // handle's age - static constexpr uint32_t HANDLE_INDEX_MASK = 0x07FFFFFFu; // handle index - static constexpr uint32_t HANDLE_TAG_MASK = HANDLE_AGE_MASK; - static constexpr uint32_t HANDLE_AGE_SHIFT = 27; + // number if bits allotted to the handle's age (currently 4 max) + static constexpr uint32_t HANDLE_AGE_BIT_COUNT = 4; + // number if bits allotted to the handle's debug tag (HANDLE_AGE_BIT_COUNT max) + static constexpr uint32_t HANDLE_DEBUG_TAG_BIT_COUNT = 2; + // bit shift for both the age and debug tag + static constexpr uint32_t HANDLE_AGE_SHIFT = 27; + // mask for the heap (vs pool) flag + static constexpr uint32_t HANDLE_HEAP_FLAG = 0x80000000u; + // mask for the age + static constexpr uint32_t HANDLE_AGE_MASK = + ((1 << HANDLE_AGE_BIT_COUNT) - 1) << HANDLE_AGE_SHIFT; + // mask for the debug tag + static constexpr uint32_t HANDLE_DEBUG_TAG_MASK = + ((1 << HANDLE_DEBUG_TAG_BIT_COUNT) - 1) << HANDLE_AGE_SHIFT; + // mask for the index + static constexpr uint32_t HANDLE_INDEX_MASK = 0x07FFFFFFu; + + static_assert(HANDLE_DEBUG_TAG_BIT_COUNT <= HANDLE_AGE_BIT_COUNT); static bool isPoolHandle(HandleBase::HandleId id) noexcept { return (id & HANDLE_HEAP_FLAG) == 0u; @@ -369,7 +376,7 @@ class HandleAllocator { // a non-pool handle. if (UTILS_LIKELY(isPoolHandle(id))) { char* const base = (char*)mHandleArena.getArea().begin(); - uint32_t const tag = id & HANDLE_TAG_MASK; + uint32_t const tag = id & HANDLE_AGE_MASK; size_t const offset = (id & HANDLE_INDEX_MASK) * Allocator::getAlignment(); return { static_cast(base + offset), tag }; } @@ -384,7 +391,7 @@ class HandleAllocator { size_t const offset = (char*)p - base; assert_invariant((offset % Allocator::getAlignment()) == 0); auto id = HandleBase::HandleId(offset / Allocator::getAlignment()); - id |= tag & HANDLE_TAG_MASK; + id |= tag & HANDLE_AGE_MASK; assert_invariant((id & HANDLE_HEAP_FLAG) == 0); return id; } From 950be941eb813a752549c012af64fa06a1952ad8 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 11:32:43 -0700 Subject: [PATCH 07/18] disabling ResourceAllocator led to incorrect assert this is because the "disposer" is now separate from the ResourceAllocator, so even if we don't use the resource allocator we need to register the handle with the disposer. --- filament/src/ResourceAllocator.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/filament/src/ResourceAllocator.cpp b/filament/src/ResourceAllocator.cpp index 471f8778bd8..6d12ccf0a3c 100644 --- a/filament/src/ResourceAllocator.cpp +++ b/filament/src/ResourceAllocator.cpp @@ -170,9 +170,9 @@ backend::TextureHandle ResourceAllocator::createTexture(const char* name, // do we have a suitable texture in the cache? TextureHandle handle; + TextureKey const key{ name, target, levels, format, samples, width, height, depth, usage, swizzle }; if constexpr (mEnabled) { auto& textureCache = mTextureCache; - const TextureKey key{ name, target, levels, format, samples, width, height, depth, usage, swizzle }; auto it = textureCache.find(key); if (UTILS_LIKELY(it != textureCache.end())) { // we do, move the entry to the in-use list, and remove from the cache @@ -190,7 +190,6 @@ backend::TextureHandle ResourceAllocator::createTexture(const char* name, swizzle[0], swizzle[1], swizzle[2], swizzle[3]); } } - mDisposer->checkout(handle, key); } else { if (swizzle == defaultSwizzle) { handle = mBackend.createTexture( @@ -201,12 +200,13 @@ backend::TextureHandle ResourceAllocator::createTexture(const char* name, swizzle[0], swizzle[1], swizzle[2], swizzle[3]); } } + mDisposer->checkout(handle, key); return handle; } void ResourceAllocator::destroyTexture(TextureHandle h) noexcept { + auto const key = mDisposer->checkin(h); if constexpr (mEnabled) { - auto const key = mDisposer->checkin(h); if (UTILS_LIKELY(key.has_value())) { uint32_t const size = key.value().getSize(); mTextureCache.emplace(key.value(), TextureCachePayload{ h, mAge, size }); From c36dd955f41a7c1b7668882f5253db8c274f8786 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 14:10:57 -0700 Subject: [PATCH 08/18] make pushGroupMarker and insertEventMarker null-terminated strings This is because we had this assumption on vk and mtl backends already and also because SYSTRACE works this way and we didn't have non-null terminated strings. This makes things more consistant and less bug prone. --- filament/backend/include/private/backend/DriverAPI.inc | 6 ++---- filament/backend/src/metal/MetalDriver.mm | 4 ++-- filament/backend/src/noop/NoopDriver.cpp | 4 ++-- filament/backend/src/opengl/OpenGLDriver.cpp | 8 ++++---- filament/backend/src/vulkan/VulkanDriver.cpp | 6 +++--- 5 files changed, 13 insertions(+), 15 deletions(-) diff --git a/filament/backend/include/private/backend/DriverAPI.inc b/filament/backend/include/private/backend/DriverAPI.inc index 283e3e0e5ca..295229809a6 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -444,12 +444,10 @@ DECL_DRIVER_API_N(setPushConstant, backend::PushConstantVariant, value) DECL_DRIVER_API_N(insertEventMarker, - const char*, string, - uint32_t, len = 0) + const char*, string) DECL_DRIVER_API_N(pushGroupMarker, - const char*, string, - uint32_t, len = 0) + const char*, string) DECL_DRIVER_API_0(popGroupMarker) diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 5c12736b914..0269c049c10 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -1302,11 +1302,11 @@ pushConstants.setPushConstant(value, index); } -void MetalDriver::insertEventMarker(const char* string, uint32_t len) { +void MetalDriver::insertEventMarker(const char* string) { } -void MetalDriver::pushGroupMarker(const char* string, uint32_t len) { +void MetalDriver::pushGroupMarker(const char* string) { mContext->groupMarkers.push(string); } diff --git a/filament/backend/src/noop/NoopDriver.cpp b/filament/backend/src/noop/NoopDriver.cpp index 096f57c3989..cd3029d64bc 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -320,10 +320,10 @@ void NoopDriver::setPushConstant(backend::ShaderStage stage, uint8_t index, backend::PushConstantVariant value) { } -void NoopDriver::insertEventMarker(char const* string, uint32_t len) { +void NoopDriver::insertEventMarker(char const* string) { } -void NoopDriver::pushGroupMarker(char const* string, uint32_t len) { +void NoopDriver::pushGroupMarker(char const* string) { } void NoopDriver::popGroupMarker(int) { diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index ed23067bb87..29987453281 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -3196,23 +3196,23 @@ void OpenGLDriver::bindSamplers(uint32_t index, Handle sbh) { CHECK_GL_ERROR(utils::slog.e) } -void OpenGLDriver::insertEventMarker(char const* string, uint32_t len) { +void OpenGLDriver::insertEventMarker(char const* string) { #ifndef __EMSCRIPTEN__ #ifdef GL_EXT_debug_marker auto& gl = mContext; if (gl.ext.EXT_debug_marker) { - glInsertEventMarkerEXT(GLsizei(len ? len : strlen(string)), string); + glInsertEventMarkerEXT(GLsizei(strlen(string)), string); } #endif #endif } -void OpenGLDriver::pushGroupMarker(char const* string, uint32_t len) { +void OpenGLDriver::pushGroupMarker(char const* string) { #ifndef __EMSCRIPTEN__ #ifdef GL_EXT_debug_marker #if DEBUG_GROUP_MARKER_LEVEL & DEBUG_GROUP_MARKER_OPENGL if (UTILS_LIKELY(mContext.ext.EXT_debug_marker)) { - glPushGroupMarkerEXT(GLsizei(len ? len : strlen(string)), string); + glPushGroupMarkerEXT(GLsizei(strlen(string)), string); } #endif #endif diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 41924b96e03..0ff4c879041 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1606,13 +1606,13 @@ void VulkanDriver::setPushConstant(backend::ShaderStage stage, uint8_t index, value); } -void VulkanDriver::insertEventMarker(char const* string, uint32_t len) { +void VulkanDriver::insertEventMarker(char const* string) { #if FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS) - mCommands.insertEventMarker(string, len); + mCommands.insertEventMarker(string, strlen(string)); #endif } -void VulkanDriver::pushGroupMarker(char const* string, uint32_t) { +void VulkanDriver::pushGroupMarker(char const* string) { // Turns out all the markers are 0-terminated, so we can just pass it without len. #if FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS) mCommands.pushGroupMarker(string); From 339e8da97668c10faa9ddcb8e55bf45e6011e434 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 15:07:42 -0700 Subject: [PATCH 09/18] repair the "no buffer padding case" with a previous change we were too aggressive in falling back to "no buffer padding", we need to do this only in the case where "as subpass" would have been used if supported by the h/w. --- filament/src/details/Renderer.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index 3cac2bafefb..a32a978f604 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -651,14 +651,19 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { const bool isProtectedContent = mSwapChain && mSwapChain->isProtected(); + // Conditions to meet to be able to use the sub-pass rendering path. This is regardless of + // whether the backend supports subpasses (or if they are disabled but the debugRegistry) + const bool isSubpassPossible = + msaaSampleCount <= 1 && + hasColorGrading && + !bloomOptions.enabled && !dofOptions.enabled && !taaOptions.enabled; + // asSubpass is disabled with TAA (although it's supported) because performance was degraded // on qualcomm hardware -- we might need a backend dependent toggle at some point const PostProcessManager::ColorGradingConfig colorGradingConfig{ .asSubpass = - msaaSampleCount <= 1 && + isSubpassPossible && driver.isFrameBufferFetchSupported() && - hasColorGrading && - !bloomOptions.enabled && !dofOptions.enabled && !taaOptions.enabled && !engine.debug.renderer.disable_subpasses, .customResolve = msaaSampleCount > 1 && @@ -702,8 +707,8 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { // this case, we would need an extra blit to "resolve" the buffer padding (because there are no // other pass that can do it as a side effect). In this case, it is better to skip the padding, // which won't be helping much. - const bool noBufferPadding - = (!hasFXAA && !scaled) || engine.debug.renderer.disable_buffer_padding; + const bool noBufferPadding = (isSubpassPossible && + !hasFXAA && !scaled) || engine.debug.renderer.disable_buffer_padding; // guardBand must be a multiple of 16 to guarantee the same exact rendering up to 4 mip levels. float const guardBand = guardBandOptions.enabled ? 16.0f : 0.0f; From 2bbbb7f4d1a015c86b54708a2cbab64698d5827a Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 15:28:18 -0700 Subject: [PATCH 10/18] Update filament/src/details/Renderer.cpp Co-authored-by: Powei Feng --- filament/src/details/Renderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index a32a978f604..9348ed9e0ce 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -652,7 +652,7 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { const bool isProtectedContent = mSwapChain && mSwapChain->isProtected(); // Conditions to meet to be able to use the sub-pass rendering path. This is regardless of - // whether the backend supports subpasses (or if they are disabled but the debugRegistry) + // whether the backend supports subpasses (or if they are disabled in the debugRegistry). const bool isSubpassPossible = msaaSampleCount <= 1 && hasColorGrading && From 2aa51db614b946ddfec3597ada1f0eb2a7154ffa Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 13:41:45 -0700 Subject: [PATCH 11/18] add texture tagging in the FrameGraph Also tag user texture "FTexture" if user doesn't provide a tag, this is so that we can distinguish them from internal textures that might not be tagged. --- filament/src/ResourceAllocator.cpp | 7 +++++-- filament/src/details/Texture.cpp | 2 ++ filament/src/details/View.cpp | 12 ++++-------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/filament/src/ResourceAllocator.cpp b/filament/src/ResourceAllocator.cpp index 6d12ccf0a3c..4699def134e 100644 --- a/filament/src/ResourceAllocator.cpp +++ b/filament/src/ResourceAllocator.cpp @@ -143,12 +143,14 @@ void ResourceAllocator::terminate() noexcept { } } -RenderTargetHandle ResourceAllocator::createRenderTarget(const char*, +RenderTargetHandle ResourceAllocator::createRenderTarget(const char* name, TargetBufferFlags targetBufferFlags, uint32_t width, uint32_t height, uint8_t samples, uint8_t layerCount, MRT color, TargetBufferInfo depth, TargetBufferInfo stencil) noexcept { - return mBackend.createRenderTarget(targetBufferFlags, + auto handle = mBackend.createRenderTarget(targetBufferFlags, width, height, samples ? samples : 1u, layerCount, color, depth, stencil); + mBackend.setDebugTag(handle.getId(), CString{ name }); + return handle; } void ResourceAllocator::destroyRenderTarget(RenderTargetHandle h) noexcept { @@ -201,6 +203,7 @@ backend::TextureHandle ResourceAllocator::createTexture(const char* name, } } mDisposer->checkout(handle, key); + mBackend.setDebugTag(handle.getId(), CString{ name }); return handle; } diff --git a/filament/src/details/Texture.cpp b/filament/src/details/Texture.cpp index a59a24da97d..b2cfbf68391 100644 --- a/filament/src/details/Texture.cpp +++ b/filament/src/details/Texture.cpp @@ -245,6 +245,8 @@ FTexture::FTexture(FEngine& engine, const Builder& builder) { } if (auto name = builder.getName(); !name.empty()) { driver.setDebugTag(mHandle.getId(), std::move(name)); + } else { + driver.setDebugTag(mHandle.getId(), CString{"FTexture"}); } } diff --git a/filament/src/details/View.cpp b/filament/src/details/View.cpp index 85fc9df5caa..f9bf724921f 100644 --- a/filament/src/details/View.cpp +++ b/filament/src/details/View.cpp @@ -1040,10 +1040,8 @@ void FView::commitFrameHistory(FEngine& engine) noexcept { auto& frameHistory = mFrameHistory; FrameHistoryEntry& last = frameHistory.back(); - disposer.destroy(last.taa.color.handle); - disposer.destroy(last.ssr.color.handle); - last.taa.color.handle.clear(); - last.ssr.color.handle.clear(); + disposer.destroy(std::move(last.taa.color.handle)); + disposer.destroy(std::move(last.ssr.color.handle)); // and then push the new history entry to the history stack frameHistory.commit(); @@ -1055,10 +1053,8 @@ void FView::clearFrameHistory(FEngine& engine) noexcept { auto& frameHistory = mFrameHistory; for (size_t i = 0; i < frameHistory.size(); ++i) { FrameHistoryEntry& last = frameHistory[i]; - disposer.destroy(last.taa.color.handle); - disposer.destroy(last.ssr.color.handle); - last.taa.color.handle.clear(); - last.ssr.color.handle.clear(); + disposer.destroy(std::move(last.taa.color.handle)); + disposer.destroy(std::move(last.ssr.color.handle)); } } From 2b620e65fdb71ddcc5d9bb758fab8bbf542345d6 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 30 Aug 2024 14:48:47 -0700 Subject: [PATCH 12/18] fix potential framegraph textures use-after free make sure to unset all textures in the per-view sampler group after they are used, because the resource could be destroyed after the pass is finished - unset the fog and ibl_specular after the color pass - move that cleanup a bit earlier - in the case of screen-space reflection the structure pass is set, but might not be used in the color pass, so we also need to unset it after the SSR pass and before any other passes. --- filament/src/PerViewUniforms.cpp | 4 +++- filament/src/details/Renderer.cpp | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/filament/src/PerViewUniforms.cpp b/filament/src/PerViewUniforms.cpp index eaf8df8cb46..35867ea855b 100644 --- a/filament/src/PerViewUniforms.cpp +++ b/filament/src/PerViewUniforms.cpp @@ -459,10 +459,12 @@ void PerViewUniforms::bind(backend::DriverApi& driver) noexcept { void PerViewUniforms::unbindSamplers() noexcept { auto& samplerGroup = mSamplers; + samplerGroup.clearSampler(PerViewSib::SHADOW_MAP); + samplerGroup.clearSampler(PerViewSib::IBL_SPECULAR); samplerGroup.clearSampler(PerViewSib::SSAO); samplerGroup.clearSampler(PerViewSib::SSR); samplerGroup.clearSampler(PerViewSib::STRUCTURE); - samplerGroup.clearSampler(PerViewSib::SHADOW_MAP); + samplerGroup.clearSampler(PerViewSib::FOG); } } // namespace filament diff --git a/filament/src/details/Renderer.cpp b/filament/src/details/Renderer.cpp index 9348ed9e0ce..a8e71b48ddc 100644 --- a/filament/src/details/Renderer.cpp +++ b/filament/src/details/Renderer.cpp @@ -1019,6 +1019,10 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { { .width = svp.width, .height = svp.height }); if (UTILS_LIKELY(reflections)) { + fg.addTrivialSideEffectPass("SSR Cleanup", [&view](DriverApi& driver) { + view.getPerViewUniforms().prepareStructure({}); + view.commitUniforms(driver); + }); // generate the mipchain PostProcessManager::generateMipmapSSR(ppm, fg, reflections, ssrConfig.reflection, false, ssrConfig); @@ -1134,6 +1138,12 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { } } + fg.addTrivialSideEffectPass("Finish Color Passes", [&view](DriverApi& driver) { + // Unbind SSAO sampler, b/c the FrameGraph will delete the texture at the end of the pass. + view.cleanupRenderPasses(); + view.commitUniforms(driver); + }); + if (colorGradingConfig.customResolve) { assert_invariant(fg.getDescriptor(colorPassOutput.linearColor).samples <= 1); // TODO: we have to "uncompress" (i.e. detonemap) the color buffer here because it's used @@ -1172,16 +1182,11 @@ void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) { // this is the output of the color pass / input to post processing, // this is only used later for comparing it with the output after post-processing FrameGraphId const postProcessInput = colorGradingConfig.asSubpass ? - colorPassOutput.tonemappedColor : - colorPassOutput.linearColor; + colorPassOutput.tonemappedColor : + colorPassOutput.linearColor; // input can change below FrameGraphId input = postProcessInput; - fg.addTrivialSideEffectPass("Finish Color Passes", [&view](DriverApi& driver) { - // Unbind SSAO sampler, b/c the FrameGraph will delete the texture at the end of the pass. - view.cleanupRenderPasses(); - view.commitUniforms(driver); - }); // Resolve depth -- which might be needed because of TAA or DoF. This pass will be culled // if the depth is not used below or if the depth is not MS (e.g. it could have been From 16bed4de00cb1ca40cf57467b291b79639f26ebd Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Tue, 3 Sep 2024 09:58:13 -0700 Subject: [PATCH 13/18] Enable mmap on iOS (#8100) --- filament/backend/src/CircularBuffer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/src/CircularBuffer.cpp b/filament/backend/src/CircularBuffer.cpp index 256762332db..b8796623e01 100644 --- a/filament/backend/src/CircularBuffer.cpp +++ b/filament/backend/src/CircularBuffer.cpp @@ -24,7 +24,7 @@ #include #include -#if !defined(WIN32) && !defined(__EMSCRIPTEN__) && !defined(IOS) +#if !defined(WIN32) && !defined(__EMSCRIPTEN__) # include # include # define HAS_MMAP 1 From cdb539b3cf13774c0deb9a091ed734e72bf78a7d Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 4 Sep 2024 09:36:54 -0700 Subject: [PATCH 14/18] Make builderMakeName public This call is used in the BuilderNameMixin template definition, which is a public API. --- filament/include/filament/FilamentAPI.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/filament/include/filament/FilamentAPI.h b/filament/include/filament/FilamentAPI.h index 7a6f16d6307..7fa19433847 100644 --- a/filament/include/filament/FilamentAPI.h +++ b/filament/include/filament/FilamentAPI.h @@ -55,7 +55,8 @@ class UTILS_PUBLIC FilamentAPI { template using BuilderBase = utils::PrivateImplementation; -void builderMakeName(utils::CString& outName, const char* name, size_t len) noexcept; +// This needs to be public because it is used in the following template. +UTILS_PUBLIC void builderMakeName(utils::CString& outName, const char* name, size_t len) noexcept; template class UTILS_PUBLIC BuilderNameMixin { From c6776073535759178f8c3afff9f69a2dc3946ec2 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 3 Sep 2024 15:22:05 -0700 Subject: [PATCH 15/18] github: Split Android CI to per ABI builds - Pulled the android continuous workflow into an action - Split the android continuous build into 3 ABIs armv7, armv8a, and x86_64 (note that x86 is not present). - Remove the upload artifacts step from previous workflow. This was meant for letting client try a tip-of-tree Android build. We can revisit this later. (Also removed mention in README.md) - Split the android continous into debug build and a release build commands, enabling deletion of intermediate output directory. --- .github/actions/android-continuous/action.yml | 17 +++++++ .github/workflows/android-continuous.yml | 47 ++++++++++--------- .github/workflows/presubmit.yml | 4 +- .github/workflows/release.yml | 2 +- README.md | 10 ---- build/android/build.sh | 22 ++++++--- 6 files changed, 61 insertions(+), 41 deletions(-) create mode 100644 .github/actions/android-continuous/action.yml diff --git a/.github/actions/android-continuous/action.yml b/.github/actions/android-continuous/action.yml new file mode 100644 index 00000000000..ecf23fc5244 --- /dev/null +++ b/.github/actions/android-continuous/action.yml @@ -0,0 +1,17 @@ +name: 'Android Continuous' +inputs: + build-abi: + description: 'The target platform ABI' + required: true + default: 'armeabi-v7a' +runs: + using: "composite" + steps: + - uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: '17' + - name: Run build script + run: | + cd build/android && printf "y" | ./build.sh continuous ${{ inputs.build-abi }} + shell: bash diff --git a/.github/workflows/android-continuous.yml b/.github/workflows/android-continuous.yml index 9bcab7ae8e0..9ade2be0d0e 100644 --- a/.github/workflows/android-continuous.yml +++ b/.github/workflows/android-continuous.yml @@ -8,32 +8,35 @@ on: - rc/** jobs: - build-android: - name: build-android + build-android-armv7: + name: build-android-armv7 runs-on: macos-14 steps: - uses: actions/checkout@v4.1.6 - - uses: actions/setup-java@v3 + - name: Run Android Continuous + uses: ./.github/actions/android-continuous with: - distribution: 'temurin' - java-version: '17' - - name: Run build script - run: | - cd build/android && printf "y" | ./build.sh continuous - - uses: actions/upload-artifact@v1.0.0 - with: - name: filament-android - path: out/filament-android-release.aar - - uses: actions/upload-artifact@v1.0.0 - with: - name: filamat-android-full - path: out/filamat-android-release.aar - - uses: actions/upload-artifact@v1.0.0 + build-abi: armeabi-v7a + + build-android-armv8a: + name: build-android-armv8a + runs-on: macos-14 + + steps: + - uses: actions/checkout@v4.1.6 + - name: Run Android Continuous + uses: ./.github/actions/android-continuous with: - name: gltfio-android-release - path: out/gltfio-android-release.aar - - uses: actions/upload-artifact@v1.0.0 + build-abi: arm64-v8a + + build-android-x86_64: + name: build-android-x86_64 + runs-on: macos-14 + + steps: + - uses: actions/checkout@v4.1.6 + - name: Run Android Continuous + uses: ./.github/actions/android-continuous with: - name: filament-utils-android-release - path: out/filament-utils-android-release.aar + build-abi: x86_64 diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml index 3fa9918e7ef..c7a160ed982 100644 --- a/.github/workflows/presubmit.yml +++ b/.github/workflows/presubmit.yml @@ -49,8 +49,10 @@ jobs: distribution: 'temurin' java-version: '17' - name: Run build script + # Only build 1 64 bit target during presubmit to cut down build times during presubmit + # Continuous builds will build everything run: | - cd build/android && printf "y" | ./build.sh presubmit + cd build/android && printf "y" | ./build.sh presubmit arm64-v8a build-ios: name: build-iOS diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d4b7239a1a5..4d070e50770 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -120,7 +120,7 @@ jobs: env: TAG: ${{ steps.git_ref.outputs.tag }} run: | - cd build/android && printf "y" | ./build.sh release + cd build/android && printf "y" | ./build.sh release armeabi-v7a,arm64-v8a,x86,x86_64 cd ../.. mv out/filament-android-release.aar out/filament-${TAG}-android.aar mv out/filamat-android-release.aar out/filamat-${TAG}-android.aar diff --git a/README.md b/README.md index f0e9ca349de..bc6b7db01a9 100644 --- a/README.md +++ b/README.md @@ -54,16 +54,6 @@ iOS projects can use CocoaPods to install the latest release: pod 'Filament', '~> 1.54.1' ``` -### Snapshots - -If you prefer to live on the edge, you can download a continuous build by following the following -steps: - -1. Find the [commit](https://github.com/google/filament/commits/main) you're interested in. -2. Click the green check mark under the commit message. -3. Click on the _Details_ link for the platform you're interested in. -4. On the top left click _Summary_, then in the _Artifacts_ section choose the desired artifact. - ## Documentation - [Filament](https://google.github.io/filament/Filament.html), an in-depth explanation of diff --git a/build/android/build.sh b/build/android/build.sh index 3eb0a98a44e..c9821b8fb43 100755 --- a/build/android/build.sh +++ b/build/android/build.sh @@ -60,13 +60,7 @@ if [[ ! -d "${ANDROID_HOME}/ndk/$FILAMENT_NDK_VERSION" ]]; then yes | ${ANDROID_HOME}/cmdline-tools/latest/bin/sdkmanager --licenses ${ANDROID_HOME}/cmdline-tools/latest/bin/sdkmanager "ndk;$FILAMENT_NDK_VERSION" fi - -# Only build 1 64 bit target during presubmit to cut down build times during presubmit -# Continuous builds will build everything ANDROID_ABIS= -if [[ "$TARGET" == "presubmit" ]]; then - ANDROID_ABIS="-q arm64-v8a" -fi # Build the Android sample-gltf-viewer APK during release. BUILD_SAMPLES= @@ -74,5 +68,19 @@ if [[ "$TARGET" == "release" ]]; then BUILD_SAMPLES="-k sample-gltf-viewer" fi +function build_android() { + local ABI=$1 + + # Do the following in two steps so that we do not run out of space + if [[ -n "${BUILD_DEBUG}" ]]; then + FILAMENT_NDK_VERSION=${FILAMENT_NDK_VERSION} ./build.sh -p android -q ${ABI} -c ${BUILD_SAMPLES} ${GENERATE_ARCHIVES} ${BUILD_DEBUG} + rm -rf out/cmake-android-debug-* + fi + if [[ -n "${BUILD_RELEASE}" ]]; then + FILAMENT_NDK_VERSION=${FILAMENT_NDK_VERSION} ./build.sh -p android -q ${ABI} -c ${BUILD_SAMPLES} ${GENERATE_ARCHIVES} ${BUILD_RELEASE} + rm -rf out/cmake-android-release-* + fi +} + pushd `dirname $0`/../.. > /dev/null -FILAMENT_NDK_VERSION=${FILAMENT_NDK_VERSION} ./build.sh -p android $ANDROID_ABIS -c $BUILD_SAMPLES $GENERATE_ARCHIVES $BUILD_DEBUG $BUILD_RELEASE +build_android $2 From 1b4afbab511cb67fe2b13a6455232637cb158338 Mon Sep 17 00:00:00 2001 From: Sungun Park Date: Wed, 4 Sep 2024 18:55:51 +0000 Subject: [PATCH 16/18] Release Filament 1.54.2 --- 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 bc6b7db01a9..bdf20804b93 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.54.1' + implementation 'com.google.android.filament:filament-android:1.54.2' } ``` @@ -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.54.1' +pod 'Filament', '~> 1.54.2' ``` ## Documentation diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 0bad42c9327..48144bb1d81 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.54.3 + + ## v1.54.2 - Add a `name` API to Filament objects for debugging handle use-after-free assertions diff --git a/android/gradle.properties b/android/gradle.properties index 994ce214046..510e34eb0a4 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.54.1 +VERSION_NAME=1.54.2 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 1e87d19b7ee..73fe0a78f81 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.54.1" + spec.version = "1.54.2" 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.54.1/filament-v1.54.1-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.54.2/filament-v1.54.2-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 21ca5f2f834..7b502cf79ab 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.54.1", + "version": "1.54.2", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From c967fb7860701c87b512c3faaabcee6920d901b2 Mon Sep 17 00:00:00 2001 From: Sungun Park Date: Wed, 4 Sep 2024 18:55:59 +0000 Subject: [PATCH 17/18] Bump version to 1.54.3 --- 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 bdf20804b93..43a3ef8de2b 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.54.2' + implementation 'com.google.android.filament:filament-android:1.54.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.54.2' +pod 'Filament', '~> 1.54.3' ``` ## Documentation diff --git a/android/gradle.properties b/android/gradle.properties index 510e34eb0a4..6b5d7c8454c 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.54.2 +VERSION_NAME=1.54.3 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 73fe0a78f81..c324c7cc115 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.54.2" + spec.version = "1.54.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.54.2/filament-v1.54.2-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.54.3/filament-v1.54.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 7b502cf79ab..410de0fec0a 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.54.2", + "version": "1.54.3", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 32b0625f36423d85dc4689ea5a11efcac5ed2ec9 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Thu, 5 Sep 2024 14:14:00 -0700 Subject: [PATCH 18/18] Revert "reenable the SimplificationPass in spirv-opt" This reverts commit 30387af61cbd29a9e9f3bdbc6959938222b64c81. Causes breakage on Pixel 8pro for 1P apps. --- libs/filamat/src/GLSLPostProcessor.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/libs/filamat/src/GLSLPostProcessor.cpp b/libs/filamat/src/GLSLPostProcessor.cpp index bd879b1ee0c..3a4a4824e0e 100644 --- a/libs/filamat/src/GLSLPostProcessor.cpp +++ b/libs/filamat/src/GLSLPostProcessor.cpp @@ -671,15 +671,13 @@ void GLSLPostProcessor::fixupClipDistance( // - 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 // -// Note: CreateSimplificationPass() used to creates a lot of problems: +// 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) -// -// However this problem was addressed in spirv-cross here: -// https://github.com/KhronosGroup/SPIRV-Cross/pull/2163 -// -// The simplification passes below are necessary when targeting Metal, otherwise the +// - 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. @@ -712,11 +710,11 @@ void GLSLPostProcessor::registerPerformancePasses(Optimizer& optimizer, Config c RegisterPass(CreateAggressiveDCEPass()); RegisterPass(CreateRedundancyEliminationPass()); RegisterPass(CreateCombineAccessChainsPass()); - RegisterPass(CreateSimplificationPass()); + RegisterPass(CreateSimplificationPass(), MaterialBuilder::TargetApi::METAL); RegisterPass(CreateVectorDCEPass()); RegisterPass(CreateDeadInsertElimPass()); RegisterPass(CreateDeadBranchElimPass()); - RegisterPass(CreateSimplificationPass()); + RegisterPass(CreateSimplificationPass(), MaterialBuilder::TargetApi::METAL); RegisterPass(CreateIfConversionPass()); RegisterPass(CreateCopyPropagateArraysPass()); RegisterPass(CreateReduceLoadSizePass()); @@ -725,7 +723,7 @@ void GLSLPostProcessor::registerPerformancePasses(Optimizer& optimizer, Config c RegisterPass(CreateRedundancyEliminationPass()); RegisterPass(CreateDeadBranchElimPass()); RegisterPass(CreateBlockMergePass()); - RegisterPass(CreateSimplificationPass()); + RegisterPass(CreateSimplificationPass(), MaterialBuilder::TargetApi::METAL); } void GLSLPostProcessor::registerSizePasses(Optimizer& optimizer, Config const& config) {