From 2c55cc6b056f93522e36943d7bbaf770a5aa2170 Mon Sep 17 00:00:00 2001 From: "Eunki, Hong" Date: Thu, 8 Dec 2022 18:29:52 +0900 Subject: [PATCH] Fix attribute cache bug 1. It is possible that GL_ACTIVE_ATTRIBUTES value is smaller than location of attributes. If it happend, crashed due to mVertexInputAttributes.insert call. To fix this crash issue, make mVertexInputAttributes size relative with location value, not relative with GL_ACTIVE_ATTRIBUTES. Also, It will guarantee that attribute's location can be index of it's real attrubutes. (Since vector.insert operation shift the index, it was depend on the location input order. and also slow) 2. Ensure the enable of vertex attributes. Since vertexInputState.attributes generated by Geometry in pipeline-cache system, Change the attribute's list of geometry at the same shader was not worked. TODO : Make VAO caching system works well. Currently, VAO doesn't useful. We should found a way to EnableVertexAttribArray only with shader's information, not depend on Geometry information. or, we might need to calculate vertex attribute location list's hash, and use it as key. Change-Id: I3b309833a0865ad175fabf376913c3ad91e64d4a Signed-off-by: Eunki, Hong --- .../dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h | 3 +++ dali/internal/graphics/gles-impl/gles-context.cpp | 10 ++++++++++ .../internal/graphics/gles-impl/gles-graphics-reflection.cpp | 12 +++++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h b/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h index 5e5fde3..7ba7834 100644 --- a/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h +++ b/automated-tests/src/dali-adaptor/dali-test-suite-utils/test-gl-abstraction.h @@ -885,6 +885,9 @@ public: case GL_ACTIVE_ATTRIBUTE_MAX_LENGTH: *params = 100; break; + case GL_ACTIVE_ATTRIBUTES: + *params = static_cast(mAttribLocs.size()); + break; } } diff --git a/dali/internal/graphics/gles-impl/gles-context.cpp b/dali/internal/graphics/gles-impl/gles-context.cpp index 1d981bc..02d32ee 100644 --- a/dali/internal/graphics/gles-impl/gles-context.cpp +++ b/dali/internal/graphics/gles-impl/gles-context.cpp @@ -64,6 +64,13 @@ struct Context::Impl mProgramVAOCurrentState = iter->second; gl.BindVertexArray(iter->second); } + + // We should re-check enable attribute usage because geometry might be changed. + // @todo : We can remove this loop if we enable vertex attrib by shader's information. + for(const auto& attr : vertexInputState.attributes) + { + gl.EnableVertexAttribArray(attr.location); + } return; } @@ -71,6 +78,9 @@ struct Context::Impl gl.GenVertexArrays(1, &vao); gl.BindVertexArray(vao); mProgramVAOMap[program] = vao; + + // @todo : Enable vertex attrib only by shader's information, not with Geometry. + // Currently, vertexInputState.attributes depend on Geometry's VertexBuffer. for(const auto& attr : vertexInputState.attributes) { gl.EnableVertexAttribArray(attr.location); diff --git a/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp b/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp index 05776c2..d25ec1d 100644 --- a/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp +++ b/dali/internal/graphics/gles-impl/gles-graphics-reflection.cpp @@ -220,6 +220,8 @@ void Reflection::BuildVertexAttributeReflection() mVertexInputAttributes.clear(); mVertexInputAttributes.resize(nAttribs); + int maximumLocation = nAttribs - 1; + name = new GLchar[maxLength]; for(int i = 0; i < nAttribs; i++) { @@ -228,13 +230,21 @@ void Reflection::BuildVertexAttributeReflection() if(location >= 0) { + if(maximumLocation < location) + { + maximumLocation = location; + // Increate continer size s.t. we can use maximumLocation as index. + mVertexInputAttributes.resize(maximumLocation + 1u); + } + AttributeInfo attributeInfo; attributeInfo.location = location; attributeInfo.name = name; attributeInfo.format = GetVertexAttributeTypeFormat(type); - mVertexInputAttributes.insert(mVertexInputAttributes.begin() + location, attributeInfo); + mVertexInputAttributes[location] = std::move(attributeInfo); } } + delete[] name; } -- 2.7.4