ShaderGen icon indicating copy to clipboard operation
ShaderGen copied to clipboard

[Bug] Hlsl.bytes no longer produced.

Open thargy opened this issue 5 years ago • 3 comments

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.

thargy avatar May 30 '19 18:05 thargy

@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?

mellinoe avatar Jun 02 '19 10:06 mellinoe

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 avatar Jun 05 '19 15:06 thargy

@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;
}

thargy avatar Jun 05 '19 15:06 thargy