Fixed issue with mesh vertex buffers not updating properly #6505
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JCash
commented
Apr 2, 2022
| @@ -28,7 +28,8 @@ namespace dmGameSystem | |||
| dmBufferDDF::BufferDesc* m_BufferDDF; | |||
| dmBuffer::HBuffer m_Buffer; | |||
| dmhash_t m_NameHash; | |||
| uint32_t m_ElementCount; | |||
| uint32_t m_ElementCount; // The number of vertices | |||
| uint32_t m_Stride; // The vertex size (bytes) | |||
There was a problem hiding this comment.
Better to keep it in the buffer resource and not the mesh resource
Comment on lines
-37
to
-38
| uint32_t m_ElementCount; | ||
| uint32_t m_VertSize; |
There was a problem hiding this comment.
Can be retrieved from the buffer resource
Comment on lines
+160
to
+171
| static uint32_t CalcBufferVersion(const MeshComponent* component, BufferResource* br) | ||
| { | ||
| HashState32 state; | ||
| dmHashInit32(&state, false); | ||
|
|
||
| uint32_t version; | ||
| dmBuffer::GetContentVersion(br->m_Buffer, &version); | ||
| dmHashUpdateBuffer32(&state, &br->m_Buffer, sizeof(br->m_Buffer)); // The handle is not a pointer, but a version+index | ||
| dmHashUpdateBuffer32(&state, &version, sizeof(version)); | ||
|
|
||
| return dmHashFinal32(&state); | ||
| } |
There was a problem hiding this comment.
One of the issues was that the buffer content version starts at 0 and counts upwards, and many other buffers may have a similar version.
So detecting a changed buffer requires more than the content version.
Comment on lines
+292
to
+293
| BufferResource* br = GetVerticesBuffer(component, resource); | ||
| dmHashUpdateBuffer32(&state, &br->m_NameHash, sizeof(br->m_NameHash)); |
There was a problem hiding this comment.
The hash is for batching, and as such we want to batch on the resource itself, not the content version which might be the same between the instances using different buffers.
Comment on lines
+435
to
+449
| dmRender::HMaterial material = GetMaterial(&component, component.m_Resource); | ||
| if (dmRender::GetMaterialVertexSpace(material) == dmRenderDDF::MaterialDesc::VERTEX_SPACE_LOCAL) | ||
| { | ||
| dmGameSystem::BufferResource* br = GetVerticesBuffer(&component, component.m_Resource); | ||
|
|
||
| VertexBufferInfo* info = world->m_ResourceToVertexBuffer.Get(br->m_NameHash); | ||
| assert(info != 0); | ||
|
|
||
| if (component.m_ReHash || (component.m_RenderConstants && dmGameSystem::AreRenderConstantsUpdated(component.m_RenderConstants))) | ||
| if (info->m_Version != component.m_BufferVersion) | ||
| { | ||
| info->m_Version = component.m_BufferVersion; | ||
|
|
||
| CopyBufferToVertexBuffer(br->m_Buffer, info->m_VertexBuffer, br->m_Stride, br->m_ElementCount, dmGraphics::BUFFER_USAGE_DYNAMIC_DRAW); | ||
| } | ||
| } |
There was a problem hiding this comment.
Moved this code to the Update(), from the RenderUpdate().
| bool BuildVertexDeclaration(BufferResource* buffer_resource, | ||
| dmGraphics::HVertexDeclaration* out_vert_decl, | ||
| uint32_t* out_elem_count, uint32_t* out_vert_size) | ||
| bool BuildVertexDeclaration(BufferResource* buffer_resource, dmGraphics::HVertexDeclaration* out_vert_decl) |
There was a problem hiding this comment.
No need for the function to handle variables that can be retrieved from the buffer resource.
| @@ -749,7 +749,7 @@ namespace dmGameSystem | |||
| *s = 0; | |||
| uint32_t version = 0; | |||
| dmBuffer::GetContentVersion(hbuffer, &version); | |||
| dmSnPrintf(buf, sizeof(buf), "buffer.%s(count = %d, version = %u", SCRIPT_TYPE_NAME_BUFFER, out_element_count, version); | |||
| dmSnPrintf(buf, sizeof(buf), "buffer.%s(count = %d, version = %u, ", SCRIPT_TYPE_NAME_BUFFER, out_element_count, version); | |||
britzl
reviewed
Apr 3, 2022
|
|
||
| uint32_t elem_count = br->m_ElementCount; | ||
| VertexBufferInfo* info = world->m_ResourceToVertexBuffer.Get(br->m_NameHash); |
britzl
approved these changes
Apr 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Under certain circumstances meshes did not update correctly even though the buffers changed. This fix ensures that a mesh is updated when the buffer content changes.