Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Margin property to ConvexHullCollider #1577

Closed
manio143 opened this issue Dec 29, 2022 · 2 comments
Closed

Add Margin property to ConvexHullCollider #1577

manio143 opened this issue Dec 29, 2022 · 2 comments
Labels
area-Physics enhancement New feature or request good first issue Good for newcomers

Comments

@manio143
Copy link
Member

Is your feature request related to a problem? Please describe.
Bullet allows specifying a margin for colliders which can be especially useful in case of ConvexHull. Reported as missing by @mathmb1986 in #1574

Suggested code changes in the thread mentioned above:
https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Assets/Physics/ColliderShapeAssetCompiler.cs
L135

                    ConvexHullColliderShapeDesc convexHullDescClone = new ConvexHullColliderShapeDesc
                    {
                        Scaling = convexHullDesc.Scaling,
+                       Margin = convexHullDesc.Margin, 
                        LocalOffset = convexHullDesc.LocalOffset,
                        LocalRotation = convexHullDesc.LocalRotation,
                        Decomposition = convexHullDesc.Decomposition,
                    };

https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Physics/Shapes/ConvexHullColliderShape.cs
L40

+        public float Margin
+        {
+            get { return InternalShape.Margin; }
+            set { InternalShape.Margin = value; }
+        }
+

https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Physics/Data/ConvexHullColliderShapeDesc.cs
L51

+        /// <userdoc>
+        /// The Margin of the generated convex hull.
+        /// </userdoc>
+        [DataMember(48)]
+        public float Margin { get; set; }
+

L84

                    shape = new ConvexHullColliderShape(ConvexHulls[0][0], ConvexHullsIndices[0][0], Scaling)
                    {
                        NeedsCustomCollisionCallback = true,
+                       Margin = Margin,
                    };

L109

                    var subHull = new ConvexHullColliderShape(verts, indices, Scaling);
+                   subHull.Margin = Margin;

L136

                    var subHull = new ConvexHullColliderShape(verts[0], indices[0], Scaling);
+                   subHull.Margin = Margin;

L151

                        var subHull = new ConvexHullColliderShape(subVerts, subIndex, Scaling);
+                       subHull.Margin = Margin;

A closer look should be placed on application of margin to all subHull components to see if this makes sense. Also, maybe it would make more sense to add Margin property on the base class ColliderShape - which is where Margin is defined in Bullet - this way all collider shapes could benefit.

@manio143 manio143 added enhancement New feature or request good first issue Good for newcomers area-Physics labels Dec 29, 2022
@dloe
Copy link
Contributor

dloe commented May 16, 2024

Hi, as a newcomer to the engine I was wondering if this issue is being worked on by anyone. If not, I would like to try this issue to get more familiar with the code base.
Thanks!

@Doprez
Copy link
Contributor

Doprez commented May 16, 2024

I'd say go for it if you want to learn the engine it will be very usefull info. AFAIK no one is working on new features for Bullet specifically right now, @Eideren, @Nicogo1705 and I are doing some unrelated work with Bepu in this PR since the future goal is to move on to Bepu but there is still a lot of work to do before that happens.

dloe added a commit to dloe/stride_fork that referenced this issue May 20, 2024
Added margin field to parent class ColliderShape, therefore all collider shapes can have access.
Eideren pushed a commit that referenced this issue May 22, 2024
* Added margin fields to ConvexHullCollider (#1577)

Added margin field to parent class ColliderShape, therefore all collider shapes can have access.

* Added default for margin to be 0.04f

Context: Margins in bullet by default should be around 0.04 for dynamic and 0.0 for static objects. Convex hulls are likely used as replacement for the lack of support of non-static meshes, so best set it to the default value for dynamics, so 0.04 by default.
@Eideren Eideren closed this as completed May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Physics enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants