Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New include path behaviour #6669

Closed
KStocky opened this issue Jun 2, 2024 · 0 comments · Fixed by #6789
Closed

New include path behaviour #6669

KStocky opened this issue Jun 2, 2024 · 0 comments · Fixed by #6789
Assignees
@pow2clk
Labels
bug Bug, regression, crash
Milestone
Summer 2024 Release

Comments

@KStocky
Copy link

KStocky commented Jun 2, 2024

Description

Due to #6317 include paths are now passed to IDxcIncludeHandler::LoadSource normalized to the operating system's path style. i.e. On Windows "/" is replaced with "\".

The issue with this is that it likely breaks some implementations of IDxcIncludeHandler because users have been expecting the argument of IDxcIncludeHandler::LoadSource to be exactly what was in the include directive in the shader source. There are not many programmers who would prefer to write "\" over "/" in their header includes, so for some, upgrading to 1.8.2405.17 will have broken their include handler.

Steps to Reproduce

Write a shader that has an include such as #include "Mylibrary/MyHeader.hlsli" and compile it with DXC using a custom IDxcIncludeHandler.

Actual Behavior

Notice that the string passed to IDxcIncludeHandler::LoadSource is ".\\MyLibrary\\Myheader.hlsli" which is different from the string used in the #include directive

Environment

  • DXC version 1.8.2405.17
  • Host Operating System Windows 10 Pro 22H2
@KStocky KStocky added bug Bug, regression, crash needs-triage Awaiting triage labels Jun 2, 2024
@damyanp damyanp removed the needs-triage Awaiting triage label Jun 12, 2024
@damyanp damyanp added this to the Next 2024 Release milestone Jun 12, 2024
@damyanp damyanp assigned pow2clk and unassigned damyanp, llvm-beanz and adam-yang Jul 15, 2024
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Jul 16, 2024
@pow2clk
Release Note for OS-normalized include paths
de8a0e5 
The behavior was changed with microsoft#6317 so that regardless of spelling in
the shader, the include path will conform to the host OS style for the
purposes of the include handler. This just adds a release note for that
new behavior.

Fixes microsoft#6669
@pow2clk pow2clk mentioned this issue Jul 16, 2024
Merged
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Jul 16, 2024
@pow2clk
Release Note for OS-normalized include paths (microsoft#6789)
9e18c18 
The behavior was changed with microsoft#6317 so that regardless of spelling in
the shader, the include path will conform to the host OS style for the
purposes of the include handler. This just adds a release note for that
new behavior.

Fixes microsoft#6669

(cherry picked from commit 9fa4618)
@pow2clk pow2clk mentioned this issue Jul 16, 2024
Merged
pow2clk added a commit that referenced this issue Jul 17, 2024
@pow2clk
Release Note for OS-normalized include paths (#6789) (#6793)
4ed55d3 
The behavior was changed with #6317 so that regardless of spelling in
the shader, the include path will conform to the host OS style for the
purposes of the include handler. This just adds a release note for that
new behavior.

Fixes #6669

(cherry picked from commit 9fa4618)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pow2clk pow2clk

Labels
bug Bug, regression, crash
Projects
HLSL Roadmap
Status: Done
HLSL Triage
Archived in project
Milestone
Summer 2024 Release
Development

Successfully merging a pull request may close this issue.

Release Note for OS-normalized include paths pow2clk/DirectXShaderCompiler
5 participants
@KStocky @damyanp @llvm-beanz @adam-yang @pow2clk