From 08d88fdd351c7404a83379f143e1a90659b5ec90 Mon Sep 17 00:00:00 2001 From: hzqst <113660872@qq.com> Date: Mon, 12 Jan 2026 10:19:58 +0800 Subject: [PATCH 1/2] Fix https://github.com/hzqst/DiligentCore/blob/inline_constants_claude/plan/SetInlineConstants_InconsistencyCommit.md for vk --- .../src/PipelineResourceSignatureVkImpl.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp index c0310ed23..0916d4d67 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp @@ -1146,13 +1146,21 @@ void PipelineResourceSignatureVkImpl::CommitInlineConstants(const CommitInlineCo } else { - VERIFY_EXPR(InlineCBAttr.pBuffer); - - // Map the shared buffer and copy the data + // Get the buffer from the SRB cache (not from the signature's InlineCBAttr.pBuffer). + // This ensures we update the same buffer that was bound to the descriptor set during SRB initialization. + // When an SRB created from a compatible but different signature is used (e.g., via PSO serialization), + // the SRB's cache contains buffer pointers from the original signature's shared buffers. + // By updating the buffer from cache, we maintain consistency with D3D11's design. + const ShaderResourceCacheVk::DescriptorSet& DescrSet = ResourceCache.GetDescriptorSet(InlineCBAttr.DescrSet); + const ShaderResourceCacheVk::Resource& CachedRes = DescrSet.GetResource(InlineCBAttr.SRBCacheOffset); + BufferVkImpl* pBuffer = CachedRes.pObject.RawPtr(); + VERIFY(pBuffer != nullptr, "Inline constant buffer is null in SRB cache"); + + // Map the buffer from SRB cache and copy the data void* pMappedData = nullptr; - Attribs.Ctx.MapBuffer(InlineCBAttr.pBuffer, MAP_WRITE, MAP_FLAG_DISCARD, pMappedData); + Attribs.Ctx.MapBuffer(pBuffer, MAP_WRITE, MAP_FLAG_DISCARD, pMappedData); memcpy(pMappedData, pInlineConstantData, DataSize); - Attribs.Ctx.UnmapBuffer(InlineCBAttr.pBuffer, MAP_WRITE); + Attribs.Ctx.UnmapBuffer(pBuffer, MAP_WRITE); } } } From f54157548fbc5ce85aa33992ef294e0662e63062 Mon Sep 17 00:00:00 2001 From: hzqst <113660872@qq.com> Date: Mon, 12 Jan 2026 11:34:06 +0800 Subject: [PATCH 2/2] Add InlineConstants CrossSignatureSRB test. --- .../src/InlineConstantsTest.cpp | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/Tests/DiligentCoreAPITest/src/InlineConstantsTest.cpp b/Tests/DiligentCoreAPITest/src/InlineConstantsTest.cpp index ea976cf0f..12c1fdbac 100644 --- a/Tests/DiligentCoreAPITest/src/InlineConstantsTest.cpp +++ b/Tests/DiligentCoreAPITest/src/InlineConstantsTest.cpp @@ -839,6 +839,180 @@ TEST_F(InlineConstants, TwoResourceSignatures) TestSignatures(2); } +// Test for "Inline Constants Cross-Signature Commit Inconsistency" fix. +// +// This test verifies that inline constants work correctly when an SRB created +// from one signature is used with a PSO created from a different but compatible +// signature instance. +// +// Bug scenario: +// 1. PSO is created with Signature A +// 2. SRB is created from Signature B (compatible but different instance) +// 3. Inline constants are set through SRB +// 4. BindResources() binds buffers from SRB cache (pointing to Signature B's buffers) +// 5. BUG (fixed): UpdateInlineConstantBuffers() was updating Signature A's buffers +// instead of the buffers from SRB cache +// +// The fix ensures that UpdateInlineConstantBuffers() updates the buffer from +// SRB cache - the same buffer that was bound during BindResources(). +// +// In Vulkan, only the first inline constant uses push constants (always works). +// The second inline constant uses buffer-emulated path where this bug manifested. +// Therefore, this test uses TWO inline constant buffers to ensure we test both paths. +TEST_F(InlineConstants, CrossSignatureSRB) +{ + GPUTestingEnvironment* pEnv = GPUTestingEnvironment::GetInstance(); + IRenderDevice* pDevice = pEnv->GetDevice(); + IDeviceContext* pContext = pEnv->GetDeviceContext(); + ISwapChain* pSwapChain = pEnv->GetSwapChain(); + + // Test all variable types to ensure complete coverage + for (Uint32 pos_type = 0; pos_type < SHADER_RESOURCE_VARIABLE_TYPE_NUM_TYPES; ++pos_type) + { + for (Uint32 col_type = 0; col_type < SHADER_RESOURCE_VARIABLE_TYPE_NUM_TYPES; ++col_type) + { + const float ClearColor[] = {sm_Rnd(), sm_Rnd(), sm_Rnd(), sm_Rnd()}; + RenderDrawCommandReference(pSwapChain, ClearColor); + + SHADER_RESOURCE_VARIABLE_TYPE PosType = static_cast(pos_type); + SHADER_RESOURCE_VARIABLE_TYPE ColType = static_cast(col_type); + + // Create TWO IDENTICAL but SEPARATE signature instances + // This is the key to reproducing the cross-signature issue: + // - pSign1 and pSign2 are compatible (same resources) + // - But they are different instances with separate internal buffers + RefCntAutoPtr pSign1, pSign2; + + PipelineResourceSignatureDescX SignDesc{"Cross-Signature Test"}; + SignDesc + // First inline constant (Vulkan: push constants path) + .AddResource(SHADER_TYPE_VERTEX, "cbInlinePositions", kNumPosConstants, + SHADER_RESOURCE_TYPE_CONSTANT_BUFFER, PosType, + PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS) + // Second inline constant (Vulkan: buffer-emulated path - where the bug was) + .AddResource(SHADER_TYPE_VS_PS, "cbInlineColors", kNumColConstants, + SHADER_RESOURCE_TYPE_CONSTANT_BUFFER, ColType, + PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS); + SignDesc.SetSRBAllocationGranularity(4); + + // Create first signature instance + pDevice->CreatePipelineResourceSignature(SignDesc, &pSign1); + ASSERT_TRUE(pSign1); + + // Create second signature instance with identical descriptor + // This creates a separate instance with its own internal buffers + pDevice->CreatePipelineResourceSignature(SignDesc, &pSign2); + ASSERT_TRUE(pSign2); + + // IMPORTANT: pSign1 and pSign2 are compatible but have separate buffer allocations + // Using an SRB from pSign2 with a PSO from pSign1 tests the cross-signature path + + // Create PSO using signature 1 + GraphicsPipelineStateCreateInfoX PsoCI{"Cross-Signature Test"}; + PsoCI + .AddRenderTarget(pSwapChain->GetDesc().ColorBufferFormat) + .SetPrimitiveTopology(PRIMITIVE_TOPOLOGY_TRIANGLE_LIST) + .AddShader(sm_Res.pVS) + .AddShader(sm_Res.pPS) + .AddSignature(pSign1); // PSO uses signature 1 + PsoCI.GraphicsPipeline.DepthStencilDesc.DepthEnable = False; + + RefCntAutoPtr pPSO; + pDevice->CreateGraphicsPipelineState(PsoCI, &pPSO); + ASSERT_TRUE(pPSO); + + // For STATIC variables, set on signature 2 (which will be used for SRB) + if (PosType == SHADER_RESOURCE_VARIABLE_TYPE_STATIC) + { + IShaderResourceVariable* pVar = pSign2->GetStaticVariableByName(SHADER_TYPE_VERTEX, "cbInlinePositions"); + ASSERT_TRUE(pVar); + pVar->SetInlineConstants(g_Positions, 0, kNumPosConstants); + } + + if (ColType == SHADER_RESOURCE_VARIABLE_TYPE_STATIC) + { + IShaderResourceVariable* pVar = pSign2->GetStaticVariableByName(SHADER_TYPE_VERTEX, "cbInlineColors"); + ASSERT_TRUE(pVar); + pVar->SetInlineConstants(g_Colors, 0, kNumColConstants); + } + + // Create SRB from signature 2 (DIFFERENT from PSO's signature!) + // This is the critical part: SRB is from pSign2, PSO is from pSign1 + RefCntAutoPtr pSRB; + pSign2->CreateShaderResourceBinding(&pSRB, true); + ASSERT_TRUE(pSRB); + + // Verify the SRB is compatible with PSO (they should be, since signatures are identical) + // Note: IsCompatibleWith checks signature compatibility + EXPECT_TRUE(pPSO->IsCompatibleWith(pSign2)); + + IShaderResourceVariable* pPosVar = nullptr; + IShaderResourceVariable* pColVarVS = nullptr; + IShaderResourceVariable* pColVarPS = nullptr; + + if (PosType != SHADER_RESOURCE_VARIABLE_TYPE_STATIC) + { + pPosVar = pSRB->GetVariableByName(SHADER_TYPE_VERTEX, "cbInlinePositions"); + ASSERT_TRUE(pPosVar); + } + + if (ColType != SHADER_RESOURCE_VARIABLE_TYPE_STATIC) + { + pColVarVS = pSRB->GetVariableByName(SHADER_TYPE_VERTEX, "cbInlineColors"); + ASSERT_TRUE(pColVarVS); + pColVarPS = pSRB->GetVariableByName(SHADER_TYPE_PIXEL, "cbInlineColors"); + ASSERT_TRUE(pColVarPS); + } + + ITextureView* pRTVs[] = {pSwapChain->GetCurrentBackBufferRTV()}; + pContext->SetRenderTargets(1, pRTVs, nullptr, RESOURCE_STATE_TRANSITION_MODE_TRANSITION); + pContext->ClearRenderTarget(pRTVs[0], ClearColor, RESOURCE_STATE_TRANSITION_MODE_TRANSITION); + + // Set first half of color constants BEFORE CommitShaderResources + if (pColVarVS != nullptr) + { + pColVarVS->SetInlineConstants(g_Colors, 0, kNumColConstants / 2); + } + + pContext->SetPipelineState(pPSO); + pContext->CommitShaderResources(pSRB, RESOURCE_STATE_TRANSITION_MODE_TRANSITION); + + // Set second half of color constants AFTER CommitShaderResources but BEFORE Draw + // This is the specific timing that triggered the original bug: + // - CommitShaderResources binds buffers from SRB cache + // - SetInlineConstants writes to CPU staging + // - Draw triggers UpdateInlineConstantBuffers which must update the CORRECT buffer + if (pColVarPS != nullptr) + { + pColVarPS->SetInlineConstants(g_Colors[0].Data() + kNumColConstants / 2, + kNumColConstants / 2, kNumColConstants / 2); + } + + if (pPosVar == nullptr) + { + // Draw both triangles as positions are static + pContext->Draw({6, DRAW_FLAG_VERIFY_ALL}); + } + else + { + // Draw first triangle + pPosVar->SetInlineConstants(g_Positions, 0, kNumPosConstants / 2); + pContext->Draw({3, DRAW_FLAG_VERIFY_ALL}); + + // Draw second triangle - also tests update after CommitShaderResources + pPosVar->SetInlineConstants(g_Positions[0].Data() + kNumPosConstants / 2, 0, kNumPosConstants / 2); + pContext->Draw({3, DRAW_FLAG_VERIFY_ALL}); + } + + Present(); + + std::cout << TestingEnvironment::GetCurrentTestStatusString() << ' ' + << " Pos " << GetShaderVariableTypeLiteralName(PosType) << ',' + << " Col " << GetShaderVariableTypeLiteralName(ColType) << std::endl; + } + } +} + constexpr Uint32 kCacheContentVersion = 7; RefCntAutoPtr CreateCache(IRenderDevice* pDevice,