From 76db64e0c8c7dd40ce2a85ef19183c3c245f1dc8 Mon Sep 17 00:00:00 2001 From: William Weaver Date: Wed, 12 Mar 2025 19:52:56 +0300 Subject: Fixes: Add guard to prevent shadow texture resize with invalid mRT dimensions after shader changes; **Replaces forced shader refresh with lightweight guard** MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit introduces a guard in `LLPipeline::resizeShadowTexture()` to prevent shadow texture resizing when the shadow render target (mRT) has invalid (zero) dimensions. **This replaces a previous, less efficient approach of forcing a full shader recompile whenever `RenderShadowResolutionScale` was changed in-session.** **Background and Problem:** Previously, the code forced a full shader recompile whenever `RenderShadowResolutionScale` changed in-session (after toggling advanced graphics settings like SSAO or HDR). While this “sledgehammer” approach did fix broken shadow rendering, it unnecessarily thrashed the shader cache and reset many pipeline states. **Solution:** This commit removes the forced shader recompile in favor of a guard check in `LLPipeline::resizeShadowTexture()`. The guard ensures mRT (the shadow render target) has non-zero dimensions before resizing. If mRT is zero for that frame, the resize operation is skipped, and a warning is logged. Once mRT becomes valid (usually in the next frame), the shadow texture is resized successfully without requiring a full shader refresh. **Detailed changes:** - Reverted the binding of `RenderShadowResolutionScale` to `handleSetShaderChanged`. - Restored the original `handleShadowsResized` listener for `RenderShadowResolutionScale` in `llviewercontrol.cpp`. - Added guard checks in `LLPipeline::resizeShadowTexture()` to skip resizing when `mRT->width` or `mRT->height` is zero. - Added logging statements to track how many frames are skipped. **Benefits:** - Prevents shader thrashing while still avoiding shadow corruption. - Shadows now update correctly as soon as mRT dimensions are valid. - Maintains a detailed record of frames skipped. - **Lightweight and targeted interim solution, much less disruptive than a full shader recompile.** Testing: 1. Reproduce the bug as described in the bug report (toggle SSAO, then change RenderShadowResolutionScale). 2. Verify that shadows are no longer broken after these steps. 3. Check the logs for the warning message indicating skipped frames when the bug is triggered. 4. Confirm that under normal operation (without shader changes causing mRT issues), shadow resizing works as expected without excessive warnings. Documentation: No user-facing documentation changes are needed for this interim fix. However, internal developer documentation should note this guard and the ongoing investigation into the root cause. Further Development: This guard is a temporary fix. The root cause of why mRT becomes invalid after shader changes needs to be investigated and resolved. See the bug report for detailed next steps for investigation. --- indra/newview/pipeline.cpp | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'indra/newview/pipeline.cpp') diff --git a/indra/newview/pipeline.cpp b/indra/newview/pipeline.cpp index 18dd694246..691d155a3c 100644 --- a/indra/newview/pipeline.cpp +++ b/indra/newview/pipeline.cpp @@ -121,7 +121,7 @@ #include "SMAAAreaTex.h" #include "SMAASearchTex.h" - +#include "llerror.h" #ifndef LL_WINDOWS #define A_GCC 1 #pragma GCC diagnostic ignored "-Wunused-function" @@ -727,6 +727,29 @@ void LLPipeline::requestResizeShadowTexture() void LLPipeline::resizeShadowTexture() { + // A static counter to keep track of skipped frames + static int sSkippedFrameCount = 0; + + if (!mRT || mRT->width == 0 || mRT->height == 0) + { + sSkippedFrameCount++; + LL_WARNS("Render") << "Shadow texture resizing aborted: render target dimensions invalid. Skipped " + << sSkippedFrameCount << " frame(s) so far." << LL_ENDL; + return; + } + + // If there were skipped frames before mRT became valid, log that information. + if (sSkippedFrameCount > 0) + { + LL_INFOS("Render") << "Render target now valid after " + << sSkippedFrameCount << " skipped frame(s)." << LL_ENDL; + sSkippedFrameCount = 0; + } + + LL_WARNS() << "LLPipeline::resizeShadowTexture() called." << LL_ENDL; + LL_INFOS() << "Resizing shadow texture. mRT->width = " + << mRT->width << " mRT->height = " << mRT->height << LL_ENDL; + releaseSunShadowTargets(); releaseSpotShadowTargets(); allocateShadowBuffer(mRT->width, mRT->height); -- cgit v1.2.3 From e1ebb33ab2966a20b63741dd84a0826e82b6a807 Mon Sep 17 00:00:00 2001 From: William Weaver Date: Fri, 28 Mar 2025 04:28:36 +0300 Subject: fix(pipeline): Remove incorrect zeroing of mRT dimensions in createGLBuffers Resolves the root cause of shadow rendering failures when changing RenderShadowResolutionScale immediately after modifying other graphics settings (e.g., SSAO, HDR). Investigation revealed that LLPipeline::createGLBuffers, which is called during certain graphics setting changes that require full buffer recreation, contained lines that incorrectly set mRT->width and mRT->height to zero *after* the call to allocateScreenBuffer had already established the correct dimensions. This created a state inconsistency. If RenderShadowResolutionScale was changed immediately following the graphics setting change, the subsequent call to LLPipeline::resizeShadowTexture (triggered via handleShadowsResized) would read these incorrect zero dimensions from mRT. This led to failed shadow buffer allocation (allocateShadowBuffer(0, 0)) and resulted in corrupted or missing shadows. This commit removes the erroneous mRT->width = 0 and mRT->height = 0 lines from the end of createGLBuffers. This ensures that the render target dimensions remain valid after buffer recreation. With this fix, resizeShadowTexture now correctly reads the valid screen dimensions immediately following a graphics setting change and successfully resizes the shadow buffers without delay or error. This eliminates the need for previous workarounds like guard conditions or forced shader recompiles. Ref: #3719 --- indra/newview/pipeline.cpp | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) (limited to 'indra/newview/pipeline.cpp') diff --git a/indra/newview/pipeline.cpp b/indra/newview/pipeline.cpp index 691d155a3c..6adf203ea1 100644 --- a/indra/newview/pipeline.cpp +++ b/indra/newview/pipeline.cpp @@ -727,29 +727,6 @@ void LLPipeline::requestResizeShadowTexture() void LLPipeline::resizeShadowTexture() { - // A static counter to keep track of skipped frames - static int sSkippedFrameCount = 0; - - if (!mRT || mRT->width == 0 || mRT->height == 0) - { - sSkippedFrameCount++; - LL_WARNS("Render") << "Shadow texture resizing aborted: render target dimensions invalid. Skipped " - << sSkippedFrameCount << " frame(s) so far." << LL_ENDL; - return; - } - - // If there were skipped frames before mRT became valid, log that information. - if (sSkippedFrameCount > 0) - { - LL_INFOS("Render") << "Render target now valid after " - << sSkippedFrameCount << " skipped frame(s)." << LL_ENDL; - sSkippedFrameCount = 0; - } - - LL_WARNS() << "LLPipeline::resizeShadowTexture() called." << LL_ENDL; - LL_INFOS() << "Resizing shadow texture. mRT->width = " - << mRT->width << " mRT->height = " << mRT->height << LL_ENDL; - releaseSunShadowTargets(); releaseSpotShadowTargets(); allocateShadowBuffer(mRT->width, mRT->height); @@ -1309,8 +1286,11 @@ void LLPipeline::createGLBuffers() } allocateScreenBuffer(resX, resY); - mRT->width = 0; - mRT->height = 0; + // Do not zero out mRT dimensions here. allocateScreenBuffer() above + // already sets the correct dimensions. Zeroing them caused resizeShadowTexture() + // to fail if called immediately after createGLBuffers (e.g., post graphics change). + // mRT->width = 0; + // mRT->height = 0; if (!mNoiseMap) -- cgit v1.2.3 From cfbcdd713baf1a1027281832f5f41ce5b99fafd7 Mon Sep 17 00:00:00 2001 From: William Weaver Date: Mon, 7 Apr 2025 16:57:12 +0300 Subject: Fix: Remove potentially redundant RenderAutoHideSurfaceAreaLimit setting registration This commit removes a seemingly duplicated `connectRefreshCachedSettingsSafe` call in `LLPipeline::init()` for the `RenderAutoHideSurfaceAreaLimit` setting. A duplicated registration for this setting was identified during a review of `LLPipeline::init()`. Double registration can lead to unexpected behavior, including potential CPU overhead. The duplication *may* have been introduced with commit 440c7b2 (Added CollectFontVertexBuffers feature), though this requires further confirmation. Testing Performed: After removing the duplicate registration, the `RenderAutoHideSurfaceAreaLimit` functionality was validated by ensuring the following behavior (consistent with the existing code): * A value of 0 (zero) causes all objects to appear regardless of size. * Values slightly above zero result in only small objects appearing, with all others hidden. * Increasing the value causes objects of increasing size to appear, while smaller objects remain visible. This change merits careful review to ensure it has no unintended side effects, and to confirm the accuracy of these observations from other developers. --- indra/newview/pipeline.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'indra/newview/pipeline.cpp') diff --git a/indra/newview/pipeline.cpp b/indra/newview/pipeline.cpp index 6adf203ea1..aa82b28bf7 100644 --- a/indra/newview/pipeline.cpp +++ b/indra/newview/pipeline.cpp @@ -599,7 +599,6 @@ void LLPipeline::init() connectRefreshCachedSettingsSafe("RenderMirrors"); connectRefreshCachedSettingsSafe("RenderHeroProbeUpdateRate"); connectRefreshCachedSettingsSafe("RenderHeroProbeConservativeUpdateMultiplier"); - connectRefreshCachedSettingsSafe("RenderAutoHideSurfaceAreaLimit"); LLPointer cntrl_ptr = gSavedSettings.getControl("CollectFontVertexBuffers"); if (cntrl_ptr.notNull()) -- cgit v1.2.3