Skip to content

Fix conditional directives in GLSL macro arguments - #3750

Merged
paroj merged 1 commit into
OGRECave:masterfrom
snowyukitty:fix/glsl-macro-ifdef
Oct 11, 2026
Merged

paroj merged 1 commit into
OGRECave:masterfrom
snowyukitty:fix/glsl-macro-ifdef

Conversation

@snowyukitty

Copy link
Copy Markdown
Contributor

Closes #3442.

Keep directive operands unexpanded while collecting macro arguments, and preserve their newline boundaries for the existing rescan. This lets #ifdef FOO inside OGRE_UNIFORMS(...) test the macro name even when FOO expands to an empty value, 0, 1, or another macro. Ordinary argument macros still expand.

Count preserved newlines before padding the expansion so following shader statements keep their source line numbers. Stabilize changed borrowed rescan results before advancing to the next pass: forwarded wrappers and rescans that return a slice of the previous buffer must outlive their argument/source storage.

Six regression tests cover #ifdef, #ifndef, #if defined(FOO) and #elif defined(FOO) with direct and forwarded wrappers (80 combinations), ordinary type expansion, declaration order, following source line numbers, and the borrowed-source rescan lifetime. A passing multiline/repeated-argument control protects existing source-line behavior.

Validation on Linux with GCC 12 / CMake 3.25:

  • Test_Ogre: 172 passed, 2 skipped; unchanged baseline: 165 passed, 2 skipped. The six regressions fail on the audited base, while the new multiline control passes.
  • All 17 preprocessor tests pass with AddressSanitizer enabled.
  • glslangValidator -S vert accepts four shaders preprocessed using the real OgreUnifiedShader.h / GLSL_GL3Support.glsl: GLSL 330 and GLES 300, each with FOO defined as 0 or undefined.
  • git diff --check passes.

The native test build enables sample resource configuration and disables tools, language bindings, ImGui and the GLES2 render-system build. Windows/macOS execution and GPU visual tests were not run locally.

Related work: draft #1314 addresses extension-condition passthrough for #614 and shares the preprocessor/test files. Its changes to HandleIf, HandleDirective and Parse are separate from this fix to Macro::Expand, ExpandMacro and GetArgument. At draft head f77b471, its hunks in both shared files apply cleanly to the audited base and this candidate. The complete older draft already needs updates in its other files; compatibility will need rechecking if that draft changes or lands.

@paroj
paroj merged commit 2f1b47d into OGRECave:master Oct 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong expansion order with nested macros

2 participants