From 5c3676d27e8591c4efb5d797115d0fe8efb3f833 Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 30 Sep 2021 10:29:43 +0100 Subject: [PATCH 1/1] Prevent unused attributes from binding bad location Change-Id: I2655aa8d6e2fb42cc2aed1f06ce91cd05dc37b5d --- dali/internal/render/renderers/pipeline-cache.cpp | 158 ++++++++++------------ 1 file changed, 75 insertions(+), 83 deletions(-) diff --git a/dali/internal/render/renderers/pipeline-cache.cpp b/dali/internal/render/renderers/pipeline-cache.cpp index 4d771b9..9584c12 100644 --- a/dali/internal/render/renderers/pipeline-cache.cpp +++ b/dali/internal/render/renderers/pipeline-cache.cpp @@ -18,10 +18,10 @@ #include // INTERNAL INCLUDES -#include #include #include #include +#include #include #include @@ -34,7 +34,7 @@ Dali::Graphics::VertexInputFormat GetPropertyVertexFormat(Property::Type propert { Dali::Graphics::VertexInputFormat type{}; - switch (propertyType) + switch(propertyType) { case Property::BOOLEAN: { @@ -77,7 +77,7 @@ Dali::Graphics::VertexInputFormat GetPropertyVertexFormat(Property::Type propert constexpr Graphics::CullMode ConvertCullFace(Dali::FaceCullingMode::Type mode) { - switch (mode) + switch(mode) { case Dali::FaceCullingMode::NONE: { @@ -104,7 +104,7 @@ constexpr Graphics::CullMode ConvertCullFace(Dali::FaceCullingMode::Type mode) constexpr Graphics::BlendFactor ConvertBlendFactor(BlendFactor::Type blendFactor) { - switch (blendFactor) + switch(blendFactor) { case BlendFactor::ZERO: return Graphics::BlendFactor::ZERO; @@ -143,7 +143,7 @@ constexpr Graphics::BlendFactor ConvertBlendFactor(BlendFactor::Type blendFactor constexpr Graphics::BlendOp ConvertBlendEquation(DevelBlendEquation::Type blendEquation) { - switch (blendEquation) + switch(blendEquation) { case DevelBlendEquation::ADD: return Graphics::BlendOp::ADD; @@ -172,54 +172,52 @@ constexpr Graphics::BlendOp ConvertBlendEquation(DevelBlendEquation::Type blendE } return Graphics::BlendOp{}; } -} +} // namespace - -PipelineCacheL0 *PipelineCache::GetPipelineCacheL0(Program *program, Render::Geometry *geometry) +PipelineCacheL0* PipelineCache::GetPipelineCacheL0(Program* program, Render::Geometry* geometry) { - auto it = std::find_if(level0nodes.begin(), level0nodes.end(), [program, geometry]( - PipelineCacheL0 &item) - { + auto it = std::find_if(level0nodes.begin(), level0nodes.end(), [program, geometry](PipelineCacheL0& item) { return ((item.program == program && item.geometry == geometry)); }); - std::vector attributeLocations; - // Add new node to cache - if (it == level0nodes.end()) + if(it == level0nodes.end()) { uint32_t bindingIndex{0u}; - auto &reflection = graphicsController->GetProgramReflection(program->GetGraphicsProgram()); + auto& reflection = graphicsController->GetProgramReflection(program->GetGraphicsProgram()); Graphics::VertexInputState vertexInputState{}; uint32_t base = 0; - for (auto &&vertexBuffer : geometry->GetVertexBuffers()) + for(auto&& vertexBuffer : geometry->GetVertexBuffers()) { - const VertexBuffer::Format &vertexFormat = *vertexBuffer->GetFormat(); + const VertexBuffer::Format& vertexFormat = *vertexBuffer->GetFormat(); vertexInputState.bufferBindings.emplace_back(vertexFormat.size, // stride Graphics::VertexInputRate::PER_VERTEX); - const uint32_t attributeCount = vertexBuffer->GetAttributeCount(); - for (uint32_t i = 0; i < attributeCount; ++i) + const uint32_t attributeCount = vertexBuffer->GetAttributeCount(); + uint32_t lastBoundAttributeIndex = 0; + for(uint32_t i = 0; i < attributeCount; ++i) { auto attributeName = vertexBuffer->GetAttributeName(i); int32_t pLocation = reflection.GetVertexAttributeLocation(std::string(attributeName.GetStringView())); - if (-1 == pLocation) + if(-1 != pLocation) + { + auto location = static_cast(pLocation); + vertexInputState.attributes.emplace_back(location, + bindingIndex, + vertexFormat.components[i].offset, + GetPropertyVertexFormat(vertexFormat.components[i].type)); + ++lastBoundAttributeIndex; + } + else { DALI_LOG_WARNING("Attribute not found in the shader: %s\n", attributeName.GetCString()); + // Don't bind unused attributes. } - attributeLocations.emplace_back(pLocation); - - auto location = static_cast(attributeLocations[base + i]); - - vertexInputState.attributes.emplace_back(location, - bindingIndex, - vertexFormat.components[i].offset, - GetPropertyVertexFormat(vertexFormat.components[i].type)); } - base += attributeCount; + base += lastBoundAttributeIndex; ++bindingIndex; } PipelineCacheL0 level0; @@ -233,7 +231,7 @@ PipelineCacheL0 *PipelineCache::GetPipelineCacheL0(Program *program, Render::Geo return &*it; } -PipelineCacheL1 *PipelineCacheL0::GetPipelineCacheL1(Render::Renderer *renderer, bool usingReflection) +PipelineCacheL1* PipelineCacheL0::GetPipelineCacheL1(Render::Renderer* renderer, bool usingReflection) { // hash must be collision free uint32_t hash = 0; @@ -247,47 +245,43 @@ PipelineCacheL1 *PipelineCacheL0::GetPipelineCacheL1(Render::Renderer *renderer, Graphics::PolygonMode::LINE, Graphics::PolygonMode::FILL, Graphics::PolygonMode::FILL, - Graphics::PolygonMode::FILL - }; + Graphics::PolygonMode::FILL}; auto poly = polyTable[topo]; static const FaceCullingMode::Type adjFaceCullingMode[4] = - { - FaceCullingMode::NONE, - FaceCullingMode::BACK, - FaceCullingMode::FRONT, - FaceCullingMode::FRONT_AND_BACK, - }; + { + FaceCullingMode::NONE, + FaceCullingMode::BACK, + FaceCullingMode::FRONT, + FaceCullingMode::FRONT_AND_BACK, + }; static const FaceCullingMode::Type normalFaceCullingMode[4] = - { - FaceCullingMode::NONE, - FaceCullingMode::FRONT, - FaceCullingMode::BACK, - FaceCullingMode::FRONT_AND_BACK, - }; - - static const FaceCullingMode::Type *cullModeTable[2] = { + { + FaceCullingMode::NONE, + FaceCullingMode::FRONT, + FaceCullingMode::BACK, + FaceCullingMode::FRONT_AND_BACK, + }; + + static const FaceCullingMode::Type* cullModeTable[2] = { normalFaceCullingMode, - adjFaceCullingMode - }; + adjFaceCullingMode}; // Retrieve cull mode auto cullModeTableIndex = uint32_t(usingReflection) & 1u; - cull = cullModeTable[cullModeTableIndex][renderer->GetFaceCullMode()]; + cull = cullModeTable[cullModeTableIndex][renderer->GetFaceCullMode()]; hash = (topo & 0xffu) | ((cull << 8u) & 0xff00u) | ((uint32_t(poly) << 16u) & 0xff0000u); // If L1 not found by hash, create rasterization state describing pipeline and store it - auto it = std::find_if(level1nodes.begin(), level1nodes.end(), - [hash](PipelineCacheL1 &item) - { - return item.hashCode == hash; - }); - - PipelineCacheL1 *retval = nullptr; - if (it == level1nodes.end()) + auto it = std::find_if(level1nodes.begin(), level1nodes.end(), [hash](PipelineCacheL1& item) { + return item.hashCode == hash; + }); + + PipelineCacheL1* retval = nullptr; + if(it == level1nodes.end()) { PipelineCacheL1 item; item.hashCode = hash; @@ -305,12 +299,12 @@ PipelineCacheL1 *PipelineCacheL0::GetPipelineCacheL1(Render::Renderer *renderer, return retval; } -PipelineCacheL2 *PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, BlendingOptions &blendingOptions) +PipelineCacheL2* PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, BlendingOptions& blendingOptions) { // early out - if (!blend) + if(!blend) { - if (noBlend.pipeline == nullptr) + if(noBlend.pipeline == nullptr) { // reset all before returning if pipeline has never been created for that case noBlend.hash = 0; @@ -322,20 +316,19 @@ PipelineCacheL2 *PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl auto bitmask = uint32_t(blendingOptions.GetBitmask()); // Find by bitmask (L2 entries must be sorted by bitmask) - auto it = std::find_if(level2nodes.begin(), level2nodes.end(), [bitmask](PipelineCacheL2 &item) - { + auto it = std::find_if(level2nodes.begin(), level2nodes.end(), [bitmask](PipelineCacheL2& item) { return item.hash == bitmask; }); // TODO: find better way of blend constants lookup - PipelineCacheL2 *retval = nullptr; - if (it != level2nodes.end()) + PipelineCacheL2* retval = nullptr; + if(it != level2nodes.end()) { bool hasBlendColor = blendingOptions.GetBlendColor(); while(hasBlendColor && it != level2nodes.end() && (*it).hash == bitmask) { - Vector4 v( it->colorBlendState.blendConstants ); - if( v == *blendingOptions.GetBlendColor() ) + Vector4 v(it->colorBlendState.blendConstants); + if(v == *blendingOptions.GetBlendColor()) { retval = &*it; } @@ -347,18 +340,18 @@ PipelineCacheL2 *PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl } } - if (!retval) + if(!retval) { // create new entry and return it with null pipeline PipelineCacheL2 l2; - l2.pipeline = nullptr; - auto &colorBlendState = l2.colorBlendState; + l2.pipeline = nullptr; + auto& colorBlendState = l2.colorBlendState; colorBlendState.SetBlendEnable(true); Graphics::BlendOp rgbOp = ConvertBlendEquation(blendingOptions.GetBlendEquationRgb()); Graphics::BlendOp alphaOp = ConvertBlendEquation(blendingOptions.GetBlendEquationRgb()); - if (blendingOptions.IsAdvancedBlendEquationApplied() && premul) + if(blendingOptions.IsAdvancedBlendEquationApplied() && premul) { - if (rgbOp != alphaOp) + if(rgbOp != alphaOp) { DALI_LOG_ERROR("Advanced Blend Equation MUST be applied by using BlendEquation.\n"); alphaOp = rgbOp; @@ -374,8 +367,8 @@ PipelineCacheL2 *PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl .SetAlphaBlendOp(alphaOp); // Blend color is optional and rarely used - auto *blendColor = const_cast(blendingOptions.GetBlendColor()); - if (blendColor) + auto* blendColor = const_cast(blendingOptions.GetBlendColor()); + if(blendColor) { colorBlendState.SetBlendConstants(blendColor->AsFloat()); } @@ -383,8 +376,7 @@ PipelineCacheL2 *PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl l2.hash = blendingOptions.GetBitmask(); level2nodes.emplace_back(std::move(l2)); - std::sort(level2nodes.begin(), level2nodes.end(), [](PipelineCacheL2 &lhs, PipelineCacheL2 &rhs) - { + std::sort(level2nodes.begin(), level2nodes.end(), [](PipelineCacheL2& lhs, PipelineCacheL2& rhs) { return lhs.hash < rhs.hash; }); @@ -395,19 +387,19 @@ PipelineCacheL2 *PipelineCacheL1::GetPipelineCacheL2(bool blend, bool premul, Bl return retval; } -PipelineCache::PipelineCache(Graphics::Controller& controller) : -graphicsController(&controller) +PipelineCache::PipelineCache(Graphics::Controller& controller) +: graphicsController(&controller) { } -PipelineResult PipelineCache::GetPipeline(const PipelineCacheQueryInfo &queryInfo, bool createNewIfNotFound) +PipelineResult PipelineCache::GetPipeline(const PipelineCacheQueryInfo& queryInfo, bool createNewIfNotFound) { - auto *level0 = GetPipelineCacheL0(queryInfo.program, queryInfo.geometry); - auto *level1 = level0->GetPipelineCacheL1(queryInfo.renderer, queryInfo.cameraUsingReflection); - auto *level2 = level1->GetPipelineCacheL2(queryInfo.blendingEnabled, queryInfo.alphaPremultiplied, *queryInfo.blendingOptions); + auto* level0 = GetPipelineCacheL0(queryInfo.program, queryInfo.geometry); + auto* level1 = level0->GetPipelineCacheL1(queryInfo.renderer, queryInfo.cameraUsingReflection); + auto* level2 = level1->GetPipelineCacheL2(queryInfo.blendingEnabled, queryInfo.alphaPremultiplied, *queryInfo.blendingOptions); // Create new pipeline at level2 if requested - if (level2->pipeline == nullptr && createNewIfNotFound) + if(level2->pipeline == nullptr && createNewIfNotFound) { Graphics::ProgramState programState{}; programState.program = &queryInfo.program->GetGraphicsProgram(); @@ -438,4 +430,4 @@ PipelineResult PipelineCache::GetPipeline(const PipelineCacheQueryInfo &queryInf return result; } -} \ No newline at end of file +} // namespace Dali::Internal::Render -- 2.7.4