Skip to content

Conversation

@ViTeXFTW
Copy link
Owner

@ViTeXFTW ViTeXFTW commented Jan 29, 2026

Greptile Overview

Greptile Summary

Fixed Windows build failure when embedding shaders by replacing command-line argument passing with a file-based approach.

Changes:

  • Writes shader file paths to shader_list.txt instead of passing as escaped command-line argument
  • Updates EmbedShaders.cmake to read from file using file(STRINGS)
  • Adds [[maybe_unused]] attribute to getShader() parameter to suppress compiler warning when no shaders are present

Rationale:
Windows has strict command-line length limits and can mishandle escaped semicolons in long file lists. Writing to an intermediate file is more robust across platforms and scales better with many shader files.

Confidence Score: 5/5

  • This PR is safe to merge with no risk - it's a build system improvement
  • The changes are straightforward build system fixes that improve cross-platform compatibility. The file-based approach is more reliable than command-line arguments and the [[maybe_unused]] attribute is a standard C++ compiler warning suppression
  • No files require special attention

Important Files Changed

Filename Overview
CMakeLists.txt Replaced command-line argument passing with file-based approach for shader list to avoid Windows command-line length limits
cmake/EmbedShaders.cmake Updated to read shader list from file instead of command-line argument, added [[maybe_unused]] attribute to suppress warning

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ViTeXFTW ViTeXFTW merged commit af58e4a into dev Jan 29, 2026
1 check passed
@ViTeXFTW ViTeXFTW deleted the build/fix-shader-build branch January 29, 2026 07:10
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.

2 participants