Skip to content

Conversation

@RyeMutt
Copy link
Member

@RyeMutt RyeMutt commented Oct 2, 2025

Description

Introduces a convex decomposition solution based on the VHACD library

Related Issues

Issue Link:


Checklist

Please ensure the following before requesting review:

  • I have provided a clear title and detailed description for this pull request.
  • If useful, I have included media such as screenshots and video to show off my changes.
  • I have tested the changes locally and verified they work as intended.
  • All new and existing tests pass.
  • Code follows the project's style guidelines.
  • Documentation has been updated if needed.
  • Any dependent changes have been merged and published in downstream modules
  • I have reviewed the contributing guidelines.

Additional Notes

@RyeMutt RyeMutt self-assigned this Oct 3, 2025
Copy link
Contributor

@LiruMouse LiruMouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this looks good enough to me, I just have some small suggestions.

return LLCD_OK;
}

LLCDResult LLConvexDecompositionVHACD::getMeshFromStage( int stage, int hull, LLCDMeshData* meshDataOut )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both get*FromStage functions are identical, aside from the datatype, so you could add a template member function to condense the logic and just have each call the template, thereby maintaining the interface.

Comment on lines +193 to +211
const U16* index_data = static_cast<const U16*>(data);
const int stride = index_stride_bytes / sizeof(U16);
for (int i = 0; i < num_indices; ++i)
{
indices.emplace_back(index_data[i * stride + 0],
index_data[i * stride + 1],
index_data[i * stride + 2]);
}
}
else
{
const U32* index_data = static_cast<const U32*>(data);
const int stride = index_stride_bytes / sizeof(U32);
for (int i = 0; i < num_indices; ++i)
{
indices.emplace_back(index_data[i * stride + 0],
index_data[i * stride + 1],
index_data[i * stride + 2]);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be condensed into a local template function, or even a lambda

auto set_indices = [&]<typename T>(){
    const T* index_data = static_cast<const T*>(data);
    const int stride = index_stride_bytes / sizeof(T);
    for (int i = 0; i < num_indices; ++i)
    {
        indices.emplace_back(index_data[i * stride + 0],
                             index_data[i * stride + 1],
                             index_data[i * stride + 2]);
    }
};

{
clear();

if (!meshIn || !meshIn->mVertexBase || (meshIn->mNumVertices < 3) || (meshIn->mVertexStrideBytes != 12 && meshIn->mVertexStrideBytes != 16))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is the same as in the function above, a template function could share the logic.

Comment on lines +4741 to +4742
LLUUID decomp_id = decomp->mMeshID; // Copy to avoid invalidation in below deletion
decomposition_map::iterator iter = mDecompositionMap.find(decomp_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does order matter? If it doesn't, you can do the erasure before deleting decomp, and then you don't need a copy.

@RyeMutt RyeMutt merged commit d709ee2 into develop Oct 11, 2025
15 checks passed
@RyeMutt RyeMutt deleted the rye/convexdecomp branch October 11, 2025 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants