FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor: use mesh shape for mesh collider by ripel2 · Pull Request #440 · EngineSquared/EngineSquared · GitHub

refactor: use mesh shape for mesh collider - #440

Merged
ripel2 merged 6 commits into
mainfrom
use-mesh-shape
Jan 25, 2026
Merged

refactor: use mesh shape for mesh collider#440
ripel2 merged 6 commits into
mainfrom
use-mesh-shape

Conversation

ripel2 commented Jan 25, 2026
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Not related to any issues

Allow for either a full mesh or convex hull shape collider

Summary by CodeRabbit

  • New Features

    • Introduced a new convex hull mesh collider component for physics-based collision detection on mesh objects with configurable hull radius.
  • Refactor

    • Modified mesh collider to use triangle mesh collision instead of convex hulls with updated configuration parameters.
    • Enhanced mesh scaling support for collision detection systems.

✏️ Tip: You can customize this high-level summary in your review settings.

ripel2 requested a review from a team January 25, 2026 15:41
ripel2 self-assigned this Jan 25, 2026

coderabbitai Bot commented Jan 25, 2026
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The changes introduce a new ConvexHullMeshCollider component for convex hull-based collisions, convert MeshCollider to handle triangle meshes with active-edge thresholding, and update RigidBodySystem to support both collision shape creation paths with scaling.

Changes

Cohort / File(s) Summary
New ConvexHullMeshCollider Component
src/plugin/physics/src/component/ConvexHullMeshCollider.hpp
New public struct with maxConvexRadius member (default 0.05f), default constructor, and parameterized constructor for convex hull configuration.
Modified MeshCollider Component
src/plugin/physics/src/component/MeshCollider.hpp
Renamed maxConvexRadius to activeEdgeCosThresholdAngle; default value changed from 0.05f to 0.996195f (cos(5°)); updated documentation for triangle mesh behavior.
RigidBodySystem Integration
src/plugin/physics/src/system/RigidBodySystem.cpp
Updated CreateConvexHullFromMesh signature to accept ConvexHullMeshCollider* and scale parameter; added new CreateMeshShapeFromMesh function for triangle mesh shapes; extended CreateShapeFromColliders to prioritize ConvexHullMeshCollider with scale extraction from Transform component.

Sequence Diagram(s)

sequenceDiagram
    participant RBS as RigidBodySystem
    participant Transform
    participant CCH as ConvexHullMeshCollider
    participant Mesh as Mesh Component
    participant JPH as Jolt Physics
    participant MeshCollider
    
    RBS->>RBS: CreateShapeFromColliders
    alt ConvexHullMeshCollider present
        RBS->>Transform: Extract scale
        RBS->>CCH: Access maxConvexRadius
        RBS->>Mesh: Get vertices & indices
        RBS->>RBS: CreateConvexHullFromMesh(mesh, collider, scale)
        RBS->>JPH: Build convex hull shape
        JPH-->>RBS: Shape reference
    else Use MeshCollider path
        RBS->>Transform: Extract scale
        RBS->>Mesh: Get vertices & indices
        RBS->>MeshCollider: Access activeEdgeCosThresholdAngle
        RBS->>RBS: CreateMeshShapeFromMesh(mesh, collider, scale)
        RBS->>JPH: Build triangle mesh shape
        JPH-->>RBS: Shape reference
    end
    RBS-->>RBS: Return Shape
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

  • feat(physics): mesh collider #439: Modifies mesh-based collider handling in RigidBodySystem, introducing a separation between convex hull and triangle mesh collision shape creation paths with updated component interfaces.

Suggested labels

enhancement

Suggested reviewers

  • Divengerss
  • ripel2

Poem

🐰 A collider splits in two, I see,
Convex hulls hop so merrily,
Triangles mesh with utmost care,
Scaling bounces through the air,
Physics shapes both near and far!

🚥 Pre-merge checks | ✅ 3 ✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately reflects the main refactoring goal: introducing mesh shapes for mesh colliders and restructuring the collision system accordingly.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/plugin/physics/src/component/MeshCollider.hpp (1)

13-22: File header documentation is outdated.

The file header still references "Convex hull mesh collider" and "ConvexHullShape," but the component has been changed to a triangle mesh collider. This creates confusion with the actual struct documentation at lines 29-42.

📝 Proposed fix for documentation
 * `@file` MeshCollider.hpp
- * `@brief` Convex hull mesh collider component
+ * `@brief` Triangle mesh collider component
 *
- * This component creates a convex hull collision shape from the
- * entity's Object::Mesh vertices using Jolt's ConvexHullShape.
+ * This component creates a triangle mesh collision shape from the
+ * entity's Object::Mesh vertices and indices using Jolt's MeshShape.
 *
src/plugin/physics/src/system/RigidBodySystem.cpp (1)

138-145: Update documentation to reflect new collider priority.

The priority order documentation doesn't mention ConvexHullMeshCollider, which is now checked before MeshCollider (at lines 188-204).

📝 Proposed documentation fix
 * `@note` Priority order when multiple colliders exist:
 * 1. SphereCollider
 * 2. CapsuleCollider
 * 3. BoxCollider
- * 4. MeshCollider (requires Object::Mesh component)
+ * 4. ConvexHullMeshCollider (requires Object::Mesh component)
+ * 5. MeshCollider (requires Object::Mesh component)
 *
- * `@note` If no collider is found, it will default to the MeshCollider with default settings, which can be pretty heavy.
+ * `@note` If no explicit collider is found, it will default to the MeshCollider with default settings, which can be heavy.
 * Make sure to always use the most appropriate colliders for RigidBodies.
🧹 Nitpick comments (1)
src/plugin/physics/src/system/RigidBodySystem.cpp (1)

106-115: Consider logging a warning for incomplete triangle data.

If indices.size() is not a multiple of 3, the trailing indices are silently ignored. This could indicate corrupted mesh data that users might want to be informed about.

💡 Suggested improvement
     JPH::IndexedTriangleList joltTriangles;
     joltTriangles.reserve(indices.size() / 3);

+    if (indices.size() % 3 != 0)
+    {
+        Log::Warn("MeshCollider: Indices count is not a multiple of 3, some indices will be ignored");
+    }
+
     for (size_t i = 0; i < indices.size(); i += 3)
     {
-        if (i + 2 >= indices.size())
-            break;
-
         joltTriangles.push_back(JPH::IndexedTriangle(indices[i], indices[i + 1], indices[i + 2], 0));
     }

Miou-zora enabled auto-merge (squash) January 25, 2026 15:57
ripel2 disabled auto-merge January 25, 2026 18:37
ripel2 merged commit fcb45ab into main Jan 25, 2026
16 checks passed
ripel2 deleted the use-mesh-shape branch January 25, 2026 18:37
This file contains hidden or 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL