From f407e2d15ccc92da68d2ec04ab3e756c01ada64f Mon Sep 17 00:00:00 2001 From: ssdfasd <2156608475@qq.com> Date: Sun, 5 Apr 2026 13:38:50 +0800 Subject: [PATCH] Remove legacy object-id and depth-style binding fallbacks --- .../Rendering/RenderMaterialUtility.h | 39 ---------- .../Passes/BuiltinDepthStylePassBase.cpp | 77 +++++++++++++------ .../Rendering/Passes/BuiltinObjectIdPass.cpp | 9 ++- .../unit/test_builtin_forward_pipeline.cpp | 73 ++++++++++++++---- 4 files changed, 119 insertions(+), 79 deletions(-) diff --git a/engine/include/XCEngine/Rendering/RenderMaterialUtility.h b/engine/include/XCEngine/Rendering/RenderMaterialUtility.h index a24da3d5..75825d24 100644 --- a/engine/include/XCEngine/Rendering/RenderMaterialUtility.h +++ b/engine/include/XCEngine/Rendering/RenderMaterialUtility.h @@ -267,45 +267,6 @@ inline Containers::Array BuildLegacyBuilti return bindings; } -inline Containers::Array BuildLegacyBuiltinObjectIdPassResourceBindings() { - Containers::Array bindings; - bindings.Resize(1); - - bindings[0].name = "PerObjectConstants"; - bindings[0].type = Resources::ShaderResourceType::ConstantBuffer; - bindings[0].set = 0; - bindings[0].binding = 0; - bindings[0].semantic = "PerObject"; - - return bindings; -} - -inline Containers::Array BuildLegacyBuiltinDepthOnlyPassResourceBindings() { - Containers::Array bindings; - bindings.Resize(1); - - bindings[0].name = "PerObjectConstants"; - bindings[0].type = Resources::ShaderResourceType::ConstantBuffer; - bindings[0].set = 0; - bindings[0].binding = 0; - bindings[0].semantic = "PerObject"; - - return bindings; -} - -inline Containers::Array BuildLegacyBuiltinShadowCasterPassResourceBindings() { - Containers::Array bindings; - bindings.Resize(1); - - bindings[0].name = "PerObjectConstants"; - bindings[0].type = Resources::ShaderResourceType::ConstantBuffer; - bindings[0].set = 0; - bindings[0].binding = 0; - bindings[0].semantic = "PerObject"; - - return bindings; -} - inline bool IsBuiltinPassResourceTypeCompatible( BuiltinPassResourceSemantic semantic, Resources::ShaderResourceType type) { diff --git a/engine/src/Rendering/Passes/BuiltinDepthStylePassBase.cpp b/engine/src/Rendering/Passes/BuiltinDepthStylePassBase.cpp index ec91f852..08aa4fdd 100644 --- a/engine/src/Rendering/Passes/BuiltinDepthStylePassBase.cpp +++ b/engine/src/Rendering/Passes/BuiltinDepthStylePassBase.cpp @@ -24,18 +24,6 @@ namespace Passes { namespace { -Containers::Array BuildLegacyBuiltinDepthStylePassResourceBindings( - BuiltinMaterialPass passType) { - switch (passType) { - case BuiltinMaterialPass::DepthOnly: - return BuildLegacyBuiltinDepthOnlyPassResourceBindings(); - case BuiltinMaterialPass::ShadowCaster: - return BuildLegacyBuiltinShadowCasterPassResourceBindings(); - default: - return {}; - } -} - bool IsSupportedPerObjectOnlyBindingPlan(const BuiltinPassResourceBindingPlan& bindingPlan) { return bindingPlan.perObject.IsValid() && bindingPlan.bindings.Size() == 1u && @@ -91,7 +79,7 @@ RHI::GraphicsPipelineDesc CreatePipelineDesc( ApplyMaterialRenderState(material, pipelineDesc); pipelineDesc.blendState.blendEnable = false; - pipelineDesc.blendState.colorWriteMask = 0; + pipelineDesc.blendState.colorWriteMask = pipelineDesc.renderTargetCount > 0 ? 0xF : 0; pipelineDesc.depthStencilState.depthTestEnable = true; pipelineDesc.depthStencilState.depthWriteEnable = true; pipelineDesc.depthStencilState.depthFunc = static_cast(RHI::ComparisonFunc::LessEqual); @@ -101,11 +89,9 @@ RHI::GraphicsPipelineDesc CreatePipelineDesc( shader.FindVariant(passName, Resources::ShaderType::Vertex, backend)) { ::XCEngine::Rendering::Detail::ApplyShaderStageVariant(*vertexVariant, pipelineDesc.vertexShader); } - if (pipelineDesc.renderTargetCount > 0) { - if (const Resources::ShaderStageVariant* fragmentVariant = - shader.FindVariant(passName, Resources::ShaderType::Fragment, backend)) { - ::XCEngine::Rendering::Detail::ApplyShaderStageVariant(*fragmentVariant, pipelineDesc.fragmentShader); - } + if (const Resources::ShaderStageVariant* fragmentVariant = + shader.FindVariant(passName, Resources::ShaderType::Fragment, backend)) { + ::XCEngine::Rendering::Detail::ApplyShaderStageVariant(*fragmentVariant, pipelineDesc.fragmentShader); } return pipelineDesc; @@ -387,12 +373,16 @@ bool BuiltinDepthStylePassBase::TryBuildSupportedBindingPlan( const Resources::ShaderPass& shaderPass, BuiltinPassResourceBindingPlan& outPlan, Containers::String* outError) const { - Containers::Array resourceBindings = shaderPass.resources; - if (resourceBindings.Empty()) { - resourceBindings = BuildLegacyBuiltinDepthStylePassResourceBindings(m_passType); + if (shaderPass.resources.Empty()) { + if (outError != nullptr) { + *outError = + Containers::String("Builtin depth-style pass requires explicit resource bindings on shader pass: ") + + shaderPass.name; + } + return false; } - if (!TryBuildBuiltinPassResourceBindingPlan(resourceBindings, outPlan, outError)) { + if (!TryBuildBuiltinPassResourceBindingPlan(shaderPass.resources, outPlan, outError)) { return false; } @@ -463,7 +453,8 @@ BuiltinDepthStylePassBase::PassResourceLayout* BuiltinDepthStylePassBase::GetOrC return failLayout("Builtin depth-style pass failed to create pipeline layout"); } - const auto result = m_passResourceLayouts.emplace(passLayoutKey, passLayout); + const auto result = m_passResourceLayouts.emplace(passLayoutKey, std::move(passLayout)); + RefreshBuiltinPassSetLayoutMetadata(result.first->second.perObjectSetLayout); return &result.first->second; } @@ -597,18 +588,29 @@ bool BuiltinDepthStylePassBase::DrawVisibleItem( const RenderSceneData& sceneData, const VisibleRenderItem& visibleItem) { if (visibleItem.mesh == nullptr || visibleItem.gameObject == nullptr) { + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + "BuiltinDepthStylePassBase skipped visible item because mesh or game object was null"); return false; } const RenderResourceCache::CachedMesh* cachedMesh = m_resourceCache.GetOrCreateMesh(m_device, visibleItem.mesh); if (cachedMesh == nullptr || cachedMesh->vertexBufferView == nullptr) { + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + (Containers::String("BuiltinDepthStylePassBase failed to cache mesh for ") + + visibleItem.gameObject->GetName().c_str()).CStr()); return false; } const Resources::Material* material = ResolveMaterial(visibleItem); const ResolvedShaderPass resolvedShaderPass = ResolveSurfaceShaderPass(material); if (resolvedShaderPass.shader == nullptr || resolvedShaderPass.pass == nullptr) { + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + (Containers::String("BuiltinDepthStylePassBase could not resolve shader pass for ") + + visibleItem.gameObject->GetName().c_str()).CStr()); return false; } @@ -618,11 +620,20 @@ bool BuiltinDepthStylePassBase::DrawVisibleItem( PassResourceLayout* passLayout = GetOrCreatePassResourceLayout(context, resolvedShaderPass); if (passLayout == nullptr || passLayout->pipelineLayout == nullptr) { + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + (Containers::String("BuiltinDepthStylePassBase failed to create pass layout for ") + + visibleItem.gameObject->GetName().c_str()).CStr()); return false; } RHI::RHIPipelineState* pipelineState = GetOrCreatePipelineState(context, surface, material); if (pipelineState == nullptr) { + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + (Containers::String("BuiltinDepthStylePassBase failed to create pipeline state for ") + + visibleItem.gameObject->GetName().c_str() + + " using pass " + resolvedShaderPass.passName).CStr()); return false; } @@ -642,12 +653,24 @@ bool BuiltinDepthStylePassBase::DrawVisibleItem( *passLayout, visibleItem.gameObject->GetID()); if (constantSet == nullptr) { + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + (Containers::String("BuiltinDepthStylePassBase failed to allocate descriptor set for ") + + visibleItem.gameObject->GetName().c_str()).CStr()); return false; } + const Math::Matrix4x4 projectionMatrix = + m_passType == BuiltinMaterialPass::ShadowCaster + ? sceneData.cameraData.viewProjection + : sceneData.cameraData.projection; + const Math::Matrix4x4 viewMatrix = + m_passType == BuiltinMaterialPass::ShadowCaster + ? Math::Matrix4x4::Identity() + : sceneData.cameraData.view; const PerObjectConstants constants = { - sceneData.cameraData.projection, - sceneData.cameraData.view, + projectionMatrix, + viewMatrix, visibleItem.localToWorld.Transpose() }; constantSet->WriteConstant(passLayout->perObject.binding, &constants, sizeof(constants)); @@ -662,6 +685,10 @@ bool BuiltinDepthStylePassBase::DrawVisibleItem( if (visibleItem.hasSection) { const Containers::Array& sections = visibleItem.mesh->GetSections(); if (visibleItem.sectionIndex >= sections.Size()) { + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + (Containers::String("BuiltinDepthStylePassBase received invalid mesh section for ") + + visibleItem.gameObject->GetName().c_str()).CStr()); return false; } diff --git a/engine/src/Rendering/Passes/BuiltinObjectIdPass.cpp b/engine/src/Rendering/Passes/BuiltinObjectIdPass.cpp index c10f78b6..d0291fc7 100644 --- a/engine/src/Rendering/Passes/BuiltinObjectIdPass.cpp +++ b/engine/src/Rendering/Passes/BuiltinObjectIdPass.cpp @@ -243,9 +243,14 @@ bool BuiltinObjectIdPass::CreateResources(const RenderContext& context) { return false; } - Containers::Array resourceBindings = objectIdPass->resources; + const Containers::Array& resourceBindings = objectIdPass->resources; if (resourceBindings.Empty()) { - resourceBindings = BuildLegacyBuiltinObjectIdPassResourceBindings(); + Debug::Logger::Get().Error( + Debug::LogCategory::Rendering, + (Containers::String("BuiltinObjectIdPass requires explicit resource bindings on shader pass: ") + + objectIdPass->name).CStr()); + DestroyResources(); + return false; } BuiltinPassResourceBindingPlan bindingPlan = {}; diff --git a/tests/Rendering/unit/test_builtin_forward_pipeline.cpp b/tests/Rendering/unit/test_builtin_forward_pipeline.cpp index 25290d4b..bf53e305 100644 --- a/tests/Rendering/unit/test_builtin_forward_pipeline.cpp +++ b/tests/Rendering/unit/test_builtin_forward_pipeline.cpp @@ -300,13 +300,22 @@ TEST(BuiltinObjectIdPass_Test, BuiltinObjectIdShaderDeclaresExplicitPerObjectRes delete shader; } -TEST(BuiltinObjectIdPass_Test, BuildsBuiltinPassResourceBindingPlanFromLegacyFallbackResources) { - const Array bindings = BuildLegacyBuiltinObjectIdPassResourceBindings(); - ASSERT_EQ(bindings.Size(), 1u); +TEST(BuiltinObjectIdPass_Test, BuildsBuiltinPassResourceBindingPlanFromExplicitObjectIdResources) { + ShaderLoader loader; + LoadResult result = loader.Load(GetBuiltinObjectIdShaderPath()); + ASSERT_TRUE(result); + ASSERT_NE(result.resource, nullptr); + + Shader* shader = static_cast(result.resource); + ASSERT_NE(shader, nullptr); + + const ShaderPass* pass = shader->FindPass("ObjectId"); + ASSERT_NE(pass, nullptr); + ASSERT_EQ(pass->resources.Size(), 1u); BuiltinPassResourceBindingPlan plan = {}; String error; - EXPECT_TRUE(TryBuildBuiltinPassResourceBindingPlan(bindings, plan, &error)) << error.CStr(); + EXPECT_TRUE(TryBuildBuiltinPassResourceBindingPlan(pass->resources, plan, &error)) << error.CStr(); ASSERT_EQ(plan.bindings.Size(), 1u); EXPECT_TRUE(plan.perObject.IsValid()); EXPECT_FALSE(plan.material.IsValid()); @@ -317,6 +326,8 @@ TEST(BuiltinObjectIdPass_Test, BuildsBuiltinPassResourceBindingPlanFromLegacyFal EXPECT_TRUE(plan.usesConstantBuffers); EXPECT_FALSE(plan.usesTextures); EXPECT_FALSE(plan.usesSamplers); + + delete shader; } TEST(BuiltinObjectIdPass_Test, UsesFloat3PositionInputLayoutForStaticMeshVertices) { @@ -346,12 +357,22 @@ TEST(BuiltinObjectIdPass_Test, UsesFloat3PositionInputLayoutForStaticMeshVertice EXPECT_EQ(texcoord.alignedByteOffset, static_cast(offsetof(StaticMeshVertex, uv0))); } -TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromLegacyObjectIdResources) { - const Array bindings = BuildLegacyBuiltinObjectIdPassResourceBindings(); +TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromExplicitObjectIdResources) { + ShaderLoader loader; + LoadResult result = loader.Load(GetBuiltinObjectIdShaderPath()); + ASSERT_TRUE(result); + ASSERT_NE(result.resource, nullptr); + + Shader* shader = static_cast(result.resource); + ASSERT_NE(shader, nullptr); + + const ShaderPass* pass = shader->FindPass("ObjectId"); + ASSERT_NE(pass, nullptr); + ASSERT_EQ(pass->resources.Size(), 1u); BuiltinPassResourceBindingPlan plan = {}; String error; - ASSERT_TRUE(TryBuildBuiltinPassResourceBindingPlan(bindings, plan, &error)) << error.CStr(); + ASSERT_TRUE(TryBuildBuiltinPassResourceBindingPlan(pass->resources, plan, &error)) << error.CStr(); std::vector setLayouts; ASSERT_TRUE(TryBuildBuiltinPassSetLayouts(plan, setLayouts, &error)) << error.CStr(); @@ -363,14 +384,26 @@ TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromLegacyObjectIdResources) EXPECT_FALSE(setLayouts[0].usesSampler); EXPECT_FALSE(setLayouts[0].shaderVisible); EXPECT_EQ(setLayouts[0].heapType, DescriptorHeapType::CBV_SRV_UAV); + + delete shader; } -TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromLegacyDepthOnlyResources) { - const Array bindings = BuildLegacyBuiltinDepthOnlyPassResourceBindings(); +TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromExplicitDepthOnlyResources) { + ShaderLoader loader; + LoadResult result = loader.Load(GetBuiltinDepthOnlyShaderPath()); + ASSERT_TRUE(result); + ASSERT_NE(result.resource, nullptr); + + Shader* shader = static_cast(result.resource); + ASSERT_NE(shader, nullptr); + + const ShaderPass* pass = shader->FindPass("DepthOnly"); + ASSERT_NE(pass, nullptr); + ASSERT_EQ(pass->resources.Size(), 1u); BuiltinPassResourceBindingPlan plan = {}; String error; - ASSERT_TRUE(TryBuildBuiltinPassResourceBindingPlan(bindings, plan, &error)) << error.CStr(); + ASSERT_TRUE(TryBuildBuiltinPassResourceBindingPlan(pass->resources, plan, &error)) << error.CStr(); std::vector setLayouts; ASSERT_TRUE(TryBuildBuiltinPassSetLayouts(plan, setLayouts, &error)) << error.CStr(); @@ -380,14 +413,26 @@ TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromLegacyDepthOnlyResources) EXPECT_FALSE(setLayouts[0].usesMaterial); EXPECT_FALSE(setLayouts[0].usesTexture); EXPECT_FALSE(setLayouts[0].usesSampler); + + delete shader; } -TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromLegacyShadowCasterResources) { - const Array bindings = BuildLegacyBuiltinShadowCasterPassResourceBindings(); +TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromExplicitShadowCasterResources) { + ShaderLoader loader; + LoadResult result = loader.Load(GetBuiltinShadowCasterShaderPath()); + ASSERT_TRUE(result); + ASSERT_NE(result.resource, nullptr); + + Shader* shader = static_cast(result.resource); + ASSERT_NE(shader, nullptr); + + const ShaderPass* pass = shader->FindPass("ShadowCaster"); + ASSERT_NE(pass, nullptr); + ASSERT_EQ(pass->resources.Size(), 1u); BuiltinPassResourceBindingPlan plan = {}; String error; - ASSERT_TRUE(TryBuildBuiltinPassResourceBindingPlan(bindings, plan, &error)) << error.CStr(); + ASSERT_TRUE(TryBuildBuiltinPassResourceBindingPlan(pass->resources, plan, &error)) << error.CStr(); std::vector setLayouts; ASSERT_TRUE(TryBuildBuiltinPassSetLayouts(plan, setLayouts, &error)) << error.CStr(); @@ -397,6 +442,8 @@ TEST(BuiltinPassLayout_Test, BuildsSharedSetLayoutsFromLegacyShadowCasterResourc EXPECT_FALSE(setLayouts[0].usesMaterial); EXPECT_FALSE(setLayouts[0].usesTexture); EXPECT_FALSE(setLayouts[0].usesSampler); + + delete shader; } TEST(BuiltinPassLayout_Test, RejectsMixedSamplerAndNonSamplerBindingsInOneSet) {