From 44a954b55981a46da51eeec033017de4a1ab14f4 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 13 Aug 2024 16:28:33 -0700 Subject: [PATCH 01/13] Fix misnumbered version in RELEASE_NOTES --- RELEASE_NOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 95f63bfa5b..84193ace37 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,7 +7,7 @@ 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.53.6 +## v1.54.0 - materials: add a new `stereoscopicType` material parameter. [⚠️ **New Material Version**] - Fix a crash when compiling shaders on IMG devices From 1c817026f28f172cd181e1ee0b7cd297c4359627 Mon Sep 17 00:00:00 2001 From: Sungun Park Date: Wed, 14 Aug 2024 08:41:10 -0700 Subject: [PATCH 02/13] Remove unused code (#8043) --- filament/src/PostProcessManager.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/filament/src/PostProcessManager.cpp b/filament/src/PostProcessManager.cpp index da5c120b8e..cc15f8b895 100644 --- a/filament/src/PostProcessManager.cpp +++ b/filament/src/PostProcessManager.cpp @@ -494,7 +494,6 @@ PostProcessManager::StructurePassOutput PostProcessManager::structure(FrameGraph struct StructureMipmapData { FrameGraphId depth; - uint32_t rt[8]; }; fg.addPass("StructureMipmap", @@ -505,7 +504,7 @@ PostProcessManager::StructurePassOutput PostProcessManager::structure(FrameGraph .level = uint8_t(i) }); out = builder.write(out, FrameGraphTexture::Usage::DEPTH_ATTACHMENT); - data.rt[i - 1] = builder.declareRenderPass("Structure mip target", { + builder.declareRenderPass("Structure mip target", { .attachments = { .depth = out } }); } From ad29b9c70a3bf6e59fdcb22eca84a183fddb97cb Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 15 Aug 2024 00:18:56 -0700 Subject: [PATCH 03/13] fix several issue with the debug datasource in View - the last View created was always overriding previous View's datasource - because of lazy registering of the data source it was possible that the registering lambda was called after the view was destroyed, leading to crashes - all view would share the same PID parameters and these would be initialized to default value instead of the user provided value. so debug build would behave differently. With this change we improve things: - now only the first view gets to publish its data source. it's still not ideal, but works for our use case with gltf_Viewer - the view can now unregister itself when it's destroyed - only the view that successfully registered uses the debug PID values and publishes its data source. - the normal parameters are used until we query the datasource (from imgui), so by default the behavior is now identical to release builds This fixes a crash in gltf_viewer when opening the Debug panel. --- filament/src/details/DebugRegistry.cpp | 21 ++++++-- filament/src/details/DebugRegistry.h | 6 ++- filament/src/details/View.cpp | 66 ++++++++++++++++++-------- filament/src/details/View.h | 7 ++- 4 files changed, 74 insertions(+), 26 deletions(-) diff --git a/filament/src/details/DebugRegistry.cpp b/filament/src/details/DebugRegistry.cpp index ad1a54df74..c2f5d3d3fb 100644 --- a/filament/src/details/DebugRegistry.cpp +++ b/filament/src/details/DebugRegistry.cpp @@ -118,19 +118,32 @@ template bool FDebugRegistry::getProperty(const char* name, float2* v) c template bool FDebugRegistry::getProperty(const char* name, float3* v) const noexcept; template bool FDebugRegistry::getProperty(const char* name, float4* v) const noexcept; -void FDebugRegistry::registerDataSource(std::string_view name, +bool FDebugRegistry::registerDataSource(std::string_view name, void const* data, size_t count) noexcept { auto& dataSourceMap = mDataSourceMap; - if (dataSourceMap.find(name) == dataSourceMap.end()) { + bool const found = dataSourceMap.find(name) == dataSourceMap.end(); + if (found) { dataSourceMap[name] = { data, count }; } + return found; } -void FDebugRegistry::registerDataSource(std::string_view name, +bool FDebugRegistry::registerDataSource(std::string_view name, utils::Invocable&& creator) noexcept { - mDataSourceCreatorMap[name] = std::move(creator); + auto& dataSourceCreatorMap = mDataSourceCreatorMap; + bool const found = dataSourceCreatorMap.find(name) == dataSourceCreatorMap.end(); + if (found) { + dataSourceCreatorMap[name] = std::move(creator); + } + return found; } +void FDebugRegistry::unregisterDataSource(std::string_view name) noexcept { + mDataSourceCreatorMap.erase(name); + mDataSourceMap.erase(name); +} + + DebugRegistry::DataSource FDebugRegistry::getDataSource(const char* name) const noexcept { std::string_view const key{ name }; auto& dataSourceMap = mDataSourceMap; diff --git a/filament/src/details/DebugRegistry.h b/filament/src/details/DebugRegistry.h index b60a1c6994..92a25b82e9 100644 --- a/filament/src/details/DebugRegistry.h +++ b/filament/src/details/DebugRegistry.h @@ -101,12 +101,14 @@ public: } // registers a DataSource directly - void registerDataSource(std::string_view name, void const* data, size_t count) noexcept; + bool registerDataSource(std::string_view name, void const* data, size_t count) noexcept; // registers a DataSource lazily - void registerDataSource(std::string_view name, + bool registerDataSource(std::string_view name, utils::Invocable&& creator) noexcept; + void unregisterDataSource(std::string_view name) noexcept; + #if !defined(_MSC_VER) private: #endif diff --git a/filament/src/details/View.cpp b/filament/src/details/View.cpp index 1a3b54deb2..85fc9df5ca 100644 --- a/filament/src/details/View.cpp +++ b/filament/src/details/View.cpp @@ -31,15 +31,17 @@ #include "details/Skybox.h" #include +#include #include #include #include +#include +#include #include #include #include -#include #include #include @@ -47,6 +49,7 @@ #include #include +#include using namespace utils; @@ -78,21 +81,29 @@ FView::FView(FEngine& engine) mPidController.setOutputDeadBand(-0.01f, 0.05f); #ifndef NDEBUG - debugRegistry.registerDataSource("d.view.frame_info", - [this]() -> DebugRegistry::DataSource { - assert_invariant(!mDebugFrameHistory); - mDebugFrameHistory = std::make_unique>(); - return { mDebugFrameHistory->data(), mDebugFrameHistory->size() }; + // This can fail if another view has already registered this data source + mDebugState->owner = debugRegistry.registerDataSource("d.view.frame_info", + [weak = std::weak_ptr(mDebugState)]() -> DebugRegistry::DataSource { + // the View could have been destroyed by the time we do this + auto const state = weak.lock(); + if (!state) { + return { nullptr, 0 }; + } + // Lazily allocate the buffer for the debug data source, and mark this + // data source as active. It can never go back to inactive. + assert_invariant(!state->debugFrameHistory); + state->active = true; + state->debugFrameHistory = + std::make_unique>(); + return { state->debugFrameHistory->data(), state->debugFrameHistory->size() }; }); - debugRegistry.registerProperty("d.view.pid.kp", &engine.debug.view.pid.kp); - debugRegistry.registerProperty("d.view.pid.ki", &engine.debug.view.pid.ki); - debugRegistry.registerProperty("d.view.pid.kd", &engine.debug.view.pid.kd); - // default parameters for debugging UI - engine.debug.view.pid.kp = 1.0f - std::exp(-1.0f / 8.0f); - engine.debug.view.pid.ki = PID_CONTROLLER_Ki; - engine.debug.view.pid.kd = PID_CONTROLLER_Kd; - mPidController.setParallelGains( - engine.debug.view.pid.kp, engine.debug.view.pid.ki, engine.debug.view.pid.kd); + + if (UTILS_UNLIKELY(mDebugState->owner)) { + // publish the properties (they will be initialized in the main loop) + debugRegistry.registerProperty("d.view.pid.kp", &engine.debug.view.pid.kp); + debugRegistry.registerProperty("d.view.pid.ki", &engine.debug.view.pid.ki); + debugRegistry.registerProperty("d.view.pid.kd", &engine.debug.view.pid.kd); + } #endif // allocate UBOs @@ -126,6 +137,12 @@ void FView::terminate(FEngine& engine) { mFroxelizer.terminate(driver); engine.getEntityManager().destroy(mFogEntity); + +#ifndef NDEBUG + if (UTILS_UNLIKELY(mDebugState->owner)) { + engine.getDebugRegistry().unregisterDataSource("d.view.frame_info"); + } +#endif } void FView::setViewport(filament::Viewport const& viewport) noexcept { @@ -167,6 +184,16 @@ float2 FView::updateScale(FEngine& engine, Renderer::FrameRateOptions const& frameRateOptions, Renderer::DisplayInfo const& displayInfo) noexcept { +#ifndef NDEBUG + if (UTILS_LIKELY(!mDebugState->active)) { + // if we're not active, update the debug properties with the normal values + // and use that for configuring the PID controller. + engine.debug.view.pid.kp = 1.0f - std::exp(-frameRateOptions.scaleRate); + engine.debug.view.pid.ki = PID_CONTROLLER_Ki; + engine.debug.view.pid.kd = PID_CONTROLLER_Kd; + } +#endif + DynamicResolutionOptions const& options = mDynamicResolution; if (options.enabled) { if (!UTILS_UNLIKELY(info.valid)) { @@ -250,14 +277,15 @@ float2 FView::updateScale(FEngine& engine, #ifndef NDEBUG // only for debugging... - if (mDebugFrameHistory) { + if (UTILS_UNLIKELY(mDebugState->active && mDebugState->debugFrameHistory)) { + auto* const debugFrameHistory = mDebugState->debugFrameHistory.get(); using namespace std::chrono; using duration_ms = duration; const float target = (1000.0f * float(frameRateOptions.interval)) / displayInfo.refreshRate; const float targetWithHeadroom = target * (1.0f - frameRateOptions.headRoomRatio); - std::move(mDebugFrameHistory->begin() + 1, - mDebugFrameHistory->end(), mDebugFrameHistory->begin()); - mDebugFrameHistory->back() = { + std::move(debugFrameHistory->begin() + 1, + debugFrameHistory->end(), debugFrameHistory->begin()); + debugFrameHistory->back() = { .target = target, .targetWithHeadroom = targetWithHeadroom, .frameTime = duration_cast(info.frameTime).count(), diff --git a/filament/src/details/View.h b/filament/src/details/View.h index 73e7c9dc61..e24f150a2d 100644 --- a/filament/src/details/View.h +++ b/filament/src/details/View.h @@ -576,7 +576,12 @@ private: }}; #ifndef NDEBUG - std::unique_ptr> mDebugFrameHistory; + struct DebugState { + std::unique_ptr> debugFrameHistory{}; + bool owner = false; + bool active = false; + }; + std::shared_ptr mDebugState{ new DebugState }; #endif }; From 26f4239d8c49feae54801512c6cd95583e107d90 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 16 Aug 2024 10:49:17 -0700 Subject: [PATCH 04/13] fix a few issues with shadowing A recent change broken the optional "depth clamp" as well as the computation of the far plane of the light frustum. There was also a case where DEBUG builds could assert. - The far plane was no longer being "optimized" (i.e. moved as close as possible), which resulted in less optimal use of the shadow texture. the far plane can be moved as close as the farthest visible shadow caster. - After the camera/light frustums intersection we now see of the 2D bounds seen from the light are empty and if so we bail, which prevents an assertion later. - finally, the "DEPTH_CLAMP" option is also updated for the new code structure. --- filament/src/ShadowMap.cpp | 66 ++++++++++++++++++++----------- filament/src/ShadowMap.h | 6 ++- filament/src/ShadowMapManager.cpp | 9 ++++- 3 files changed, 53 insertions(+), 28 deletions(-) diff --git a/filament/src/ShadowMap.cpp b/filament/src/ShadowMap.cpp index 29e843ec67..bb43797989 100644 --- a/filament/src/ShadowMap.cpp +++ b/filament/src/ShadowMap.cpp @@ -57,9 +57,6 @@ namespace filament { using namespace math; using namespace backend; -// do this only if depth-clamp is available -static constexpr bool USE_DEPTH_CLAMP = false; - ShadowMap::ShadowMap(FEngine& engine) noexcept : mPerShadowMapUniforms(engine), mShadowType(ShadowType::DIRECTIONAL), @@ -126,7 +123,8 @@ ShadowMap::ShaderParameters ShadowMap::updateDirectional(FEngine& engine, FScene::LightSoa const& lightData, size_t index, filament::CameraInfo const& camera, ShadowMapInfo const& shadowMapInfo, - SceneInfo const& sceneInfo) noexcept { + SceneInfo const& sceneInfo, + bool useDepthClamp) noexcept { // reset the visible shadow status mHasVisibleShadows = false; @@ -137,13 +135,20 @@ ShadowMap::ShaderParameters ShadowMap::updateDirectional(FEngine& engine, const auto direction = lightData.elementAt(index); - auto [Mv, znear, zfar, lsClippedShadowVolume, vertexCount, visibleShadows] = - computeDirectionalShadowBounds(engine, direction, params, camera, sceneInfo); + auto const [Mv, znear, zfar, lsClippedShadowVolume, vertexCount, visibleShadows] = + computeDirectionalShadowBounds(engine, direction, params, camera, sceneInfo, + useDepthClamp); if (UTILS_UNLIKELY(!visibleShadows)) { return {}; } + // if the 2D bounds of the intersection is empty seen from the light, we have no shadow + Aabb const bounds = compute2DBounds(Mv, lsClippedShadowVolume.data(), vertexCount); + if (UTILS_UNLIKELY(!(bounds.min.x < bounds.max.x && bounds.min.y < bounds.max.y))) { + return {}; + } + /* * Compute the light's projection matrix * (directional/point lights, i.e. projection to use, including znear/zfar clip planes) @@ -386,7 +391,8 @@ ShadowMap::DirectionalShadowBounds ShadowMap::computeDirectionalShadowBounds( float3 const direction, FLightManager::ShadowParams params, filament::CameraInfo const& camera, - SceneInfo const& sceneInfo) noexcept { + SceneInfo const& sceneInfo, + bool useDepthClamp) noexcept { // we use aligned_storage<> here to avoid the default initialization of std::array<> using Storage = std::aligned_storage::type; @@ -457,27 +463,39 @@ ShadowMap::DirectionalShadowBounds ShadowMap::computeDirectionalShadowBounds( } /* - * Adjust the scene's zmax (i.e. Near plane) and zmin (i.e. Far plane) in light space. - * (near/far correspond to max/min because the light looks down the -z axis). - * - The Near plane is set to the shadow casters max z (i.e. closest to the light) - * - The Far plane is set to the closest of the farthest shadow casters and receivers - * i.e.: shadow casters behind the last receivers can't cast any shadows - * - * If "depth clamp" is supported, we can further tighten the near plane to the - * shadow receiver. - * - * Note: L has no influence here, since we're only interested in z values - * (L is a rotation around z) - */ + * Adjust the scene's zmax (i.e. Near plane) and zmin (i.e. Far plane) in light space. + * (near/far correspond to max/min because the light looks down the -z axis). + * + * - For the Far plane, the light frustum is already set to the closest of receivers and + * casters (i.e.: shadow casters behind the last receivers can't cast any shadows). + * But it can be moved closer up to the farthest point in the camera frustum & light + * frustum intersection. + * + * - The Near plane is already set to the shadow casters max z (i.e. closest to the light) + * + * If "depth clamp" is supported, we can further tighten the near plane to the + * shadow receiver (i.e. to the closest point of camera frustum & light frustum intersection). + * + * Note: L has no influence here, since we're only interested in z values + * (L is a rotation around z) + */ + float lsClippedShadowVolumeFarthest = std::numeric_limits::max(); + float lsClippedShadowVolumeNearest = std::numeric_limits::min(); for (size_t i = 0; i < vertexCount; ++i) { float3 const v = lsClippedShadowVolume[i]; - // far: figure out the farthest shadow receivers - lsLightFrustumBounds.min.z = min(lsLightFrustumBounds.min.z, v.z); - if constexpr (USE_DEPTH_CLAMP) { - lsLightFrustumBounds.max.z = max(lsLightFrustumBounds.max.z, v.z); - } + lsClippedShadowVolumeFarthest = std::min(lsClippedShadowVolumeFarthest, v.z); + lsClippedShadowVolumeNearest = std::max(lsClippedShadowVolumeNearest, v.z); } + + lsLightFrustumBounds.min.z = + std::max(lsLightFrustumBounds.min.z, lsClippedShadowVolumeFarthest); + + if (useDepthClamp) { + lsLightFrustumBounds.max.z = + std::min(lsLightFrustumBounds.max.z, lsClippedShadowVolumeNearest); + } + if (engine.debug.shadowmap.far_uses_shadowcasters) { // far: closest of the farthest shadow casters and receivers lsLightFrustumBounds.min.z = diff --git a/filament/src/ShadowMap.h b/filament/src/ShadowMap.h index d0ca26945f..98b8ce15c0 100644 --- a/filament/src/ShadowMap.h +++ b/filament/src/ShadowMap.h @@ -154,7 +154,8 @@ public: const FScene::LightSoa& lightData, size_t index, filament::CameraInfo const& camera, ShadowMapInfo const& shadowMapInfo, - SceneInfo const& sceneInfo) noexcept; + SceneInfo const& sceneInfo, + bool useDepthClamp) noexcept; ShaderParameters updateSpot(FEngine& engine, const FScene::LightSoa& lightData, size_t index, @@ -244,7 +245,8 @@ private: math::float3 direction, FLightManager::ShadowParams params, filament::CameraInfo const& camera, - SceneInfo const& sceneInfo) noexcept; + SceneInfo const& sceneInfo, + bool useDepthClamp) noexcept; static math::mat4f applyLISPSM(math::mat4f& Wp, filament::CameraInfo const& camera, FLightManager::ShadowParams const& params, diff --git a/filament/src/ShadowMapManager.cpp b/filament/src/ShadowMapManager.cpp index 8943371855..4a40519b42 100644 --- a/filament/src/ShadowMapManager.cpp +++ b/filament/src/ShadowMapManager.cpp @@ -63,6 +63,9 @@ 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) { FDebugRegistry& debugRegistry = engine.getDebugRegistry(); debugRegistry.registerProperty("d.shadowmap.visualize_cascades", @@ -631,8 +634,10 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng // Compute scene-dependent values shared across all cascades ShadowMap::updateSceneInfoDirectional(MvAtOrigin, *scene, sceneInfo); + // we always do culling without depth clamp, because objects behind the camera + // must be rendered regardless shadowMap.updateDirectional(engine, - lightData, 0, cameraInfo, shadowMapInfo, sceneInfo); + lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, false); hasVisibleShadows = shadowMap.hasVisibleShadows(); @@ -698,7 +703,7 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng sceneInfo.csNearFar = { csSplitPosition[i], csSplitPosition[i + 1] }; auto shaderParameters = shadowMap.updateDirectional(engine, - lightData, 0, cameraInfo, shadowMapInfo, sceneInfo); + lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, USE_DEPTH_CLAMP); if (shadowMap.hasVisibleShadows()) { const size_t shadowIndex = shadowMap.getShadowIndex(); From 1795c4059166ec515966c3d8f27ff26cf26b5692 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 19 Aug 2024 00:01:24 -0700 Subject: [PATCH 05/13] fix typo when calculation shadowmap frustum min() and lowest() are different! --- filament/src/ShadowMap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/src/ShadowMap.cpp b/filament/src/ShadowMap.cpp index bb43797989..aa50c3fff9 100644 --- a/filament/src/ShadowMap.cpp +++ b/filament/src/ShadowMap.cpp @@ -481,7 +481,7 @@ ShadowMap::DirectionalShadowBounds ShadowMap::computeDirectionalShadowBounds( */ float lsClippedShadowVolumeFarthest = std::numeric_limits::max(); - float lsClippedShadowVolumeNearest = std::numeric_limits::min(); + float lsClippedShadowVolumeNearest = std::numeric_limits::lowest(); for (size_t i = 0; i < vertexCount; ++i) { float3 const v = lsClippedShadowVolume[i]; lsClippedShadowVolumeFarthest = std::min(lsClippedShadowVolumeFarthest, v.z); From 5966b5dd8fadfa685070c78b38026afa29529e7b Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Sat, 17 Aug 2024 00:07:47 -0700 Subject: [PATCH 06/13] fix shadow cascade computations shadow cascades where not calculated properly because part of the calculation took the cascade near/far into account, while another part didn't. This resulted in cascades being too large. It didn't create wrong shadows, but reduced (and in some case canceled) the usefulness of the cascade. We fix the problem by always using the projection matrix only for describing the cascade's frustum, as opposed to just passing the near/far plane distances. Now the calculation of each cascade is completely self contained and identical. We also improve the orientation of the light frustum: We can rotate the light frustum around the light direction axis, so it aligns with the view direction, this generally result in smaller light frustums. This cannot be used in stable mode. --- filament/src/ShadowMap.cpp | 38 +++---- filament/src/ShadowMap.h | 16 +-- filament/src/ShadowMapManager.cpp | 160 +++++++++++++++--------------- filament/src/ShadowMapManager.h | 17 ++-- filament/src/details/Engine.h | 5 +- samples/gltf_viewer.cpp | 2 + 6 files changed, 114 insertions(+), 124 deletions(-) diff --git a/filament/src/ShadowMap.cpp b/filament/src/ShadowMap.cpp index aa50c3fff9..4c40668054 100644 --- a/filament/src/ShadowMap.cpp +++ b/filament/src/ShadowMap.cpp @@ -166,12 +166,17 @@ ShadowMap::ShaderParameters ShadowMap::updateDirectional(FEngine& engine, mat4f W, Wp, L; // We can't use LISPSM in stable mode const bool useLispsm = params.options.lispsm && !params.options.stable; - if (useLispsm) { + + if (useLispsm || + (!params.options.stable && !engine.debug.shadowmap.disable_light_frustum_align)) { // Orient the shadow map in the direction of the view vector by constructing a const float3 lsCameraFwd = Mv.upperLeft() * camera.getForwardVector(); L = computeLightRotation(lsCameraFwd); LMp = L * Mp; LMpMv = LMp * Mv; + } + + if (useLispsm) { W = applyLISPSM(Wp, camera, params, LMp, Mv, LMpMv, lsClippedShadowVolume, vertexCount, direction); } @@ -189,8 +194,7 @@ ShadowMap::ShaderParameters ShadowMap::updateDirectional(FEngine& engine, // In LiPSM mode, we're using the warped space here. float4 f = computeFocusParams(LMpMv, WLMp, lsClippedShadowVolume, vertexCount, - camera, sceneInfo.csNearFar, - params.options.shadowFar, params.options.stable); + camera, params.options.shadowFar, params.options.stable); if (params.options.stable) { const auto lsRef = lightData.elementAt(index); @@ -454,9 +458,10 @@ ShadowMap::DirectionalShadowBounds ShadowMap::computeDirectionalShadowBounds( // Compute the intersection of the view volume with the intersection of receivers and casters // in light space. This returns a set of points on the convex-hull of the intersection. + mat4f const projection = camera.cullingProjection * + (camera.view * FCamera::rigidTransformInverse(MvAtOrigin)); size_t const vertexCount = intersectFrustumWithBox(lsClippedShadowVolume, - camera.cullingProjection * camera.view * FCamera::rigidTransformInverse(MvAtOrigin), - sceneInfo.csNearFar, lsLightFrustumBounds); + projection, lsLightFrustumBounds); if (UTILS_UNLIKELY(vertexCount < 4)) { return {}; @@ -629,7 +634,7 @@ math::float4 ShadowMap::computeFocusParams( mat4f const& LMpMv, mat4f const& WLMp, FrustumBoxIntersection const& lsShadowVolume, size_t vertexCount, - filament::CameraInfo const& camera, float2 const& csNearFar, + filament::CameraInfo const& camera, float shadowFar, bool stable) noexcept { float2 s, o; if (stable) { @@ -644,7 +649,7 @@ math::float4 ShadowMap::computeFocusParams( return wsViewVolumeBoundingSphere; } else { mat4f const viewFromClip = inverse(camera.cullingProjection); - Corners const wsFrustumVertices = computeFrustumCorners(viewFromClip, csNearFar); + Corners const wsFrustumVertices = computeFrustumCorners(viewFromClip); float4 const wsViewVolumeBoundingSphere = computeBoundingSphere(wsFrustumVertices.vertices, 8); return wsViewVolumeBoundingSphere; @@ -812,15 +817,14 @@ Aabb ShadowMap::compute2DBounds(const mat4f& lightView, } ShadowMap::Corners ShadowMap::computeFrustumCorners( - mat4f const& projectionInverse, - float2 csNearFar) noexcept { + mat4f const& projectionInverse) noexcept { Corners out; // compute view frustum in world space (from its NDC) // matrix to convert: ndc -> camera -> world - float const near = csNearFar.x; - float const far = csNearFar.y; + float const near = -1.0f; + float const far = 1.0f; Corners const csViewFrustumCorners = { .vertices = { @@ -913,11 +917,11 @@ void ShadowMap::snapLightFrustum(float2& s, float2& o, size_t ShadowMap::intersectFrustumWithBox( FrustumBoxIntersection& UTILS_RESTRICT outVertices, - mat4f const& UTILS_RESTRICT projection, math::float2 const& UTILS_RESTRICT csNearFar, + mat4f const& UTILS_RESTRICT projection, Aabb const& UTILS_RESTRICT box) { Frustum const frustum{ projection }; - Corners const frustumVertices{ computeFrustumCorners(inverse(projection), csNearFar) }; + Corners const frustumVertices{ computeFrustumCorners(inverse(projection)) }; constexpr const float EPSILON = 1.0f / 8192.0f; // ~0.012 mm size_t vertexCount = 0; @@ -1221,9 +1225,8 @@ void ShadowMap::visitScene(const FScene& scene, uint32_t visibleLayers, } ShadowMap::SceneInfo::SceneInfo( - FScene const& scene, uint8_t visibleLayers, mat4f const& viewMatrix) noexcept - : vsNearFar(std::numeric_limits::lowest(), std::numeric_limits::max()), - visibleLayers(visibleLayers) { + FScene const& scene, uint8_t visibleLayers) noexcept + : visibleLayers(visibleLayers) { // the code below only works with affine transforms // Filament's API requires viewMatrix to be rigid (and thus affine). @@ -1242,9 +1245,6 @@ ShadowMap::SceneInfo::SceneInfo( [&](Aabb receiver, Culler::result_type) { wsShadowReceiversVolume.min = min(wsShadowReceiversVolume.min, receiver.min); wsShadowReceiversVolume.max = max(wsShadowReceiversVolume.max, receiver.max); - auto r = Aabb::transform(viewMatrix.upperLeft(), viewMatrix[3].xyz, receiver); - vsNearFar.x = std::max(vsNearFar.x, r.max.z); - vsNearFar.y = std::min(vsNearFar.y, r.min.z); } ); } diff --git a/filament/src/ShadowMap.h b/filament/src/ShadowMap.h index 98b8ce15c0..58e8f9bfbc 100644 --- a/filament/src/ShadowMap.h +++ b/filament/src/ShadowMap.h @@ -109,20 +109,13 @@ public: struct SceneInfo { SceneInfo() noexcept = default; - SceneInfo(FScene const& scene, uint8_t visibleLayers, math::mat4f const& viewMatrix) noexcept; - - // scratch data: The near and far planes, in clip space, to use for this shadow map - math::float2 csNearFar = { -1.0f, 1.0f }; + SceneInfo(FScene const& scene, uint8_t visibleLayers) noexcept; // scratch data: light's near/far expressed in light-space, calculated from the scene's // content assuming the light is at the origin. math::float2 lsCastersNearFar; math::float2 lsReceiversNearFar; - // Viewing camera's near/far expressed in view-space, calculated from the - // scene's content. - math::float2 vsNearFar; - // World-space shadow-casters volume Aabb wsShadowCastersVolume; @@ -262,7 +255,7 @@ private: math::mat4f const& LMpMv, math::mat4f const& WLMp, FrustumBoxIntersection const& lsShadowVolume, size_t vertexCount, - filament::CameraInfo const& camera, math::float2 const& csNearFar, + filament::CameraInfo const& camera, float shadowFar, bool stable) noexcept; static inline void snapLightFrustum(math::float2& s, math::float2& o, @@ -273,8 +266,7 @@ private: SceneInfo const& sceneInfo, bool stable, bool focusShadowCasters, bool farUsesShadowCasters) noexcept; - static Corners computeFrustumCorners(const math::mat4f& projectionInverse, - math::float2 csNearFar = { -1.0f, 1.0f }) noexcept; + static Corners computeFrustumCorners(const math::mat4f& projectionInverse) noexcept; static inline math::float2 computeNearFar(math::mat4f const& view, math::float3 const* wsVertices, size_t count) noexcept; @@ -309,7 +301,7 @@ private: static size_t intersectFrustumWithBox( FrustumBoxIntersection& outVertices, - math::mat4f const& projection, math::float2 const& csNearFar, + math::mat4f const& projection, Aabb const& box); static math::mat4f warpFrustum(float n, float f) noexcept; diff --git a/filament/src/ShadowMapManager.cpp b/filament/src/ShadowMapManager.cpp index 4a40519b42..63a1bb900e 100644 --- a/filament/src/ShadowMapManager.cpp +++ b/filament/src/ShadowMapManager.cpp @@ -39,17 +39,20 @@ #include #include -#include -#include -#include #include #include +#include +#include +#include +#include #include #include #include #include +#include +#include #include #include #include @@ -70,8 +73,8 @@ ShadowMapManager::ShadowMapManager(FEngine& engine) { FDebugRegistry& debugRegistry = engine.getDebugRegistry(); debugRegistry.registerProperty("d.shadowmap.visualize_cascades", &engine.debug.shadowmap.visualize_cascades); - debugRegistry.registerProperty("d.shadowmap.tightly_bound_scene", - &engine.debug.shadowmap.tightly_bound_scene); + debugRegistry.registerProperty("d.shadowmap.disable_light_frustum_align", + &engine.debug.shadowmap.disable_light_frustum_align); } ShadowMapManager::~ShadowMapManager() { @@ -150,7 +153,7 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::update( calculateTextureRequirements(engine, view, lightData); // Compute scene-dependent values shared across all shadow maps - ShadowMap::SceneInfo const info{ *view.getScene(), view.getVisibleLayers(), cameraInfo.view }; + ShadowMap::SceneInfo const info{ *view.getScene(), view.getVisibleLayers() }; shadowTechnique |= updateCascadeShadowMaps( engine, view, cameraInfo, renderableData, lightData, info); @@ -546,65 +549,9 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng FLightManager::ShadowParams const& params = lcm.getShadowParams(directionalLight); // Adjust the camera's projection for the light's shadowFar - cameraInfo.zf = params.options.shadowFar > 0.0f ? params.options.shadowFar : cameraInfo.zf; if (UTILS_UNLIKELY(params.options.shadowFar > 0.0f)) { cameraInfo.zf = params.options.shadowFar; - float const n = cameraInfo.zn; - float const f = cameraInfo.zf; - - /* - * Updating a projection matrix near and far planes: - * - * We assume that the near and far plane equations are of the form: - * N = { 0, 0, 1, n } - * F = { 0, 0, -1, -f } - * - * We also assume that the lower-left 2x2 of the projection is all 0: - * P = A 0 C 0 - * 0 B D 0 - * 0 0 E F - * 0 0 G H - * - * It result that we need to calculate E and F while leaving all other parameter unchanged. - * - * We know that: - * with N, F the near/far normalized plane equation parameters - * sn, sf arbitrary scale factors (they don't affect the planes) - * m is the transpose of projection (see Frustum.cpp) - * - * sn * N == -m[3] - m[2] - * sf * F == -m[3] + m[2] - * - * sn * N + sf * F == -2 * m[3] - * sn * N - sf * F == -2 * m[2] - * - * sn * N.z + sf * F.z == -2 * m[3].z - * sn * N.w + sf * F.w == -2 * m[3].w - * sn * N.z - sf * F.z == -2 * m[2].z - * sn * N.w - sf * F.w == -2 * m[2].w - * - * sn * N.z + sf * F.z == -2 * p[2].w - * sn * N.w + sf * F.w == -2 * p[3].w - * sn * N.z - sf * F.z == -2 * p[2].z - * sn * N.w - sf * F.w == -2 * p[3].z - * - * We now need to solve for { p[2].z, p[3].z, sn, sf } : - * - * sn == -2 * (p[2].w * F.w - p[3].w * F.z) / (N.z * F.w - N.w * F.z) - * sf == -2 * (p[2].w * N.w - p[3].w * N.z) / (F.z * N.w - F.w * N.z) - * p[2].z == (sf * F.z - sn * N.z) / 2 - * p[3].z == (sf * F.w - sn * N.w) / 2 - */ - auto& p = cameraInfo.cullingProjection; - float4 const N = { 0, 0, 1, n }; // near plane equation - float4 const F = { 0, 0, -1, -f }; // far plane equation - // near plane equation scale factor - float const sn = -2.0f * (p[2].w * F.w - p[3].w * F.z) / (N.z * F.w - N.w * F.z); - // far plane equation scale factor - float const sf = -2.0f * (p[2].w * N.w - p[3].w * N.z) / (F.z * N.w - F.w * N.z); - // New values for the projection - p[2].z = (sf * F.z - sn * N.z) * 0.5f; - p[3].z = (sf * F.w - sn * N.w) * 0.5f; + updateNearFarPlanes(&cameraInfo.cullingProjection, cameraInfo.zn, cameraInfo.zf); } const ShadowMap::ShadowMapInfo shadowMapInfo{ @@ -653,15 +600,7 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng uint32_t cascadeHasVisibleShadows = 0; if (hasVisibleShadows) { - // Adjust the near and far planes to tightly bound the scene. - float vsNear = -cameraInfo.zn; - float vsFar = -cameraInfo.zf; - if (engine.debug.shadowmap.tightly_bound_scene && !params.options.stable) { - vsNear = std::min(vsNear, sceneInfo.vsNearFar.x); - vsFar = std::max(vsFar, sceneInfo.vsNearFar.y); - } - - const size_t cascadeCount = cascadedShadowMaps.size(); + uint32_t const cascadeCount = cascadedShadowMaps.size(); // We divide the camera frustum into N cascades. This gives us N + 1 split positions. // The first split position is the near plane; the last split position is the far plane. @@ -672,9 +611,8 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng } const CascadeSplits splits({ - .proj = cameraInfo.cullingProjection, - .near = vsNear, - .far = vsFar, + .near = -cameraInfo.zn, + .far = -cameraInfo.zf, .cascadeCount = cascadeCount, .splitPositions = splitPercentages }); @@ -684,10 +622,7 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng static_assert(CONFIG_MAX_SHADOW_CASCADES <= 5, "At most, a float4 can fit 4 split positions for 5 shadow cascades"); float4 wsSplitPositionUniform{ -std::numeric_limits::infinity() }; - std::copy(splits.beginWs() + 1, splits.endWs(), &wsSplitPositionUniform[0]); - - float csSplitPosition[CONFIG_MAX_SHADOW_CASCADES + 1]; - std::copy(splits.beginCs(), splits.endCs(), csSplitPosition); + std::copy(splits.begin() + 1, splits.end(), &wsSplitPositionUniform[0]); mShadowMappingUniforms.cascadeSplits = wsSplitPositionUniform; @@ -700,7 +635,11 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng ShadowMap& shadowMap = cascadedShadowMaps[i]; assert_invariant(shadowMap.getLightIndex() == 0); - sceneInfo.csNearFar = { csSplitPosition[i], csSplitPosition[i + 1] }; + // update cameraInfo culling projection for the cascade + float const* nearFarPlanes = splits.begin(); + cameraInfo.zn = -nearFarPlanes[i]; + cameraInfo.zf = -nearFarPlanes[i + 1]; + updateNearFarPlanes(&cameraInfo.cullingProjection, cameraInfo.zn, cameraInfo.zf); auto shaderParameters = shadowMap.updateDirectional(engine, lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, USE_DEPTH_CLAMP); @@ -1074,8 +1013,67 @@ ShadowMapManager::CascadeSplits::CascadeSplits(Params const& params) noexcept : mSplitCount(params.cascadeCount + 1) { for (size_t s = 0; s < mSplitCount; s++) { mSplitsWs[s] = params.near + (params.far - params.near) * params.splitPositions[s]; - mSplitsCs[s] = mat4f::project(params.proj, float3(0.0f, 0.0f, mSplitsWs[s])).z; } } +void ShadowMapManager::updateNearFarPlanes(mat4f* projection, + float nearDistance, float farDistance) noexcept { + float const n = nearDistance; + float const f = farDistance; + + /* + * Updating a projection matrix near and far planes: + * + * We assume that the near and far plane equations are of the form: + * N = { 0, 0, 1, n } + * F = { 0, 0, -1, -f } + * + * We also assume that the lower-left 2x2 of the projection is all 0: + * P = A 0 C 0 + * 0 B D 0 + * 0 0 E F + * 0 0 G H + * + * It result that we need to calculate E and F while leaving all other parameter unchanged. + * + * We know that: + * with N, F the near/far normalized plane equation parameters + * sn, sf arbitrary scale factors (they don't affect the planes) + * m is the transpose of projection (see Frustum.cpp) + * + * sn * N == -m[3] - m[2] + * sf * F == -m[3] + m[2] + * + * sn * N + sf * F == -2 * m[3] + * sn * N - sf * F == -2 * m[2] + * + * sn * N.z + sf * F.z == -2 * m[3].z + * sn * N.w + sf * F.w == -2 * m[3].w + * sn * N.z - sf * F.z == -2 * m[2].z + * sn * N.w - sf * F.w == -2 * m[2].w + * + * sn * N.z + sf * F.z == -2 * p[2].w + * sn * N.w + sf * F.w == -2 * p[3].w + * sn * N.z - sf * F.z == -2 * p[2].z + * sn * N.w - sf * F.w == -2 * p[3].z + * + * We now need to solve for { p[2].z, p[3].z, sn, sf } : + * + * sn == -2 * (p[2].w * F.w - p[3].w * F.z) / (N.z * F.w - N.w * F.z) + * sf == -2 * (p[2].w * N.w - p[3].w * N.z) / (F.z * N.w - F.w * N.z) + * p[2].z == (sf * F.z - sn * N.z) / 2 + * p[3].z == (sf * F.w - sn * N.w) / 2 + */ + auto& p = *projection; + float4 const N = { 0, 0, 1, n }; // near plane equation + float4 const F = { 0, 0, -1, -f }; // far plane equation + // near plane equation scale factor + float const sn = -2.0f * (p[2].w * F.w - p[3].w * F.z) / (N.z * F.w - N.w * F.z); + // far plane equation scale factor + float const sf = -2.0f * (p[2].w * N.w - p[3].w * N.z) / (F.z * N.w - F.w * N.z); + // New values for the projection + p[2].z = (sf * F.z - sn * N.z) * 0.5f; + p[3].z = (sf * F.w - sn * N.w) * 0.5f; +} + } // namespace filament diff --git a/filament/src/ShadowMapManager.h b/filament/src/ShadowMapManager.h index 38017f5c8b..8b12eaa6a0 100644 --- a/filament/src/ShadowMapManager.h +++ b/filament/src/ShadowMapManager.h @@ -148,6 +148,9 @@ private: void terminate(FEngine& engine); + static void updateNearFarPlanes(math::mat4f* projection, + float nearDistance, float farDistance) noexcept; + ShadowMapManager::ShadowTechnique updateCascadeShadowMaps(FEngine& engine, FView& view, CameraInfo cameraInfo, FScene::RenderableSoa& renderableData, FScene::LightSoa const& lightData, ShadowMap::SceneInfo sceneInfo) noexcept; @@ -186,27 +189,21 @@ private: constexpr static size_t SPLIT_COUNT = CONFIG_MAX_SHADOW_CASCADES + 1; struct Params { - math::mat4f proj; float near = 0.0f; float far = 0.0f; - size_t cascadeCount = 1; + uint32_t cascadeCount = 1; std::array splitPositions = { 0.0f }; }; explicit CascadeSplits(Params const& params) noexcept; // Split positions in world-space. - const float* beginWs() const { return mSplitsWs; } - const float* endWs() const { return mSplitsWs + mSplitCount; } - - // Split positions in clip-space. - const float* beginCs() const { return mSplitsCs; } - const float* endCs() const { return mSplitsCs + mSplitCount; } + const float* begin() const { return mSplitsWs; } + const float* end() const { return mSplitsWs + mSplitCount; } private: float mSplitsWs[SPLIT_COUNT]; - float mSplitsCs[SPLIT_COUNT]; - size_t mSplitCount; + uint32_t mSplitCount; }; // Atlas requirements, updated in ShadowMapManager::update(), diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index 7290b9d5cd..d456a7b042 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -590,7 +590,8 @@ private: Config mConfig; public: - // these are the debug properties used by FDebug. They're accessed directly by modules who need them. + // These are the debug properties used by FDebug. + // They're accessed directly by modules who need them. struct { struct { bool debug_directional_shadowmap = false; @@ -598,7 +599,7 @@ public: bool far_uses_shadowcasters = true; bool focus_shadowcasters = true; bool visualize_cascades = false; - bool tightly_bound_scene = true; + bool disable_light_frustum_align = false; 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 d3481c485d..47d8c84369 100644 --- a/samples/gltf_viewer.cpp +++ b/samples/gltf_viewer.cpp @@ -894,6 +894,8 @@ int main(int argc, char** argv) { debug.getPropertyAddress("d.shadowmap.far_uses_shadowcasters")); ImGui::Checkbox("Focus shadow casters", debug.getPropertyAddress("d.shadowmap.focus_shadowcasters")); + ImGui::Checkbox("Disable light frustum alignment", + debug.getPropertyAddress("d.shadowmap.disable_light_frustum_align")); bool debugDirectionalShadowmap; if (debug.getProperty("d.shadowmap.debug_directional_shadowmap", From 9c857f64aeef948b6ddb1c78a25ea56fce6f48ff Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 16 Aug 2024 20:57:58 -0700 Subject: [PATCH 07/13] improve split-view mode - side panel doesn't overlap with content anymore --- libs/filamentapp/src/FilamentApp.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/libs/filamentapp/src/FilamentApp.cpp b/libs/filamentapp/src/FilamentApp.cpp index 84071b4261..8ffc4c0f34 100644 --- a/libs/filamentapp/src/FilamentApp.cpp +++ b/libs/filamentapp/src/FilamentApp.cpp @@ -907,9 +907,7 @@ void FilamentApp::Window::configureCamerasForWindow() { const bool splitview = mViews.size() > 2; - // To trigger a floating-point exception, users could shrink the window to be smaller than - // the sidebar. To prevent this we simply clamp the width of the main viewport. - const uint32_t mainWidth = splitview ? width : std::max(1, (int) width - sidebar); + const uint32_t mainWidth = std::max(2, (int) width - sidebar); double near = mFilamentApp->mCameraNear; double far = mFilamentApp->mCameraFar; @@ -924,7 +922,7 @@ void FilamentApp::Window::configureCamerasForWindow() { } else { mMainCamera->setLensProjection(mFilamentApp->mCameraFocalLength, 1.0, near, far); } - mDebugCamera->setProjection(45.0, double(width) / height, 0.0625, 4096, Camera::Fov::VERTICAL); + mDebugCamera->setProjection(45.0, double(mainWidth) / height, 0.0625, 4096, Camera::Fov::VERTICAL); auto aspectRatio = double(mainWidth) / height; if (mMainView->getView()->getStereoscopicOptions().enabled) { @@ -935,12 +933,12 @@ void FilamentApp::Window::configureCamerasForWindow() { // We're in split view when there are more views than just the Main and UI views. if (splitview) { - uint32_t vpw = width / 2; - uint32_t vph = height / 2; - mMainView->setViewport ({ 0, 0, vpw, vph }); - mDepthView->setViewport({ int32_t(vpw), 0, width - vpw, vph }); - mGodView->setViewport ({ int32_t(vpw), int32_t(vph), width - vpw, height - vph }); - mOrthoView->setViewport({ 0, int32_t(vph), vpw, height - vph }); + uint32_t const vpw = mainWidth / 2; + uint32_t const vph = height / 2; + mMainView->setViewport ({ sidebar + 0, 0, vpw, vph }); + mDepthView->setViewport({ sidebar + int32_t(vpw), 0, vpw, vph }); + mGodView->setViewport ({ sidebar + int32_t(vpw), int32_t(vph), vpw, vph }); + mOrthoView->setViewport({ sidebar + 0, int32_t(vph), vpw, vph }); } else { mMainView->setViewport({ sidebar, 0, mainWidth, height }); } From 063affb61261a366225b44a7f03c76f0b2210d32 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 16 Aug 2024 23:54:55 -0700 Subject: [PATCH 08/13] more improvement of csm display --- filament/include/filament/View.h | 3 +- filament/src/ShadowMapManager.cpp | 11 +++ filament/src/ShadowMapManager.h | 13 ++-- filament/src/View.cpp | 4 +- filament/src/details/View.h | 6 +- libs/filamentapp/include/filamentapp/Cube.h | 10 ++- .../include/filamentapp/FilamentApp.h | 8 +- libs/filamentapp/src/Cube.cpp | 16 +++- libs/filamentapp/src/FilamentApp.cpp | 75 ++++++++++++------- 9 files changed, 102 insertions(+), 44 deletions(-) diff --git a/filament/include/filament/View.h b/filament/include/filament/View.h index 5264cdae49..d4c9848de8 100644 --- a/filament/include/filament/View.h +++ b/filament/include/filament/View.h @@ -24,6 +24,7 @@ #include #include +#include #include @@ -727,7 +728,7 @@ public: void setDebugCamera(Camera* UTILS_NULLABLE camera) noexcept; //! debugging: returns a Camera from the point of view of *the* dominant directional light used for shadowing. - Camera const* UTILS_NULLABLE getDirectionalShadowCamera() const noexcept; + utils::FixedCapacityVector getDirectionalShadowCameras() const noexcept; /** Result of a picking query */ diff --git a/filament/src/ShadowMapManager.cpp b/filament/src/ShadowMapManager.cpp index 63a1bb900e..a6f4a9fa3c 100644 --- a/filament/src/ShadowMapManager.cpp +++ b/filament/src/ShadowMapManager.cpp @@ -1076,4 +1076,15 @@ void ShadowMapManager::updateNearFarPlanes(mat4f* projection, p[3].z = (sf * F.w - sn * N.w) * 0.5f; } +utils::FixedCapacityVector +ShadowMapManager::getDirectionalShadowCameras() const noexcept { + if (!mInitialized) return {}; + auto const csm = getCascadedShadowMap(); + auto result = utils::FixedCapacityVector::with_capacity(csm.size()); + for (ShadowMap const& sm : csm) { + result.push_back(sm.hasVisibleShadows() ? sm.getDebugCamera() : nullptr); + } + return result; +} + } // namespace filament diff --git a/filament/src/ShadowMapManager.h b/filament/src/ShadowMapManager.h index 8b12eaa6a0..18b4a24e19 100644 --- a/filament/src/ShadowMapManager.h +++ b/filament/src/ShadowMapManager.h @@ -40,6 +40,7 @@ #include #include +#include #include #include #include @@ -58,6 +59,7 @@ namespace filament { +class Camera; class FCamera; class FView; class FrameGraph; @@ -138,10 +140,7 @@ public: bool hasSpotShadows() const { return !mSpotShadowMapCount; } // for debugging only - FCamera const* getDirectionalShadowCamera() const noexcept { - if (!mInitialized) return nullptr; - return getShadowMap(0).getDebugCamera(); - } + utils::FixedCapacityVector getDirectionalShadowCameras() const noexcept; private: explicit ShadowMapManager(FEngine& engine); @@ -245,10 +244,14 @@ private: } utils::Slice getCascadedShadowMap() noexcept { - ShadowMap* const p = &getShadowMap(0); + ShadowMap const* const p = &getShadowMap(0); return { p, mDirectionalShadowMapCount }; } + utils::Slice getCascadedShadowMap() const noexcept { + return const_cast(this)->getCascadedShadowMap(); + } + utils::Slice getSpotShadowMaps() noexcept { ShadowMap* const p = &getShadowMap(CONFIG_MAX_SHADOW_CASCADES); return { p, mSpotShadowMapCount }; diff --git a/filament/src/View.cpp b/filament/src/View.cpp index abdd615ab8..65635f3f64 100644 --- a/filament/src/View.cpp +++ b/filament/src/View.cpp @@ -73,8 +73,8 @@ const char* View::getName() const noexcept { return downcast(this)->getName(); } -Camera const* View::getDirectionalShadowCamera() const noexcept { - return downcast(this)->getDirectionalShadowCamera(); +utils::FixedCapacityVector View::getDirectionalShadowCameras() const noexcept { + return downcast(this)->getDirectionalShadowCameras(); } void View::setShadowingEnabled(bool enabled) noexcept { diff --git a/filament/src/details/View.h b/filament/src/details/View.h index e24f150a2d..2a2f9a414f 100644 --- a/filament/src/details/View.h +++ b/filament/src/details/View.h @@ -201,9 +201,9 @@ public: void setStereoscopicOptions(StereoscopicOptions const& options) noexcept; - FCamera const* getDirectionalShadowCamera() const noexcept { - if (!mShadowMapManager) return nullptr; - return mShadowMapManager->getDirectionalShadowCamera(); + utils::FixedCapacityVector getDirectionalShadowCameras() const noexcept { + if (!mShadowMapManager) return {}; + return mShadowMapManager->getDirectionalShadowCameras(); } void setRenderTarget(FRenderTarget* renderTarget) noexcept { diff --git a/libs/filamentapp/include/filamentapp/Cube.h b/libs/filamentapp/include/filamentapp/Cube.h index 778c645fd0..53a8ef36d1 100644 --- a/libs/filamentapp/include/filamentapp/Cube.h +++ b/libs/filamentapp/include/filamentapp/Cube.h @@ -28,8 +28,14 @@ class Cube { public: + Cube(filament::Engine& engine, filament::Material const* material, filament::math::float3 linearColor, bool culling = true); + Cube(Cube const&) = delete; + Cube& operator=(Cube const&) = delete; + + Cube(Cube&& rhs) noexcept; + utils::Entity getSolidRenderable() { return mSolidRenderable; } @@ -55,8 +61,8 @@ private: filament::Material const* mMaterial = nullptr; filament::MaterialInstance* mMaterialInstanceSolid = nullptr; filament::MaterialInstance* mMaterialInstanceWireFrame = nullptr; - utils::Entity mSolidRenderable; - utils::Entity mWireFrameRenderable; + utils::Entity mSolidRenderable{}; + utils::Entity mWireFrameRenderable{}; }; diff --git a/libs/filamentapp/include/filamentapp/FilamentApp.h b/libs/filamentapp/include/filamentapp/FilamentApp.h index d7087d0f0b..e50f312cd3 100644 --- a/libs/filamentapp/include/filamentapp/FilamentApp.h +++ b/libs/filamentapp/include/filamentapp/FilamentApp.h @@ -211,11 +211,11 @@ private: filament::Camera* mOrthoCamera; std::vector> mViews; - CView* mMainView; - CView* mUiView; + CView* mMainView; // well, the main view + CView* mUiView; // the imgui ui CView* mDepthView; - GodView* mGodView; - CView* mOrthoView; + GodView* mGodView; // the debug view with "god" camera + CView* mOrthoView; // directional shadow map view size_t mWidth = 0; size_t mHeight = 0; diff --git a/libs/filamentapp/src/Cube.cpp b/libs/filamentapp/src/Cube.cpp index cd2faa0bc0..a8027b6953 100644 --- a/libs/filamentapp/src/Cube.cpp +++ b/libs/filamentapp/src/Cube.cpp @@ -104,11 +104,23 @@ Cube::Cube(Engine& engine, filament::Material const* material, float3 linearColo .build(engine, mWireFrameRenderable); } +Cube::Cube(Cube&& rhs) noexcept + : mEngine(rhs.mEngine) { + using std::swap; + swap(rhs.mVertexBuffer, mVertexBuffer); + swap(rhs.mIndexBuffer, mIndexBuffer); + swap(rhs.mMaterial, mMaterial); + swap(rhs.mMaterialInstanceSolid, mMaterialInstanceSolid); + swap(rhs.mMaterialInstanceWireFrame, mMaterialInstanceWireFrame); + swap(rhs.mSolidRenderable, mSolidRenderable); + swap(rhs.mWireFrameRenderable, mWireFrameRenderable); +} + void Cube::mapFrustum(filament::Engine& engine, Camera const* camera) { // the Camera far plane is at infinity, but we want it closer for display const mat4 vm(camera->getModelMatrix()); mat4 p(vm * inverse(camera->getCullingProjectionMatrix())); - return mapFrustum(engine, p); + mapFrustum(engine, p); } void Cube::mapFrustum(filament::Engine& engine, filament::math::mat4 const& transform) { @@ -122,7 +134,7 @@ void Cube::mapFrustum(filament::Engine& engine, filament::math::mat4 const& tran void Cube::mapAabb(filament::Engine& engine, filament::Box const& box) { mat4 p = mat4::translation(box.center) * mat4::scaling(box.halfExtent); - return mapFrustum(engine, p); + mapFrustum(engine, p); } Cube::~Cube() { diff --git a/libs/filamentapp/src/FilamentApp.cpp b/libs/filamentapp/src/FilamentApp.cpp index 8ffc4c0f34..d4e4b44ac3 100644 --- a/libs/filamentapp/src/FilamentApp.cpp +++ b/libs/filamentapp/src/FilamentApp.cpp @@ -1,4 +1,3 @@ -#include /* * Copyright (C) 2015 The Android Open Source Project @@ -56,6 +55,13 @@ #include +#include +#include +#include +#include + +#include + #include "generated/resources/filamentapp.h" using namespace filament; @@ -138,10 +144,17 @@ void FilamentApp::run(const Config& config, SetupCallback setupCallback, .package(FILAMENTAPP_TRANSPARENTCOLOR_DATA, FILAMENTAPP_TRANSPARENTCOLOR_SIZE) .build(*mEngine); - std::unique_ptr cameraCube(new Cube(*mEngine, mTransparentMaterial, {1,0,0})); + std::unique_ptr cameraCube{ new Cube(*mEngine, mTransparentMaterial, { 1, 0, 0 }) }; + // we can't cull the light-frustum because it's not applied a rigid transform // and currently, filament assumes that for culling - std::unique_ptr lightmapCube(new Cube(*mEngine, mTransparentMaterial, {0,1,0}, false)); + std::vector lightmapCubes; + lightmapCubes.reserve(4); + lightmapCubes.emplace_back(*mEngine, mTransparentMaterial, float3{ 0, 1, 0 }, false); + lightmapCubes.emplace_back(*mEngine, mTransparentMaterial, float3{ 0, 0, 1 }, false); + lightmapCubes.emplace_back(*mEngine, mTransparentMaterial, float3{ 1, 1, 0 }, false); + lightmapCubes.emplace_back(*mEngine, mTransparentMaterial, float3{ 1, 0, 0 }, false); + mScene = mEngine->createScene(); window->mMainView->getView()->setVisibleLayers(0x4, 0x4); @@ -151,16 +164,15 @@ void FilamentApp::run(const Config& config, SetupCallback setupCallback, rcm.setLayerMask(rcm.getInstance(cameraCube->getSolidRenderable()), 0x3, 0x2); rcm.setLayerMask(rcm.getInstance(cameraCube->getWireFrameRenderable()), 0x3, 0x2); - - rcm.setLayerMask(rcm.getInstance(lightmapCube->getSolidRenderable()), 0x3, 0x2); - rcm.setLayerMask(rcm.getInstance(lightmapCube->getWireFrameRenderable()), 0x3, 0x2); - - // Create the camera mesh mScene->addEntity(cameraCube->getWireFrameRenderable()); mScene->addEntity(cameraCube->getSolidRenderable()); - mScene->addEntity(lightmapCube->getWireFrameRenderable()); - mScene->addEntity(lightmapCube->getSolidRenderable()); + for (auto&& cube : lightmapCubes) { + rcm.setLayerMask(rcm.getInstance(cube.getSolidRenderable()), 0x3, 0x2); + rcm.setLayerMask(rcm.getInstance(cube.getWireFrameRenderable()), 0x3, 0x2); + mScene->addEntity(cube.getWireFrameRenderable()); + mScene->addEntity(cube.getSolidRenderable()); + } window->mDepthView->getView()->setVisibleLayers(0x4, 0x4); window->mGodView->getView()->setVisibleLayers(0x6, 0x6); @@ -425,10 +437,26 @@ void FilamentApp::run(const Config& config, SetupCallback setupCallback, window->mDebugCamera->lookAt(eye, center, up); // Update the cube distortion matrix used for frustum visualization. - const Camera* lightmapCamera = window->mMainView->getView()->getDirectionalShadowCamera(); - if (lightmapCamera) { - lightmapCube->mapFrustum(*mEngine, lightmapCamera); + auto& rcm = mEngine->getRenderableManager(); + auto const csm = window->mMainView->getView()->getDirectionalShadowCameras(); + // show/hide the cascades + for (size_t i = 0 ; i < 4; i++) { + rcm.setLayerMask(rcm.getInstance(lightmapCubes[i].getSolidRenderable()), 0x3, 0x0); + rcm.setLayerMask(rcm.getInstance(lightmapCubes[i].getWireFrameRenderable()), 0x3, 0x0); } + if (!csm.empty()) { + for (size_t i = 0, c = csm.size(); i < c; i++) { + if (csm[i]) { + lightmapCubes[i].mapFrustum(*mEngine, csm[i]); + } + uint8_t const layer = csm[i] ? 0x2 : 0x0; + rcm.setLayerMask(rcm.getInstance(lightmapCubes[i].getSolidRenderable()), + 0x3, layer); + rcm.setLayerMask(rcm.getInstance(lightmapCubes[i].getWireFrameRenderable()), + 0x3, layer); + } + } + cameraCube->mapFrustum(*mEngine, window->mMainCamera); // Delay rendering for roughly one monitor refresh interval @@ -452,11 +480,14 @@ void FilamentApp::run(const Config& config, SetupCallback setupCallback, if (config.splitView) { if(!window->mOrthoView->getView()->hasCamera()) { - Camera const* debugDirectionalShadowCamera = - window->mMainView->getView()->getDirectionalShadowCamera(); - if (debugDirectionalShadowCamera) { - window->mOrthoView->setCamera( - const_cast(debugDirectionalShadowCamera)); + auto const csm = window->mMainView->getView()->getDirectionalShadowCameras(); + if (!csm.empty()) { + // here we could choose the cascade + Camera const* debugDirectionalShadowCamera = csm[0]; + if (debugDirectionalShadowCamera) { + window->mOrthoView->setCamera( + const_cast(debugDirectionalShadowCamera)); + } } } } @@ -484,7 +515,7 @@ void FilamentApp::run(const Config& config, SetupCallback setupCallback, cleanupCallback(mEngine, window->mMainView->getView(), mScene); cameraCube.reset(); - lightmapCube.reset(); + lightmapCubes.clear(); window.reset(); mIBL.reset(); @@ -736,12 +767,6 @@ FilamentApp::Window::Window(FilamentApp* filamentApp, mGodView->setCamera(mMainCamera); mGodView->setGodCamera(mDebugCamera); mGodView->setCameraManipulator(mDebugCameraMan); - - // Ortho view obviously uses an ortho camera - Camera const* debugDirectionalShadowCamera = mMainView->getView()->getDirectionalShadowCamera(); - if (debugDirectionalShadowCamera) { - mOrthoView->setCamera(const_cast(debugDirectionalShadowCamera)); - } } // configure the cameras From 6c0bd360b309a46a603668531e85baa845000826 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 22 Nov 2022 12:58:32 -0800 Subject: [PATCH 09/13] 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 | 10 ++++---- 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 | 6 +++++ .../src/vulkan/VulkanPipelineCache.cpp | 1 + .../backend/src/vulkan/VulkanPipelineCache.h | 4 +++- .../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 | 20 ++++++++++++---- filament/src/ShadowMapManager.h | 1 + filament/src/details/DebugRegistry.cpp | 24 +++++++------------ filament/src/details/Engine.h | 1 + samples/gltf_viewer.cpp | 2 ++ 23 files changed, 104 insertions(+), 36 deletions(-) diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index 8da0cfb02f..35cd89a57e 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 b92a07f37b..d27cea8f4c 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -305,6 +305,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 f771ca5b41..fc8cfc12b4 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 a9a0df24ed..397a159933 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -834,6 +834,10 @@ bool MetalDriver::isProtectedTexturesSupported() { return false; } +bool MetalDriver::isDepthClampSupported() { + return true; +} + bool MetalDriver::isWorkaroundNeeded(Workaround workaround) { switch (workaround) { case Workaround::SPLIT_EASU: @@ -1751,6 +1755,13 @@ void MetalDriver::bindPipeline(PipelineState const& ps) { [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 83579cb5d5..1d541cc006 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 ea5c526402..a692cfce83 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 423f8865cc..9654545825 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -679,6 +679,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); @@ -749,6 +750,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 a4a43e8fe0..3b4724b095 100644 --- a/filament/backend/src/opengl/OpenGLContext.h +++ b/filament/backend/src/opengl/OpenGLContext.h @@ -220,8 +220,9 @@ public: 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 @@ public: bool KHR_parallel_shader_compile; bool KHR_texture_compression_astc_hdr; bool KHR_texture_compression_astc_ldr; - bool OES_depth_texture; - bool OES_depth24; - bool OES_packed_depth_stencil; bool OES_EGL_image_external_essl3; + bool OES_depth24; + bool OES_depth_texture; + bool OES_packed_depth_stencil; bool OES_rgb8_rgba8; bool OES_standard_derivatives; bool OES_texture_npot; @@ -636,6 +637,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 274e7b71d3..fdf567849e 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 569c8ed285..04ad63ff6a 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 9f506e03f7..46fe475612 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/filament/backend/src/vulkan/VulkanContext.h @@ -125,6 +125,10 @@ public: 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 bf5604a0df..4602067663 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,8 @@ void VulkanDriver::bindPipeline(PipelineState const& pipelineState) { .dstAlphaBlendFactor = getBlendFactor(rasterState.blendFunctionDstAlpha), .colorWriteMask = (VkColorComponentFlags) (rasterState.colorWrite ? 0xf : 0x0), .rasterizationSamples = rt->getSamples(), + .depthClamp = rasterState.depthClamp, + .reserved = 0, .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 4c77525bd1..b3d09da86e 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 e6816497c0..985574b74c 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.h +++ b/filament/backend/src/vulkan/VulkanPipelineCache.h @@ -90,7 +90,9 @@ public: 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 : 1; + uint8_t reserved : 3; 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 b4152807cc..2e88542080 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 3dc95fa4f8..d9f4a74dc4 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 e729d67dd7..3cad6b5603 100644 --- a/filament/src/RenderPass.h +++ b/filament/src/RenderPass.h @@ -284,6 +284,7 @@ public: 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 @@ private: 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 4c40668054..bbf008e4a0 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 a6f4a9fa3c..1f3c902be5 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,16 @@ FrameGraphId ShadowMapManager::render(FEngine& engine, FrameG // generate and sort the commands for rendering the shadow map + RenderPass::RenderFlags renderPassFlags{}; + if (view.isFrontFaceWindingInverted()) { + renderPassFlags |= RenderPass::HAS_INVERSE_FRONT_FACES; + } + if (mIsDepthClampSupported && engine.debug.shadowmap.depth_clamp) { + renderPassFlags |= RenderPass::HAS_DEPTH_CLAMP; + } + RenderPass const pass = passBuilder + .renderFlags(renderPassFlags) .camera(cameraInfo) .visibilityMask(entry.visibilityMask) .geometry(scene->getRenderableData(), @@ -642,7 +651,8 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng updateNearFarPlanes(&cameraInfo.cullingProjection, cameraInfo.zn, cameraInfo.zf); auto shaderParameters = shadowMap.updateDirectional(engine, - lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, USE_DEPTH_CLAMP); + lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, + mIsDepthClampSupported && engine.debug.shadowmap.depth_clamp); if (shadowMap.hasVisibleShadows()) { const size_t shadowIndex = shadowMap.getShadowIndex(); diff --git a/filament/src/ShadowMapManager.h b/filament/src/ShadowMapManager.h index 18b4a24e19..95ad012774 100644 --- a/filament/src/ShadowMapManager.h +++ b/filament/src/ShadowMapManager.h @@ -232,6 +232,7 @@ private: 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/DebugRegistry.cpp b/filament/src/details/DebugRegistry.cpp index c2f5d3d3fb..8915cf312e 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; } diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index d456a7b042..fa96370417 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -600,6 +600,7 @@ public: 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 47d8c84369..b5c8e1b708 100644 --- a/samples/gltf_viewer.cpp +++ b/samples/gltf_viewer.cpp @@ -896,6 +896,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 28ef805e5dbffb31b0a13e62a4b55d3e51476b6d Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 20 Aug 2024 08:55:09 -0700 Subject: [PATCH 10/13] Release Filament 1.54.0 --- 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 66dfda7840..443168d2a6 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.53.5' + implementation 'com.google.android.filament:filament-android:1.54.0' } ``` @@ -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.53.5' +pod 'Filament', '~> 1.54.0' ``` ### Snapshots diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 84193ace37..c0680a5505 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.1 + + ## v1.54.0 - materials: add a new `stereoscopicType` material parameter. [⚠️ **New Material Version**] diff --git a/android/gradle.properties b/android/gradle.properties index a9eb5d0046..f8284a4f77 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.53.5 +VERSION_NAME=1.54.0 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index ad8c4c6add..8ec4c8d8cc 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.53.5" + spec.version = "1.54.0" 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.53.5/filament-v1.53.5-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.54.0/filament-v1.54.0-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 1397ef3231..60f36d09e9 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.53.5", + "version": "1.54.0", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From aa5f36e1e338ad7f48480f8c565bc03bcb7fe504 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 20 Aug 2024 08:55:19 -0700 Subject: [PATCH 11/13] Bump version to 1.54.1 --- 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 443168d2a6..f0e9ca349d 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.54.0' + implementation 'com.google.android.filament:filament-android:1.54.1' } ``` @@ -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.0' +pod 'Filament', '~> 1.54.1' ``` ### Snapshots diff --git a/android/gradle.properties b/android/gradle.properties index f8284a4f77..994ce21404 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.54.0 +VERSION_NAME=1.54.1 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 8ec4c8d8cc..1e87d19b7e 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.0" + spec.version = "1.54.1" 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.0/filament-v1.54.0-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.54.1/filament-v1.54.1-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 60f36d09e9..21ca5f2f83 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.54.0", + "version": "1.54.1", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 40ce15cfbd0a7db7276671c2981fc08103da5630 Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Sat, 24 Aug 2024 09:17:05 -0700 Subject: [PATCH 12/13] Support tagging driver handles with a name (#8038) --- NEW_RELEASE_NOTES.md | 2 + filament/CMakeLists.txt | 1 + .../include/private/backend/DriverAPI.inc | 4 ++ .../include/private/backend/HandleAllocator.h | 38 +++++++++++++++++-- filament/backend/src/HandleAllocator.cpp | 3 ++ filament/backend/src/metal/MetalDriver.mm | 4 ++ filament/backend/src/noop/NoopDriver.cpp | 3 ++ filament/backend/src/opengl/OpenGLDriver.cpp | 4 ++ filament/backend/src/vulkan/VulkanDriver.cpp | 4 ++ .../src/vulkan/VulkanResourceAllocator.h | 4 ++ filament/include/filament/BufferObject.h | 17 ++++++++- filament/include/filament/FilamentAPI.h | 17 +++++++++ filament/include/filament/IndexBuffer.h | 17 ++++++++- filament/include/filament/InstanceBuffer.h | 17 ++++++++- filament/include/filament/MorphTargetBuffer.h | 17 ++++++++- filament/include/filament/SkinningBuffer.h | 17 ++++++++- filament/include/filament/Stream.h | 17 ++++++++- filament/include/filament/Texture.h | 17 ++++++++- filament/include/filament/VertexBuffer.h | 17 ++++++++- filament/src/FilamentBuilder.cpp | 31 +++++++++++++++ filament/src/components/RenderableManager.cpp | 3 ++ filament/src/details/BufferObject.cpp | 5 +++ filament/src/details/IndexBuffer.cpp | 5 +++ filament/src/details/InstanceBuffer.cpp | 3 +- filament/src/details/InstanceBuffer.h | 4 ++ filament/src/details/Material.cpp | 1 + filament/src/details/MaterialInstance.cpp | 2 + filament/src/details/MorphTargetBuffer.cpp | 7 ++++ filament/src/details/SkinningBuffer.cpp | 7 ++++ filament/src/details/Stream.cpp | 6 ++- filament/src/details/Texture.cpp | 3 ++ filament/src/details/VertexBuffer.cpp | 11 +++++- 32 files changed, 294 insertions(+), 14 deletions(-) create mode 100644 filament/src/FilamentBuilder.cpp diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 4a1a9c7fa7..01d998de3c 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,3 +7,5 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut + +- Add a `name` API to Filament objects for debugging handle use-after-free assertions diff --git a/filament/CMakeLists.txt b/filament/CMakeLists.txt index 796acedb7d..18f402ad9e 100644 --- a/filament/CMakeLists.txt +++ b/filament/CMakeLists.txt @@ -61,6 +61,7 @@ set(SRCS src/Engine.cpp src/Exposure.cpp src/Fence.cpp + src/FilamentBuilder.cpp src/FrameInfo.cpp src/FrameSkipper.cpp src/Froxelizer.cpp diff --git a/filament/backend/include/private/backend/DriverAPI.inc b/filament/backend/include/private/backend/DriverAPI.inc index d27cea8f4c..283e3e0e5c 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -162,6 +162,10 @@ DECL_DRIVER_API_0(finish) // reset state tracking, if the driver does any state tracking (e.g. GL) DECL_DRIVER_API_0(resetState) +DECL_DRIVER_API_N(setDebugTag, + backend::HandleBase::HandleId, handleId, + utils::CString, tag) + /* * Creating driver objects * ----------------------- diff --git a/filament/backend/include/private/backend/HandleAllocator.h b/filament/backend/include/private/backend/HandleAllocator.h index 9c31b59487..b61f8e376f 100644 --- a/filament/backend/include/private/backend/HandleAllocator.h +++ b/filament/backend/include/private/backend/HandleAllocator.h @@ -20,11 +20,12 @@ #include #include +#include #include +#include #include #include #include -#include #include @@ -173,8 +174,10 @@ public: uint8_t const age = (tag & HANDLE_AGE_MASK) >> HANDLE_AGE_SHIFT; auto const pNode = static_cast(p); uint8_t const expectedAge = pNode[-1].age; - FILAMENT_CHECK_POSTCONDITION(expectedAge == age) << - "use-after-free of Handle with id=" << handle.getId(); + // getHandleTag() is only called if the check fails. + FILAMENT_CHECK_POSTCONDITION(expectedAge == age) + << "use-after-free of Handle with id=" << handle.getId() + << ", tag=" << getHandleTag(handle.getId()).c_str_safe(); } } @@ -201,6 +204,34 @@ public: return handle_cast(const_cast&>(handle)); } + void associateTagToHandle(HandleBase::HandleId id, utils::CString&& tag) noexcept { + // 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); + + // This line is the costly part. In the future, we could potentially use a custom + // allocator. + mDebugTags[key] = std::move(tag); + } + } + + utils::CString getHandleTag(HandleBase::HandleId id) const noexcept { + if (!isPoolHandle(id)) { + return "(no tag)"; + } + if (auto pos = mDebugTags.find(id); pos != mDebugTags.end()) { + return pos->second; + } + return "(no tag)"; + } + private: template @@ -363,6 +394,7 @@ private: // Below is only used when running out of space in the HandleArena mutable utils::Mutex mLock; tsl::robin_map mOverflowMap; + tsl::robin_map mDebugTags; HandleBase::HandleId mId = 0; bool mUseAfterFreeCheckDisabled = false; }; diff --git a/filament/backend/src/HandleAllocator.cpp b/filament/backend/src/HandleAllocator.cpp index 07d028d9d4..6cdcaa4af4 100644 --- a/filament/backend/src/HandleAllocator.cpp +++ b/filament/backend/src/HandleAllocator.cpp @@ -80,6 +80,9 @@ HandleAllocator::HandleAllocator(const char* name, size_t size, bool disableUseAfterFreeCheck) noexcept : mHandleArena(name, size, disableUseAfterFreeCheck), mUseAfterFreeCheckDisabled(disableUseAfterFreeCheck) { + // Reserve initial space for debug tags. This prevents excessive calls to malloc when the first + // few tags are set. + mDebugTags.reserve(512); } template diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 397a159933..3022e7381b 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -2072,6 +2072,10 @@ void MetalDriver::enumerateBoundBuffers(BufferObjectBinding bindingType, void MetalDriver::resetState(int) { } +void MetalDriver::setDebugTag(HandleBase::HandleId handleId, utils::CString tag) { + mHandleAllocator.associateTagToHandle(handleId, std::move(tag)); +} + void MetalDriver::runAtNextTick(const std::function& fn) noexcept { std::lock_guard const lock(mTickOpsLock); mTickOps.push_back(fn); diff --git a/filament/backend/src/noop/NoopDriver.cpp b/filament/backend/src/noop/NoopDriver.cpp index a692cfce83..096f57c398 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -392,4 +392,7 @@ void NoopDriver::endTimerQuery(Handle tqh) { void NoopDriver::resetState(int) { } +void NoopDriver::setDebugTag(HandleBase::HandleId handleId, utils::CString tag) { +} + } // namespace filament diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index fdf567849e..ed23067bb8 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -2222,6 +2222,10 @@ void OpenGLDriver::makeCurrent(Handle schDraw, Handle // Updating driver objects // ------------------------------------------------------------------------------------------------ +void OpenGLDriver::setDebugTag(HandleBase::HandleId handleId, utils::CString tag) { + mHandleAllocator.associateTagToHandle(handleId, std::move(tag)); +} + void OpenGLDriver::setVertexBufferObject(Handle vbh, uint32_t index, Handle boh) { DEBUG_MARKER() diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 4602067663..0b444c0b75 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -2038,6 +2038,10 @@ void VulkanDriver::debugCommandBegin(CommandStream* cmds, bool synchronous, cons void VulkanDriver::resetState(int) { } +void VulkanDriver::setDebugTag(HandleBase::HandleId handleId, utils::CString tag) { + mResourceAllocator.associateHandle(handleId, std::move(tag)); +} + // explicit instantiation of the Dispatcher template class ConcreteDispatcher; diff --git a/filament/backend/src/vulkan/VulkanResourceAllocator.h b/filament/backend/src/vulkan/VulkanResourceAllocator.h index 04187658b7..572aeed4f7 100644 --- a/filament/backend/src/vulkan/VulkanResourceAllocator.h +++ b/filament/backend/src/vulkan/VulkanResourceAllocator.h @@ -105,6 +105,10 @@ public: mHandleAllocatorImpl.deallocate(handle, obj); } + inline void associateHandle(HandleBase::HandleId id, utils::CString&& tag) noexcept { + mHandleAllocatorImpl.associateTagToHandle(id, std::move(tag)); + } + private: AllocatorImpl mHandleAllocatorImpl; diff --git a/filament/include/filament/BufferObject.h b/filament/include/filament/BufferObject.h index 74a4b1ff39..8c76719c85 100644 --- a/filament/include/filament/BufferObject.h +++ b/filament/include/filament/BufferObject.h @@ -54,7 +54,7 @@ public: using BufferDescriptor = backend::BufferDescriptor; using BindingType = backend::BufferObjectBinding; - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: Builder() noexcept; @@ -78,6 +78,21 @@ public: */ Builder& bindingType(BindingType bindingType) noexcept; + /** + * Associate an optional name with this BufferObject for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this BufferObject + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the BufferObject and returns a pointer to it. After creation, the buffer * object is uninitialized. Use BufferObject::setBuffer() to initialize it. diff --git a/filament/include/filament/FilamentAPI.h b/filament/include/filament/FilamentAPI.h index 19d6ba246a..7a6f16d630 100644 --- a/filament/include/filament/FilamentAPI.h +++ b/filament/include/filament/FilamentAPI.h @@ -19,6 +19,7 @@ #include #include +#include #include @@ -54,6 +55,22 @@ public: template using BuilderBase = utils::PrivateImplementation; +void builderMakeName(utils::CString& outName, const char* name, size_t len) noexcept; + +template +class UTILS_PUBLIC BuilderNameMixin { +public: + Builder& name(const char* name, size_t len) noexcept { + builderMakeName(mName, name, len); + return static_cast(*this); + } + + utils::CString const& getName() const noexcept { return mName; } + +private: + utils::CString mName; +}; + } // namespace filament #endif // TNT_FILAMENT_FILAMENTAPI_H diff --git a/filament/include/filament/IndexBuffer.h b/filament/include/filament/IndexBuffer.h index 35b8a10ef2..ff29e1f44d 100644 --- a/filament/include/filament/IndexBuffer.h +++ b/filament/include/filament/IndexBuffer.h @@ -59,7 +59,7 @@ public: UINT = uint8_t(backend::ElementType::UINT), //!< 32-bit indices }; - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: Builder() noexcept; @@ -83,6 +83,21 @@ public: */ Builder& bufferType(IndexType indexType) noexcept; + /** + * Associate an optional name with this IndexBuffer for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this IndexBuffer + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the IndexBuffer object and returns a pointer to it. After creation, the index * buffer is uninitialized. Use IndexBuffer::setBuffer() to initialize the IndexBuffer. diff --git a/filament/include/filament/InstanceBuffer.h b/filament/include/filament/InstanceBuffer.h index 2135152d88..843c7983bb 100644 --- a/filament/include/filament/InstanceBuffer.h +++ b/filament/include/filament/InstanceBuffer.h @@ -38,7 +38,7 @@ class UTILS_PUBLIC InstanceBuffer : public FilamentAPI { struct BuilderDetails; public: - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: @@ -70,6 +70,21 @@ public: */ Builder& localTransforms(math::mat4f const* UTILS_NULLABLE localTransforms) noexcept; + /** + * Associate an optional name with this InstanceBuffer for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this InstanceBuffer + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the InstanceBuffer object and returns a pointer to it. */ diff --git a/filament/include/filament/MorphTargetBuffer.h b/filament/include/filament/MorphTargetBuffer.h index 655bb8d848..cb77bb79c8 100644 --- a/filament/include/filament/MorphTargetBuffer.h +++ b/filament/include/filament/MorphTargetBuffer.h @@ -39,7 +39,7 @@ class UTILS_PUBLIC MorphTargetBuffer : public FilamentAPI { struct BuilderDetails; public: - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: Builder() noexcept; @@ -63,6 +63,21 @@ public: */ Builder& count(size_t count) noexcept; + /** + * Associate an optional name with this MorphTargetBuffer for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this MorphTargetBuffer + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the MorphTargetBuffer object and returns a pointer to it. * diff --git a/filament/include/filament/SkinningBuffer.h b/filament/include/filament/SkinningBuffer.h index 36ae30ed43..27b13191e5 100644 --- a/filament/include/filament/SkinningBuffer.h +++ b/filament/include/filament/SkinningBuffer.h @@ -39,7 +39,7 @@ class UTILS_PUBLIC SkinningBuffer : public FilamentAPI { struct BuilderDetails; public: - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: Builder() noexcept; @@ -69,6 +69,21 @@ public: */ Builder& initialize(bool initialize = true) noexcept; + /** + * Associate an optional name with this SkinningBuffer for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this SkinningBuffer + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the SkinningBuffer object and returns a pointer to it. * diff --git a/filament/include/filament/Stream.h b/filament/include/filament/Stream.h index 6cafbacc8b..883a8ccab3 100644 --- a/filament/include/filament/Stream.h +++ b/filament/include/filament/Stream.h @@ -94,7 +94,7 @@ public: * * To create a NATIVE stream, call the 
stream
method on the builder. */ - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: Builder() noexcept; @@ -136,6 +136,21 @@ public: */ Builder& height(uint32_t height) noexcept; + /** + * Associate an optional name with this Stream for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this Stream + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the Stream object and returns a pointer to it. * diff --git a/filament/include/filament/Texture.h b/filament/include/filament/Texture.h index 8a27f831c2..b351c42943 100644 --- a/filament/include/filament/Texture.h +++ b/filament/include/filament/Texture.h @@ -112,7 +112,7 @@ public: //! Use Builder to construct a Texture object instance - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: Builder() noexcept; @@ -202,6 +202,21 @@ public: */ Builder& swizzle(Swizzle r, Swizzle g, Swizzle b, Swizzle a) noexcept; + /** + * Associate an optional name with this Texture for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this Texture + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the Texture object and returns a pointer to it. * diff --git a/filament/include/filament/VertexBuffer.h b/filament/include/filament/VertexBuffer.h index fccbd0046f..64106434ab 100644 --- a/filament/include/filament/VertexBuffer.h +++ b/filament/include/filament/VertexBuffer.h @@ -61,7 +61,7 @@ public: using AttributeType = backend::ElementType; using BufferDescriptor = backend::BufferDescriptor; - class Builder : public BuilderBase { + class Builder : public BuilderBase, public BuilderNameMixin { friend struct BuilderDetails; public: Builder() noexcept; @@ -158,6 +158,21 @@ public: */ Builder& advancedSkinning(bool enabled) noexcept; + /** + * Associate an optional name with this VertexBuffer for debugging purposes. + * + * name will show in error messages and should be kept as short as possible. The name is + * truncated to a maximum of 128 characters. + * + * The name string is copied during this method so clients may free its memory after + * the function returns. + * + * @param name A string to identify this VertexBuffer + * @param len Length of name, should be less than or equal to 128 + * @return This Builder, for chaining calls. + */ + // Builder& name(const char* UTILS_NONNULL name, size_t len) noexcept; // inherited + /** * Creates the VertexBuffer object and returns a pointer to it. * diff --git a/filament/src/FilamentBuilder.cpp b/filament/src/FilamentBuilder.cpp new file mode 100644 index 0000000000..08f1cb3231 --- /dev/null +++ b/filament/src/FilamentBuilder.cpp @@ -0,0 +1,31 @@ +/* + * 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 + +namespace filament { + +void builderMakeName(utils::CString& outName, const char* name, size_t len) noexcept { + if (!name) { + return; + } + size_t const length = std::min(len, size_t { 128u }); + outName = utils::CString(name, length); +} + +} // namespace filament diff --git a/filament/src/components/RenderableManager.cpp b/filament/src/components/RenderableManager.cpp index 6338d53b72..7b79477bcd 100644 --- a/filament/src/components/RenderableManager.cpp +++ b/filament/src/components/RenderableManager.cpp @@ -588,6 +588,9 @@ void FRenderableManager::create( // full size of the UBO. instances.handle = driver.createBufferObject(sizeof(PerRenderableUib), BufferObjectBinding::UNIFORM, backend::BufferUsage::DYNAMIC); + if (auto name = instances.buffer->getName(); !name.empty()) { + driver.setDebugTag(instances.handle.getId(), std::move(name)); + } } const uint32_t boneCount = builder->mSkinningBoneCount; diff --git a/filament/src/details/BufferObject.cpp b/filament/src/details/BufferObject.cpp index 04a68223ba..a462e42a42 100644 --- a/filament/src/details/BufferObject.cpp +++ b/filament/src/details/BufferObject.cpp @@ -20,6 +20,8 @@ #include "FilamentAPI-impl.h" +#include + namespace filament { struct BufferObject::BuilderDetails { @@ -56,6 +58,9 @@ FBufferObject::FBufferObject(FEngine& engine, const BufferObject::Builder& build FEngine::DriverApi& driver = engine.getDriverApi(); mHandle = driver.createBufferObject(builder->mByteCount, builder->mBindingType, backend::BufferUsage::STATIC); + if (auto name = builder.getName(); !name.empty()) { + driver.setDebugTag(mHandle.getId(), std::move(name)); + } } void FBufferObject::terminate(FEngine& engine) { diff --git a/filament/src/details/IndexBuffer.cpp b/filament/src/details/IndexBuffer.cpp index 66fc5d2f58..ae881de358 100644 --- a/filament/src/details/IndexBuffer.cpp +++ b/filament/src/details/IndexBuffer.cpp @@ -20,6 +20,8 @@ #include "FilamentAPI-impl.h" +#include + namespace filament { struct IndexBuffer::BuilderDetails { @@ -58,6 +60,9 @@ FIndexBuffer::FIndexBuffer(FEngine& engine, const IndexBuffer::Builder& builder) (backend::ElementType)builder->mIndexType, uint32_t(builder->mIndexCount), backend::BufferUsage::STATIC); + if (auto name = builder.getName(); !name.empty()) { + driver.setDebugTag(mHandle.getId(), std::move(name)); + } } void FIndexBuffer::terminate(FEngine& engine) { diff --git a/filament/src/details/InstanceBuffer.cpp b/filament/src/details/InstanceBuffer.cpp index af22554d0f..89fdd0e5d9 100644 --- a/filament/src/details/InstanceBuffer.cpp +++ b/filament/src/details/InstanceBuffer.cpp @@ -61,7 +61,8 @@ InstanceBuffer* InstanceBuffer::Builder::build(Engine& engine) { // ------------------------------------------------------------------------------------------------ -FInstanceBuffer::FInstanceBuffer(FEngine& engine, const Builder& builder) { +FInstanceBuffer::FInstanceBuffer(FEngine& engine, const Builder& builder) + : mName(builder.getName()) { mInstanceCount = builder->mInstanceCount; mLocalTransforms.reserve(mInstanceCount); diff --git a/filament/src/details/InstanceBuffer.h b/filament/src/details/InstanceBuffer.h index 2d25f7833e..56bac500c2 100644 --- a/filament/src/details/InstanceBuffer.h +++ b/filament/src/details/InstanceBuffer.h @@ -25,6 +25,7 @@ #include +#include #include namespace filament { @@ -46,10 +47,13 @@ public: void prepare(FEngine& engine, math::mat4f rootTransform, const PerRenderableData& ubo, backend::Handle handle); + utils::CString const& getName() const noexcept { return mName; } + private: friend class RenderableManager; utils::FixedCapacityVector mLocalTransforms; + utils::CString mName; size_t mInstanceCount; }; diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index 69c1e529ff..4fe186cd29 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -610,6 +610,7 @@ Program FMaterial::getProgramWithVariants( void FMaterial::createAndCacheProgram(Program&& p, Variant variant) const noexcept { auto program = mEngine.getDriverApi().createProgram(std::move(p)); + mEngine.getDriverApi().setDebugTag(program.getId(), mName); assert_invariant(program); mCachedPrograms[variant.key] = program; } diff --git a/filament/src/details/MaterialInstance.cpp b/filament/src/details/MaterialInstance.cpp index 4595c7e6c3..152ecd480a 100644 --- a/filament/src/details/MaterialInstance.cpp +++ b/filament/src/details/MaterialInstance.cpp @@ -51,6 +51,7 @@ FMaterialInstance::FMaterialInstance(FEngine& engine, FMaterial const* material) mUniforms = UniformBuffer(material->getUniformInterfaceBlock().getSize()); mUbHandle = driver.createBufferObject(mUniforms.getSize(), BufferObjectBinding::UNIFORM, backend::BufferUsage::STATIC); + driver.setDebugTag(mUbHandle.getId(), material->getName()); } if (!material->getSamplerInterfaceBlock().isEmpty()) { @@ -116,6 +117,7 @@ FMaterialInstance::FMaterialInstance(FEngine& engine, mUniforms.setUniforms(other->getUniformBuffer()); mUbHandle = driver.createBufferObject(mUniforms.getSize(), BufferObjectBinding::UNIFORM, backend::BufferUsage::DYNAMIC); + driver.setDebugTag(mUbHandle.getId(), material->getName()); } if (!material->getSamplerInterfaceBlock().isEmpty()) { diff --git a/filament/src/details/MorphTargetBuffer.cpp b/filament/src/details/MorphTargetBuffer.cpp index d4124fb2a6..d3323b92c3 100644 --- a/filament/src/details/MorphTargetBuffer.cpp +++ b/filament/src/details/MorphTargetBuffer.cpp @@ -25,6 +25,8 @@ #include #include +#include + namespace filament { using namespace backend; @@ -124,6 +126,11 @@ FMorphTargetBuffer::FMorphTargetBuffer(FEngine& engine, const Builder& builder) mCount, TextureUsage::DEFAULT); + if (auto name = builder.getName(); !name.empty()) { + driver.setDebugTag(mPbHandle.getId(), name); + driver.setDebugTag(mTbHandle.getId(), std::move(name)); + } + // create and update sampler group mSbHandle = driver.createSamplerGroup(PerRenderPrimitiveMorphingSib::SAMPLER_COUNT, utils::FixedSizeString<32>("Morph target samplers")); diff --git a/filament/src/details/SkinningBuffer.cpp b/filament/src/details/SkinningBuffer.cpp index b709263867..c4c79c3a99 100644 --- a/filament/src/details/SkinningBuffer.cpp +++ b/filament/src/details/SkinningBuffer.cpp @@ -27,6 +27,8 @@ #include #include +#include + #include namespace filament { @@ -80,6 +82,11 @@ FSkinningBuffer::FSkinningBuffer(FEngine& engine, const Builder& builder) BufferObjectBinding::UNIFORM, BufferUsage::DYNAMIC); + if (auto name = builder.getName(); !name.empty()) { + // TODO: We should also tag the texture created inside createIndicesAndWeightsHandle. + driver.setDebugTag(mHandle.getId(), std::move(name)); + } + if (builder->mInitialize) { // initialize the bones to identity (before rounding up) auto* out = driver.allocatePod(mBoneCount); diff --git a/filament/src/details/Stream.cpp b/filament/src/details/Stream.cpp index 317113ace0..c60bcf8bef 100644 --- a/filament/src/details/Stream.cpp +++ b/filament/src/details/Stream.cpp @@ -23,10 +23,10 @@ #include +#include #include #include - namespace filament { using namespace backend; @@ -80,6 +80,10 @@ FStream::FStream(FEngine& engine, const Builder& builder) noexcept } else { mStreamHandle = engine.getDriverApi().createStreamAcquired(); } + + if (auto name = builder.getName(); !name.empty()) { + engine.getDriverApi().setDebugTag(mStreamHandle.getId(), std::move(name)); + } } void FStream::terminate(FEngine& engine) noexcept { diff --git a/filament/src/details/Texture.cpp b/filament/src/details/Texture.cpp index 3734df4b6d..a59a24da97 100644 --- a/filament/src/details/Texture.cpp +++ b/filament/src/details/Texture.cpp @@ -243,6 +243,9 @@ FTexture::FTexture(FEngine& engine, const Builder& builder) { mHandle = driver.importTexture(builder->mImportedId, mTarget, mLevelCount, mFormat, mSampleCount, mWidth, mHeight, mDepth, mUsage); } + if (auto name = builder.getName(); !name.empty()) { + driver.setDebugTag(mHandle.getId(), std::move(name)); + } } // frees driver resources, object becomes invalid diff --git a/filament/src/details/VertexBuffer.cpp b/filament/src/details/VertexBuffer.cpp index 6269bc5ed7..8aabcf4fc7 100644 --- a/filament/src/details/VertexBuffer.cpp +++ b/filament/src/details/VertexBuffer.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -259,7 +260,9 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const VertexBuffer::Builder& build mBufferCount, mDeclaredAttributes.count(), mAttributes); mHandle = driver.createVertexBuffer(mVertexCount, mVertexBufferInfoHandle); - + if (auto name = builder.getName(); !name.empty()) { + driver.setDebugTag(mHandle.getId(), name); + } // calculate buffer sizes size_t bufferSizes[MAX_VERTEX_BUFFER_COUNT] = {}; @@ -287,6 +290,9 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const VertexBuffer::Builder& build if (!mBufferObjects[i]) { BufferObjectHandle bo = driver.createBufferObject(bufferSizes[i], backend::BufferObjectBinding::VERTEX, backend::BufferUsage::STATIC); + if (auto name = builder.getName(); !name.empty()) { + driver.setDebugTag(bo.getId(), name); + } driver.setVertexBufferObject(mHandle, i, bo); mBufferObjects[i] = bo; } @@ -303,6 +309,9 @@ FVertexBuffer::FVertexBuffer(FEngine& engine, const VertexBuffer::Builder& build if (!mBufferObjects[i]) { BufferObjectHandle const bo = driver.createBufferObject(bufferSizes[i], backend::BufferObjectBinding::VERTEX, backend::BufferUsage::STATIC); + if (auto name = builder.getName(); !name.empty()) { + driver.setDebugTag(bo.getId(), name); + } driver.setVertexBufferObject(mHandle, i, bo); mBufferObjects[i] = bo; } From eb12e06387e0072b83f3183fe899f83e16348884 Mon Sep 17 00:00:00 2001 From: Sungun Park Date: Mon, 26 Aug 2024 16:30:15 -0700 Subject: [PATCH 13/13] Revert two depth relevant changes (#8083) This reverts commits - b70aa43727a5bdaff16b484b8c4ddba7ab809500 "depth clamp cannot work with VSM" - 6c0bd360b309a46a603668531e85baa845000826 "Add support for depth clamp and use it for shadows" --- .../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 | 6 ----- .../src/vulkan/VulkanPipelineCache.cpp | 1 - .../backend/src/vulkan/VulkanPipelineCache.h | 4 +--- .../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 | 20 ++++------------ filament/src/ShadowMapManager.h | 1 - filament/src/details/DebugRegistry.cpp | 24 ++++++++++++------- filament/src/details/Engine.h | 1 - samples/gltf_viewer.cpp | 2 -- 23 files changed, 35 insertions(+), 103 deletions(-) diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index 35cd89a57e..8da0cfb02f 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 - bool depthClamp : 1; // 32 + uint8_t padding : 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 283e3e0e5c..99ea2d528e 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -309,7 +309,6 @@ 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 fc8cfc12b4..f771ca5b41 100644 --- a/filament/backend/src/metal/MetalContext.h +++ b/filament/backend/src/metal/MetalContext.h @@ -112,7 +112,6 @@ 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 3022e7381b..b7d0a31dea 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -834,10 +834,6 @@ bool MetalDriver::isProtectedTexturesSupported() { return false; } -bool MetalDriver::isDepthClampSupported() { - return true; -} - bool MetalDriver::isWorkaroundNeeded(Workaround workaround) { switch (workaround) { case Workaround::SPLIT_EASU: @@ -1755,13 +1751,6 @@ void MetalDriver::bindPipeline(PipelineState const& ps) { [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 1d541cc006..83579cb5d5 100644 --- a/filament/backend/src/metal/MetalState.h +++ b/filament/backend/src/metal/MetalState.h @@ -382,7 +382,6 @@ 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 096f57c398..adc9c1bc55 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -202,10 +202,6 @@ 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 9654545825..423f8865cc 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -679,7 +679,6 @@ 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); @@ -750,7 +749,6 @@ 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 3b4724b095..a4a43e8fe0 100644 --- a/filament/backend/src/opengl/OpenGLContext.h +++ b/filament/backend/src/opengl/OpenGLContext.h @@ -220,9 +220,8 @@ public: bool EXT_color_buffer_float; bool EXT_color_buffer_half_float; bool EXT_debug_marker; - bool EXT_depth_clamp; - bool EXT_discard_framebuffer; bool EXT_disjoint_timer_query; + bool EXT_discard_framebuffer; bool EXT_multisampled_render_to_texture2; bool EXT_multisampled_render_to_texture; bool EXT_protected_textures; @@ -240,10 +239,10 @@ public: bool KHR_parallel_shader_compile; bool KHR_texture_compression_astc_hdr; bool KHR_texture_compression_astc_ldr; - bool OES_EGL_image_external_essl3; - bool OES_depth24; bool OES_depth_texture; + bool OES_depth24; bool OES_packed_depth_stencil; + bool OES_EGL_image_external_essl3; bool OES_rgb8_rgba8; bool OES_standard_derivatives; bool OES_texture_npot; @@ -637,7 +636,6 @@ 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 ed23067bb8..bb9d2a63a5 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -451,14 +451,6 @@ 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 { @@ -2127,10 +2119,6 @@ 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 04ad63ff6a..569c8ed285 100644 --- a/filament/backend/src/opengl/gl_headers.h +++ b/filament/backend/src/opengl/gl_headers.h @@ -201,12 +201,6 @@ 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 46fe475612..9f506e03f7 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/filament/backend/src/vulkan/VulkanContext.h @@ -125,10 +125,6 @@ public: 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 0b444c0b75..0855102dd9 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -944,10 +944,6 @@ bool VulkanDriver::isProtectedTexturesSupported() { return false; } -bool VulkanDriver::isDepthClampSupported() { - return mContext.isDepthClampSupported(); -} - bool VulkanDriver::isWorkaroundNeeded(Workaround workaround) { switch (workaround) { case Workaround::SPLIT_EASU: { @@ -1820,8 +1816,6 @@ void VulkanDriver::bindPipeline(PipelineState const& pipelineState) { .dstAlphaBlendFactor = getBlendFactor(rasterState.blendFunctionDstAlpha), .colorWriteMask = (VkColorComponentFlags) (rasterState.colorWrite ? 0xf : 0x0), .rasterizationSamples = rt->getSamples(), - .depthClamp = rasterState.depthClamp, - .reserved = 0, .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 b3d09da86e..4c77525bd1 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.cpp +++ b/filament/backend/src/vulkan/VulkanPipelineCache.cpp @@ -184,7 +184,6 @@ 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 985574b74c..e6816497c0 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.h +++ b/filament/backend/src/vulkan/VulkanPipelineCache.h @@ -90,9 +90,7 @@ public: VkBlendFactor srcAlphaBlendFactor : 5; VkBlendFactor dstAlphaBlendFactor : 5; VkColorComponentFlags colorWriteMask : 4; - uint8_t rasterizationSamples : 4;// offset = 4 bytes - uint8_t depthClamp : 1; - uint8_t reserved : 3; + uint8_t rasterizationSamples; // offset = 4 bytes 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 2e88542080..b4152807cc 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp @@ -325,7 +325,6 @@ 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 d9f4a74dc4..3dc95fa4f8 100644 --- a/filament/src/RenderPass.cpp +++ b/filament/src/RenderPass.cpp @@ -419,8 +419,7 @@ 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, bool hasDepthClamp) noexcept { + FMaterialInstance const* const UTILS_RESTRICT mi, bool inverseFrontFaces) noexcept { FMaterial const * const UTILS_RESTRICT ma = mi->getMaterial(); variant = Variant::filterVariant(variant, ma->isVariantLit()); @@ -461,7 +460,6 @@ 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) } @@ -560,9 +558,6 @@ 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(); @@ -582,7 +577,6 @@ 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) { @@ -697,8 +691,7 @@ RenderPass::Command* RenderPass::generateCommandsImpl(RenderPass::CommandTypeFla cmd.info.morphingOffset = primitive.getMorphingBufferOffset(); if constexpr (isColorPass) { - RenderPass::setupColorCommand(cmd, renderableVariant, mi, - inverseFrontFaces, hasDepthClamp); + RenderPass::setupColorCommand(cmd, renderableVariant, mi, inverseFrontFaces); 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 3cad6b5603..e729d67dd7 100644 --- a/filament/src/RenderPass.h +++ b/filament/src/RenderPass.h @@ -284,7 +284,6 @@ public: 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< @@ -445,7 +444,7 @@ private: uint8_t instancedStereoEyeCount) noexcept; static void setupColorCommand(Command& cmdDraw, Variant variant, - FMaterialInstance const* mi, bool inverseFrontFaces, bool hasDepthClamp) noexcept; + FMaterialInstance const* mi, bool inverseFrontFaces) noexcept; static void updateSummedPrimitiveCounts( FScene::RenderableSoa& renderableData, utils::Range vr) noexcept; diff --git a/filament/src/ShadowMap.cpp b/filament/src/ShadowMap.cpp index bbf008e4a0..4c40668054 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 1f3c902be5..a6f4a9fa3c 100644 --- a/filament/src/ShadowMapManager.cpp +++ b/filament/src/ShadowMapManager.cpp @@ -66,15 +66,15 @@ namespace filament { using namespace backend; using namespace math; -ShadowMapManager::ShadowMapManager(FEngine& engine) - : mIsDepthClampSupported(engine.getDriverApi().isDepthClampSupported()) { +// do this only if depth-clamp is available +static constexpr bool USE_DEPTH_CLAMP = false; + +ShadowMapManager::ShadowMapManager(FEngine& engine) { 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,16 +367,7 @@ FrameGraphId ShadowMapManager::render(FEngine& engine, FrameG // generate and sort the commands for rendering the shadow map - RenderPass::RenderFlags renderPassFlags{}; - if (view.isFrontFaceWindingInverted()) { - renderPassFlags |= RenderPass::HAS_INVERSE_FRONT_FACES; - } - if (mIsDepthClampSupported && engine.debug.shadowmap.depth_clamp) { - renderPassFlags |= RenderPass::HAS_DEPTH_CLAMP; - } - RenderPass const pass = passBuilder - .renderFlags(renderPassFlags) .camera(cameraInfo) .visibilityMask(entry.visibilityMask) .geometry(scene->getRenderableData(), @@ -651,8 +642,7 @@ ShadowMapManager::ShadowTechnique ShadowMapManager::updateCascadeShadowMaps(FEng updateNearFarPlanes(&cameraInfo.cullingProjection, cameraInfo.zn, cameraInfo.zf); auto shaderParameters = shadowMap.updateDirectional(engine, - lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, - mIsDepthClampSupported && engine.debug.shadowmap.depth_clamp); + lightData, 0, cameraInfo, shadowMapInfo, sceneInfo, USE_DEPTH_CLAMP); if (shadowMap.hasVisibleShadows()) { const size_t shadowIndex = shadowMap.getShadowIndex(); diff --git a/filament/src/ShadowMapManager.h b/filament/src/ShadowMapManager.h index 95ad012774..18b4a24e19 100644 --- a/filament/src/ShadowMapManager.h +++ b/filament/src/ShadowMapManager.h @@ -232,7 +232,6 @@ private: 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/DebugRegistry.cpp b/filament/src/details/DebugRegistry.cpp index 8915cf312e..c2f5d3d3fb 100644 --- a/filament/src/details/DebugRegistry.cpp +++ b/filament/src/details/DebugRegistry.cpp @@ -28,6 +28,12 @@ #include #include +#ifndef NDEBUG +# define DEBUG_PROPERTIES_WRITABLE true +#else +# define DEBUG_PROPERTIES_WRITABLE false +#endif + using namespace filament::math; using namespace utils; @@ -73,15 +79,17 @@ bool FDebugRegistry::hasProperty(const char* name) const noexcept { template bool FDebugRegistry::setProperty(const char* name, T v) noexcept { - 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(); + 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; } - return true; } return false; } diff --git a/filament/src/details/Engine.h b/filament/src/details/Engine.h index fa96370417..d456a7b042 100644 --- a/filament/src/details/Engine.h +++ b/filament/src/details/Engine.h @@ -600,7 +600,6 @@ public: 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 b5c8e1b708..47d8c84369 100644 --- a/samples/gltf_viewer.cpp +++ b/samples/gltf_viewer.cpp @@ -896,8 +896,6 @@ 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",