Vulkan MSAA bug

[philcn]

Hi,

I recently discovered a bug in the Vulkan implementation which I first found on Linux, but it should manifest on other platforms as well.

Symptom:

MSAA 4x and 8x makes the render look washed out. RenderDoc shows the 2nd, 5th, 6th and 7th sample of the render target are empty. The resolve then mixes those empty pixels with valid pixels.

Cause:

Because GraphicsStateObject::Desc::getSampleMaks() returns by value, when initVkMultiSampleInfo() function is called, sampleMask parameter is bound to a reference-to-temporary-variable, making the pSampleMask field of vulkan multisample state create info struct point to garbage memory when creating the pipeline.

Proposed fix:

diff --git a/Framework/Source/API/GraphicsStateObject.h b/Framework/Source/API/GraphicsStateObject.h
index 76eff4e..d49f8ef 100644
--- a/Framework/Source/API/GraphicsStateObject.h
+++ b/Framework/Source/API/GraphicsStateObject.h
@@ -79,7 +79,7 @@ namespace Falcor
             BlendState::SharedPtr getBlendState() const { return mpBlendState; }
             RasterizerState::SharedPtr getRasterizerState() const { return mpRasterizerState; }
             DepthStencilState::SharedPtr getDepthStencilState() const { return mpDepthStencilState; }
-            uint32_t getSampleMask() const { return mSampleMask; }
+            const uint32_t& getSampleMask() const { return mSampleMask; }
             GraphicsStateObject::PrimitiveType getPrimitiveType() const { return mPrimType; }
             VertexLayout::SharedConstPtr getVertexLayout() const { return mpLayout; }
             Fbo::Desc getFboDesc() const { return mFboDesc; }

Because of our company policy, making a PR would require a rather long turn-around time. Could you please verify and apply this change?

[nbentyNV]

Thanks. @kyaoNV Can you merge this change and release a new minor version?

[philcn]

Continuing on the MSAA deep dive, I found out that Falcor's support for sample rate shading is probably wrong.

https://github.com/NVIDIAGameWorks/Falcor/blob/master/Framework/Source/Graphics/Program/ProgramReflection.cpp#L797 this line will never be executed. Also, it seems that Slang now correctly reports shader stage frequency for HLSL.