[Atom] Honor shader-debug overrides and trim unused shader dependencies - #20106
Open
Grimwarrior wants to merge 5 commits into
Open
Grimwarrior wants to merge 5 commits into
Grimwarrior wants to merge 5 commits into
Conversation
Every ENABLE_ option in LightingOptions.azsli is #ifndef guarded, so a material pipeline can turn one off with a -D on the azslc command line. The twelve forward and transparent pass files define ENABLE_SHADER_DEBUGGING unconditionally, and that exception does not look deliberate: it sits in the same block as FORCE_OPAQUE and ENABLE_CLEAR_COAT, which genuinely are fixed per pass, but the debug views are a developer feature rather than a correctness flag. Unconditional means the command line silently loses and azslc warns about the redefinition, so a pipeline that wants the debug code out has no way to ask for it. Guarding it changes nothing for existing pipelines. None of them define the macro, so the default of 1 still applies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Grimwarrior <143121582+Grimwarrior@users.noreply.github.com>
TransformColor.azsli includes AcesCcToAcesCg.azsli unconditionally, and that pulls in Atom/Features/PostProcessing/Aces.azsli: 808 preprocessed lines of ACES reference transforms. Every shader that calls TransformColor pays for them whether or not ACEScc is ever selected. For Material Canvas that is every graph containing a texture, because sample_texture_2d requests the file for its colour space conversion, and the library is then the single largest contributor to what azslc parses. The two ACEScc conversions are now guarded by ENABLE_ACESCC_COLOR_SPACE, which defaults to 1, so nothing changes unless a pipeline asks. With it off, conversions to or from ACEScc fall through to the same magenta that any other unsupported pair already produces below. That is deliberate: visibly wrong beats quietly approximate. No other colour space is affected, and nothing else in the material shader path reaches Aces.azsli. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Grimwarrior <143121582+Grimwarrior@users.noreply.github.com>
ENABLE_LIGHT_CULLING guards the call sites inside LightCullingTileIterator, so setting it to 0 is widely assumed to remove the light culling code. It does not. LightCullingShared.azsli includes NVLC.azsli unconditionally and the iterator includes that header outside any guard, so about 285 lines survive the option in every forward shader. With culling off, Init() compiles down to InitNoCulling() and the only things still wanted from the header are two sentinel values the iterator compares against. The #else defines those directly and skips the include. Note the include order. Atom/Features/PBR/LightingOptions.azsli has to come first, because it declares ENABLE_LIGHT_CULLING and #if reads an undefined macro as 0. The original order would have dropped light culling from every shader in the engine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Grimwarrior <143121582+Grimwarrior@users.noreply.github.com>
…that force it off The mobile and multi view pipelines define ENABLE_SHADER_DEBUGGING to 0 rather than 1, so the previous commit left them alone: a pipeline wanting the debug views compiled out already had that there. They have the mirror of the same defect. Unconditional means the command line loses either way, so nothing can turn the debug views back on for these pipelines, and asking warns about a redefinition rather than doing anything. Guarding them changes nothing about what is built. The default stays 0, which is what these pipelines have always compiled, and no pipeline in the tree defines the macro on the command line today. With these eight, every definition of ENABLE_SHADER_DEBUGGING in the tree is guarded -- the twelve pipeline files from the previous commit, these eight, and the two in ShaderLib that were already written this way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Grimwarrior <143121582+Grimwarrior@users.noreply.github.com>
ENABLE_ shader options actually respond to -D
EnterTheArcane
self-requested a review
September 13, 2026 18:11
Signed-off-by: EnterTheArcane <96613937+EnterTheArcane@users.noreply.github.com>
ENABLE_ shader options actually respond to -D
EnterTheArcane
approved these changes
Sep 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Atom: honor shader-debug overrides and trim unused shader dependencies
This keeps the existing behavior of every material pipeline while removing preprocessing work that disabled features do not need.
ENABLE_SHADER_DEBUGGINGThe 20 forward and transparent material-pipeline templates defined
ENABLE_SHADER_DEBUGGINGunconditionally. A shader-builder-Dtherefore could not override the pipeline default and produced a macro-redefinition warning.Each definition is now an
#ifndef-guarded default. Main and LowEnd still default to1; Mobile and MultiView still default to0. This exposes the debugging code already supported by each pipeline; it does not add missing debug-output integration to pipelines that do not currently have it.ACEScc conversion dependency
AcesCcToAcesCg.azsliincluded the 802-line post-processingAces.azslilibrary only to read itsHALF_MAXconstant. The converter now uses the same value locally asconst float halfMax = 65504.0f, preserving both ACEScc conversions while removing that unrelated dependency from material shaders.ENABLE_LIGHT_CULLING=0LightCullingTileIterator.azsliincludedLightCullingShared.azsliand NVLC even when light culling was already disabled. The include is now conditional, with the two iterator sentinel values supplied directly in the disabled path.LightingOptions.azsliremains before this condition soENABLE_LIGHT_CULLINGalways has its established default. This change removes unused declarations; it does not make GPU light culling a capability of Mobile or MultiView, whose stock passes intentionally keep it disabled and do not bind the required resources.Compatibility
Verification
ENABLE_SHADER_DEBUGGINGunderGems/are guarded.0and1, plus light-culling0for the 12 Main/LowEnd templates.