Sitelet https://github.com/o3de/o3de/pull/20106
Skip to content

[Atom] Honor shader-debug overrides and trim unused shader dependencies - #20106

Open
Grimwarrior wants to merge 5 commits into
o3de:developmentfrom
Grimwarrior:Shader_Option_Guards
Open

Grimwarrior wants to merge 5 commits into
o3de:developmentfrom
Grimwarrior:Shader_Option_Guards

Conversation

@Grimwarrior

@Grimwarrior Grimwarrior commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

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_DEBUGGING

The 20 forward and transparent material-pipeline templates defined ENABLE_SHADER_DEBUGGING unconditionally. A shader-builder -D therefore 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 to 1; Mobile and MultiView still default to 0. 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.azsli included the 802-line post-processing Aces.azsli library only to read its HALF_MAX constant. The converter now uses the same value locally as const float halfMax = 65504.0f, preserving both ACEScc conversions while removing that unrelated dependency from material shaders.

ENABLE_LIGHT_CULLING=0

LightCullingTileIterator.azsli included LightCullingShared.azsli and 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.azsli remains before this condition so ENABLE_LIGHT_CULLING always 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

  • Existing pipeline defaults are unchanged.
  • ACEScc-to-ACEScg and ACEScg-to-ACEScc conversion behavior is unchanged.
  • No pipeline currently supplies these debug overrides through its checked-in shader build arguments.

Verification

  • All 22 definitions of ENABLE_SHADER_DEBUGGING under Gems/ are guarded.
  • Preprocessed and AZSL semantic-compiled 72 configurations across all 20 affected material-pipeline templates: every default, explicit debug 0 and 1, plus light-culling 0 for the 12 Main/LowEnd templates.
  • All 20 default outputs were byte-identical to preprocessing with their existing pipeline value supplied explicitly.
  • Debug code was present in all 20 explicit-on outputs and absent from all 20 explicit-off outputs.
  • NVLC code was absent from every culling-off output and from Mobile/MultiView defaults.
  • ACEScc conversions remained present while post-processing ACES library symbols were absent.

Grimwarrior and others added 4 commits September 9, 2026 04:53
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>
@Grimwarrior
Grimwarrior requested review from a team as code owners September 9, 2026 02:58
@EnterTheArcane EnterTheArcane changed the title Shader option guards [Atom] Make the ENABLE_ shader options actually respond to -D Sep 13, 2026
@EnterTheArcane EnterTheArcane self-assigned this Sep 13, 2026
@EnterTheArcane
EnterTheArcane self-requested a review September 13, 2026 18:11
@EnterTheArcane EnterTheArcane removed their assignment Sep 13, 2026
Signed-off-by: EnterTheArcane <96613937+EnterTheArcane@users.noreply.github.com>
@EnterTheArcane EnterTheArcane changed the title [Atom] Make the ENABLE_ shader options actually respond to -D [Atom] Honor shader-debug overrides and trim unused shader dependencies Sep 13, 2026
@byrcolin byrcolin added the sig/graphics-audio Categorizes an issue or PR as relevant to SIG graphics-audio. label Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/graphics-audio Categorizes an issue or PR as relevant to SIG graphics-audio.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants