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.

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

[Bug] Hlsl.bytes no longer produced. #91

Open
thargy opened this issue May 30, 2019 · 3 comments
Open

[Bug] Hlsl.bytes no longer produced. #91

thargy opened this issue May 30, 2019 · 3 comments

Comments

@thargy
Copy link
Contributor

thargy commented May 30, 2019

Upgrading ShaderGen to 1.2.0.beta-3 stops creation of "hlsl.bytes" which I suspect is due to #89, though no warnings or errors are produced. I can confirm from the Build logs that SharpDx.D3DCompiler.dll is being found.

@mellinoe
Copy link
Owner

mellinoe commented Jun 2, 2019

@thargy Sorry about that. It might be caused by the specific version of Windows you are on, or a dependency being missing. Even if SharpDx.D3DCompiler.dll is present, the native library that it calls into might be missing or in the wrong (or unexpected) location. Unfortunately, I don't have a ton of time to spend on a fix for this. Perhaps the simplest solution would be to keep around the old codepath (that #89 replaced) as a fallback?

@thargy
Copy link
Contributor Author

thargy commented Jun 5, 2019

Hi @mellinoe,

cc'ing @Jiuyong as the original dev, in case he has time to fix for you.

Sorry, I don't have time to branch, fix, and test but a code review allowed me to spot the bug, and I've confirmed by noting that although hlsl.bytes are generated in the obj folder, the _sggeneratedfiles.txt does not list them. So I have high confidence in the solution.

The issue is that the original implementation (using FXC) updates the output parameter path on successful compile to the outputPath here. This has the effect of changing the file that is added to the list of generated files from the uncompiled to the compiled file. The new method CompileHlslBySharpDX does not do this, and should do here. This results in the original hlsl file being added here, instead of the newly compiled hlsl.bytes file. Fixing that by updating the path on success will almost certainly fix the issue (and hlsl.bytes files will never be correctly output on builds otherwise).

However, I would also say that this condition should probably add the condition && compilationResult.ResultCode.Code != 0, rather than just checking for null - as you probably shouldn't rely on the bytes if the exit code isn't 0. Also, I'm pretty sure File.OpenWrite returns an IDisposable and so this usage should probably be placed in a using block. I'd make those two changes whilst I was at it.

@thargy
Copy link
Contributor Author

thargy commented Jun 5, 2019

@Jiuyong - here's my proposed changes for lines 381-388:

// Ensure compilation result does not have errors
if (null == compilationResult.Bytecode && !compilationResult.HasErrors && compilationResult.ResultCode != 0)
{
    Console.WriteLine($"Failed to compile HLSL: {compilationResult.Message}.");
}
else
{
	// Ensure stream is correctly disposed.
    using (FileStream fileStream = File.OpenWrite(outputPath))
    {
        compilationResult.Bytecode.Save(fileStream);
    }
	
	// Ensure path is updated to compiled output
    path = outputPath;
}

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

No branches or pull requests

2 participants