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

KHR_implicit_shapes extension Draft Proposal #2370

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

eoineoineoin
Copy link
Member

@eoineoineoin eoineoineoin commented Mar 19, 2024

Draft of an extension which describes generic, non-renderable shapes suitable for collision detection.

Known Implementations

Blender importer/exporter
Babylon.js importer
Godot importer

Sample Assets

https:/eoineoineoin/glTF_Physics/tree/master/samples

"extras": { }
},
"required": [
"convex",
Copy link
Member

Choose a reason for hiding this comment

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

Not a strong opinion but would it be reasonable to make the property not required with the false being the default value?


As both the `mesh` shape references a `mesh`, it additionally allows for optional `skin` and `weights` properties, which have the same semantics and requirements enforced by the properties of the same name associated with a `node`. When specified on a `mesh` whose `convex` property is `true`, the resulting collision shape should be the convex hull of the deformed mesh. As collision detection is typically performed on CPU, the performance impact of deforming a mesh in such a use-case is typically higher than inside a vertex shader. As such, use of this functionality should be given careful consideration with respect to performance. When `convex` is `true`, the referenced mesh need not necessarily be convex itself, nor is there any requirement for the geometry to be closed. An implementation must generate a convex hull from the input vertices.

Degenerate shapes are prohibited. A `sphere` must have a positive, non-zero radius. `box` shapes must have positive non-zero values for each component of `size`. `cylinder` and `capsule` shapes must have a positive, non-zero `height` and both `radiusTop` and `radiusBottom` should be positive; at least one of the radii should be non-zero. For `mesh` shapes whose `convex` property is `false`, the referenced mesh must contain at least one non-degenerate triangle primitive. For `mesh` shapes whose `convex` property is `true`, the referenced mesh must contain contain primitives with at least four non-coplanar vertices.
Copy link
Member

Choose a reason for hiding this comment

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

Please do not use backticks unless explicitly referring to properties. For example:

A sphere must...

Box shapes must...

@EricGriffith
Copy link

  1. It would be great to include example usage of the schemas in the spec.

  2. both seems to be a typo here:

As both the mesh shape

  1. The convex property name is confusing. To me, it implies that the mesh is required to be convex. I prefer the previous version where convex and mesh were separate shapes. If momentum means it they must stay combined, please use a more descriptive name like convexHull.

  2. The skin and weights properties should probably be disallowed when a mesh shape is used together with a node that uses the same mesh for rendering. Otherwise, every extension using these shapes will need to repeat the same normative text explaining the precedence of the two (node + shape) skin properties and the three (node + mesh + shape) weights properties.

Rename "convex"->"convexHull". Tidy up phrasing, documentation.
Improved introduction text; add an inline sample defining a shape.
Clarifications around degenerate shapes & node scale.
Remove backticks when not referring to a property.
Copy link
Contributor

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

The contents of this PR do not match the repo in which the extension was developed: https:/eoineoineoin/glTF_Physics/tree/master/extensions/2.0/Khronos/KHR_collision_shapes/schema

If changes are needed, the original repo should also be updated so that it stays in sync.

Comment on lines 18 to 29
"skin": {
"description": "The index of a skin with which to deform the mesh.",
"allOf": [ { "$ref": "glTFid.schema.json" } ]
},
"weights": {
"type": "array",
"description": "The weights of the instantiated morph target.",
"minItems": 1,
"items": {
"type": "number"
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused, why are skins and weights used for collision meshes? Where was the discussion on this? What is the use case?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was discussed in several Khronos meetings on the interactivity spec. These parameters match functionality already in the glTF node. The usecase is to allow users to describe collision shapes which accurately match the render meshes.

Copy link

Choose a reason for hiding this comment

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

collision shapes which accurately match the render meshes

I could see weights/skins being calculated once at load time, but since "interactivity" is brought up, it sounds like the intent is to permit changing vertices on any given frame. I cannot find a good specification for the intended behavior when vertices of a collision mesh are animated at runtime (e.g. on every frame). I see two big problems:

  1. Firstly, it seems highly unperformant: both the need to regenerate optimized collision geometry (such as a convex hull or deduplicated trimesh vertices); and to run skinning on the CPU (some engines upload skinned mesh geometry to the GPU and do not store a copy on the CPU, so this may involve data transfer from the GPU on each frame). In the case of convex hulls, it might not be feasible to re-build the hull every frame in which it may have been animated.

  2. In a physics sense, it seems to me problematic that the skinned mesh deformation is non-physical: if the change in shape causes a collision, does it exert a force? if so, what force? if not, will it be allowed to intersect with adjacent geometry from one frame to the next, or entirely phase through it?

As a concrete example, imagine a razor-thin mesh floor of an elevator, using a skin joint to move upwards, and a small marble resting on the floor of this elevator. Imagine an implementation performs CPU deformation by calling BakeMesh() every frame, effectively swapping out one mesh for another. If the joint node moves quickly enough, the floor of the elevator may move more in one physics frame than the radius of the marble, causing it to fall. This will be highly dependent on physics framerate, which could differ either due to hardware specs or by user agent.

Ultimately, I believe the likely outcome is the majority of conventional game engines will choose between ignoring support for skins/morphs for physics shapes (which means inconsistent support), or doing something unperformant (for example, calling BakeMesh() every frame). This sort of feature sounds cool, but it's a pie in the sky, and the likelihood of inconsistent support could scare users away from using the feature, which tells me that it belongs in a separate extension.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that mesh objects already have weights defined in the core glTF specification. Those weights may be animated, so this needs to be supported anyway. These properties merely the shapes consistent with how nodes can apply per-instance weights to a mesh, as well as using a skinned mesh.

As with many glTF features, it's possible to describe scenarios which are non-physical or may perform poorly, but at the end of the day, glTF content is made by a human who is ultimately making decisions about what that content is. The fact that a feature may be misused is not a good reason to say "we don't support this feature which is in the core spec."

@npolys
Copy link

npolys commented May 31, 2024 via email

    Remove convex hull options from base KHR_collision_shapes; for many
    cases, convex hulls are not necessary and having to support convex hull
    generation in a viewer may be a significant implementation burden.

    Refactor morph targets to support the common use-case of describing a
    mesh collider which intends to use the same morph target weights as a
    node's instance weights. Add an explicit recommendation for how an
    extension can reference a collision shape.

    Upgrade schemas to draft 2020-12.
@eoineoineoin eoineoineoin changed the title Merge of KHR_collision_shapes extension KHR_collision_shapes extension Draft Proposal Jul 10, 2024
@wallabyway
Copy link

wallabyway commented Aug 6, 2024

@eoineoineoin - Thanks for bringing up 'implicit shapes'.

If I could pick two shapes that would help AEC, it would be cylinder and cube. We could add a million other shapes, but I would compromise everything, just to get these two into a future spec somehow.

We pay a very high price for cylinders (HVAC, railings, cables, rebar, etc). If the industry agreed on this, there would be a lot of precision gained (measurement), less data-loss (from pre-baked tessellation) and far fewer triangles transported and rendered.

Same for 'Cubes'. If the industry agreed on a cube shape, rather than "I think these 16 triangles are a cube", it would add a lot of meaning for AEC - the design intent is captured, it's manifold, it has an agreed volume, render outline-style without guessing, improved z-fighting of overlapping volumes.

I know it's a very low bar, but you've got to start somewhere and these two additions add an impressive amount of value IMHO.

@javagl
Copy link
Contributor

javagl commented Aug 8, 2024

@wallabyway I've lost track of some of the discussion/developments here, and am (unfortuantely) not up to date. But a while ago, I also advocated for basically creating KHR_shapes and KHR_collision (using KHR_shapes) extensions. Somewhere around eoineoineoin/glTF_Physics#1 (comment) . The use-case would be for CAD/AEC (maybe going so far that these could even serve as a basis for some CSG at some point...).

When "shapes" are defined here (non-renderable, as part of a "collision" extension), then someone will create a new extension that defines the same shapes, but "renderable". And in the worst case, one defines a "box" as (min, max), and the other one defines a "box" as (origin, size), and we all know what happens then...

Some further related discussion is in #1135 and around #1986 (comment) , and many issues that are linked to/from these. (It's hard to wrap this up - that's why I lost track...).

@eoineoineoin
Copy link
Member Author

@wallabyway I've lost track of some of the discussion/developments here, and am (unfortuantely) not up to date. But a while ago, I also advocated for basically creating KHR_shapes and KHR_collision (using KHR_shapes) extensions. Somewhere around eoineoineoin/glTF_Physics#1 (comment) . The use-case would be for CAD/AEC (maybe going so far that these could even serve as a basis for some CSG at some point...).

When "shapes" are defined here (non-renderable, as part of a "collision" extension), then someone will create a new extension that defines the same shapes, but "renderable". And in the worst case, one defines a "box" as (min, max), and the other one defines a "box" as (origin, size), and we all know what happens then...

Some further related discussion is in #1135 and around #1986 (comment) , and many issues that are linked to/from these. (It's hard to wrap this up - that's why I lost track...).

Hey Marco,

Right, it does seem that there's interest in using the shapes for rendering, amongst other things. So, at the very least, there's a rename of the extension and associated reworking of documentation in the works, to ensure it's obviously usable for multiple different use-cases.

The "mesh" shape type is causing us some hassle, though, so I want to see if you have any thoughts here. For things like physics, selectability, UI, etc., it's useful to be able to describe a shape which is derived from a glTF.mesh. Though, for the "shape rendering" extension, this shape.mesh is redundant, since glTF meshes are already renderable via nodes. That leaves us with a couple of options:

  1. Remove shapes.mesh, require "physics extension" and friends to reference glTF.mesh via some other mechanism
  2. Leave as-is; "shape rendering extension" and friends would define how those shapes.mesh are handled

My concern is that with option 1, the options available in the shapes.mesh today will have to be re-defined by any extension which wants to use meshes. Similarly, with option 2, extensions might all require documentation to say how each shape type is handled (though, this might need to be the case anyway?) Any preferences? Other ideas?

@javagl
Copy link
Contributor

javagl commented Aug 14, 2024

It's hard to say anything really profound here, without all the context, and not knowing the plans that may have been discussed internally already. So all I can say is very high-level, and should be taken with a grain of salt.

I think that having a real glTF mesh as a collision shape already does raise questions. Meshes can be arbitrarily complex, could be self-intersecting, don't have to be manifold - heck, a mesh could actually be a soup of unconnected triangles. Quickly looking at the README here, it does not seem to impose any real constraints for that mesh. I could imagine that implementors of physics engines will not really know what to do with that, and think that there should either be

  1. a description in the README about how to deal with such shapes (i.e. which degrees of freedom implementors have for handling it - like computing a convex hull from it, and using that as the collision shape)
  2. or constraints on the specification level - like that mesh must be manifold or maybe even that it must be a convex hull (some thoughts about convex hulls are also mentioned at the bottom of this comment)

That said, from my current, limited understanding, I'm not sure what the problem would be with the point that " glTF meshes are already renderable via nodes" that you mentioned:

  • A glTF can define a mesh
  • The mesh can be attached to a node, so that it is rendered
  • The mesh can be referred to by a shape
  • A shape can be attached to a node, so that it is rendered
    (At least, that's what is considered as part of the 'generalization' to use shapes not only for collisions, but also for rendering)

If I understood your concern correctly, then it is that someone could do something like
node -> shape -> mesh
instead of the direct
node -> mesh
to render a mesh. If this is correct, then: Yes, that would be "quirky", and an unnecessary 'detour'. But it would just be the result of saying "Attach this shape to the node (regardless of what kind of shape it is)".

Or, referring to your point

with option 2, extensions might all require documentation to say how each shape type is handled

Then I think that the mesh type would actually be the easiest case here: It would boil down to rendering the mesh as-if it was attached directly to the node. (For the other types, this may be more tricky - e.g. "How many triangles should a rendered sphere consist of?" ...)

(I might be missing a point here...)

@eoineoineoin eoineoineoin changed the title KHR_collision_shapes extension Draft Proposal KHR_implicit_shapes extension Draft Proposal Oct 16, 2024
@lyuma
Copy link

lyuma commented Oct 17, 2024

I have been reading through the recent changes to use a collider or trigger "geometry" object.

I have some concern with the new version of the spec because it references "node", that implies referencing a node, which must be visible in core gltf. Is there a way to use meshes in physics colliders without rendering those meshes?

Related to this problem is a confusing paragraph here, so I would like some clarification on this:

Collision shapes are parameterized in local space of the node they are associated with. If a shape is required to have an offset from the local space of the node the shape is associated with (for example a sphere not centered at local origin or a rotated box,) a child node should be added with the desired offset applied, and the shape properties added to that child.

if I understand correctly, does this paragraph that the nodes and child nodes (a comment by Khronos members implies this actually means descendants) in the context of mesh geometry are taken in local space of the referenced geometry / node?

if so, can I ask if an acceptable way to implement hidden mesh collision shapes is to make a node whose scale is [0,0,0] (which hides the mesh as per the core gltf spec): since the collision shape children use a coordinate space relative to this node, child nodes will ignore the 0 scale. Does this make any sense?

if for some reason using [0,0,0] scale is not possible, the only alternatives I can think of are using a translation at 1e+99 to effectively hide the mesh, or relying on the node visbility extension (but this will not degrade well to older versions of Blender, for example)

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.

8 participants