Skip to content

Shader compiler options to save preprocessed and spirv, optimizer upgrade#1051

Open
Fletterio wants to merge 5 commits intomasterfrom
optimizer-improvement
Open

Shader compiler options to save preprocessed and spirv, optimizer upgrade#1051
Fletterio wants to merge 5 commits intomasterfrom
optimizer-improvement

Conversation

@Fletterio
Copy link
Copy Markdown
Contributor

Gives the shader compiler the option of outputting preprocessed and spirv files to a destination file, useful for debugging. Also when passing an optimizer give the option of replacing the default spirv-opt pass entirely (-O0 flag forwarded to DXC) OR to to run after the default optimization passes. Useful for stripping debug info without disabling optimization, for example.

@devshgraphicsprogramming
Copy link
Copy Markdown
Member

@AnastaZIuk this is your territory, bring this to completion

Comment thread src/nbl/asset/utils/CHLSLCompiler.cpp Outdated
Comment thread include/nbl/asset/utils/IShaderCompiler.h
Comment thread src/nbl/asset/utils/CHLSLCompiler.cpp Outdated
Comment thread src/nbl/asset/utils/CHLSLCompiler.cpp Outdated
@Fletterio
Copy link
Copy Markdown
Contributor Author

Done @AnastaZIuk

Comment on lines +569 to +570
m_system->deleteFile(filePath);
auto renameResult = m_system->moveFileOrDirectory(filePath + "_temp", filePath);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is risky because the temp file is still open here and rename can fail

ECF_SHARE_DELETE would make this more robust on windows but I would still rather drop saveFile before moveFileOrDirectory

saveFile = nullptr; // drop handle first
m_system->deleteFile(filePath); // safe to delete + rename
auto renameResult = m_system->moveFileOrDirectory(filePath + "_temp", filePath);

we already do it for depfiles

@AnastaZIuk
Copy link
Copy Markdown
Member

we also need to resolve conflicts, please pull master to nabla and examples

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.

3 participants