Revert two depth relevant changes (#8083)

This reverts commits - b70aa43 "depth clamp cannot work with VSM" - 6c0bd36 "Add support for depth clamp and use it for shadows"
google · Aug 27, 2024 · eb12e06 · eb12e06
1 parent 40ce15c
commit eb12e06
Show file tree

Hide file tree

Showing 23 changed files with 35 additions and 103 deletions.
diff --git 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
@@ -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
@@ -112,7 +112,6 @@ struct MetalContext {
     std::array<BufferState, MAX_SSBO_COUNT> ssboState;
     CullModeStateTracker cullModeState;
     WindingStateTracker windingState;
-    DepthClampStateTracker depthClampState;
     Handle<HwRenderPrimitive> currentRenderPrimitive;
 
     // State caches.

diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm
@@ -834,10 +834,6 @@
     return false;
 }
 
-bool MetalDriver::isDepthClampSupported() {
-    return true;
-}
-
 bool MetalDriver::isWorkaroundNeeded(Workaround workaround) {
     switch (workaround) {
         case Workaround::SPLIT_EASU:
@@ -1755,13 +1751,6 @@
         [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
@@ -382,7 +382,6 @@ using SamplerStateCache = StateCache<SamplerState, id<MTLSamplerState>, SamplerS
 
 using CullModeStateTracker = StateTracker<MTLCullMode>;
 using WindingStateTracker = StateTracker<MTLWinding>;
-using DepthClampStateTracker = StateTracker<MTLDepthClipMode>;
 
 // Argument encoder
 

diff --git 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
@@ -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
@@ -220,9 +220,8 @@ class OpenGLContext final : public TimerQueryFactoryInterface {
         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 @@ class OpenGLContext final : public TimerQueryFactoryInterface {
         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
@@ -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
@@ -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
@@ -125,10 +125,6 @@ struct VulkanContext {
         return mPhysicalDeviceFeatures.imageCubeArray == VK_TRUE;
     }
 
-    inline bool isDepthClampSupported() const noexcept {
-        return mPhysicalDeviceFeatures.depthClamp == VK_TRUE;
-    }
-
     inline bool isDebugMarkersSupported() const noexcept {
         return mDebugMarkersSupported;
     }

diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp
@@ -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
@@ -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
@@ -90,9 +90,7 @@ class VulkanPipelineCache {
         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
@@ -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
@@ -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<FScene::WORLD_AABB_CENTER>();
@@ -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
@@ -284,7 +284,6 @@ class RenderPass {
     static constexpr RenderFlags HAS_SHADOWING             = 0x01;
     static constexpr RenderFlags HAS_INVERSE_FRONT_FACES   = 0x02;
     static constexpr RenderFlags IS_INSTANCED_STEREOSCOPIC = 0x04;
-    static constexpr RenderFlags HAS_DEPTH_CLAMP           = 0x08;
 
     // Arena used for commands
     using Arena = utils::Arena<
@@ -445,7 +444,7 @@ class RenderPass {
             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<uint32_t> vr) noexcept;

diff --git 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
@@ -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<FrameGraphTexture> 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
@@ -232,7 +232,6 @@ class ShadowMapManager {
     ShadowMapCacheContainer mShadowMapCache;
     uint32_t mDirectionalShadowMapCount = 0;
     uint32_t mSpotShadowMapCount = 0;
-    bool const mIsDepthClampSupported;
     bool mInitialized = false;
 
     ShadowMap& getShadowMap(size_t index) noexcept {

diff --git a/filament/src/details/DebugRegistry.cpp b/filament/src/details/DebugRegistry.cpp
@@ -28,6 +28,12 @@
 #include <string_view>
 #include <utility>
 
+#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<typename T>
 bool FDebugRegistry::setProperty(const char* name, T v) noexcept {
-    auto info = getPropertyInfo(name);
-    T* const addr = static_cast<T*>(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<T*>(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
@@ -600,7 +600,6 @@ class FEngine : public Engine {
             bool focus_shadowcasters = true;
             bool visualize_cascades = false;
             bool disable_light_frustum_align = false;
-            bool depth_clamp = true;
             float dzn = -1.0f;
             float dzf =  1.0f;
             float display_shadow_texture_scale = 0.25f;