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

Potential skinning issue with glb files #84479

Open
BalintCsala opened this issue Nov 5, 2023 · 11 comments
Open

Potential skinning issue with glb files #84479

BalintCsala opened this issue Nov 5, 2023 · 11 comments

Comments

@BalintCsala
Copy link

Godot version

4.1.3

System information

Windows 11 - Godot 4.1.3 - Forward+

Issue description

I was experimenting with using Blender's bendy bones for animation by copying the transform of the segments to standard bones, but the exported animation shows up weirdly in Godot. Here's what the animation at a certain frame should look like
image
And here's the result from Godot:
image

The same issue doesn't happen when I export to FBX and use FBX2GLTF and with other GLTF viewers (I tried this and this), so this is most likely an edge-case in gltf-s godot doesn't handle properly.

Steps to reproduce

  1. Open the attached project
  2. Make sure "test" has "Editable children" enabled
  3. Select "AnimationPlayer" and set "Current Animation" to "ArmatureAction"

Minimal reproduction project

SkinningIssue.zip

@fire
Copy link
Member

fire commented Nov 6, 2023

Thats so odd @lyuma someone finally found an error case

@game009f
Copy link

game009f commented Nov 6, 2023

He was caused by LOD
1
2

@BalintCsala
Copy link
Author

Disabling LODs does solve the issue, but should that be on by default in this case?

@Calinou
Copy link
Member

Calinou commented Nov 6, 2023

I suggest testing #84384 locally and see if it resolves the issue on your end.

Disabling LODs does solve the issue, but should that be on by default in this case?

Yes, as LOD isn't supposed to break meshes like this 🙂

If we disable LOD generation by default, we lose a lot of its benefits as a lot of people won't bother enabling it manually.

@fire
Copy link
Member

fire commented Nov 7, 2023

The lod operator doesn't account for per vertex bone groups.

tl;dr We only match the closest normals in the vertex attributes.

The layout of inputs are:

size_t new_index_count = SurfaceTool::simplify_with_attrib_func(
	(unsigned int *)new_indices.ptrw(),
	(const uint32_t *)merged_indices_ptr, index_count,
	merged_vertices_f32.ptr(), merged_vertex_count,
	sizeof(float) * 3, // Vertex stride
	merged_normals_f32.ptr(),
	sizeof(float) * 3, // Attribute stride
	normal_weights, 3,
	index_target,
	max_mesh_error,
	simplify_options,
	&mesh_error);

Probably want the notion that bone groups that are close are kept together, but each different bone group has very different values numerically. Like bone group 0 for a "head" is numerically different from bone group 8 for the "neck" even though the vertex groups are connected.

We can check for "closeness" for normals, but we need to define what it means for a bone group to be close.

Edit 1:

The ai assistant suggests calculating the center point of each bone group by averaging the positions of all bones in the group, then calculates the distance between these center points and stores them in a list. This list was then suggested to be added as a per vertex custom attribute to the mesh, but it is not feasible to add a 70,000 (Example Mesh) vertex distance list to the meshoptimizer.

Edit 2:

The AI assistance suggests we can modify our simplification algorithm to better preserve vertices that belong to the same bone group. The idea is to adjust the cost function in a way that adds a penalty for collapsing edges that connect vertices from different bone groups. This mesh optimization could make it less likely for these edges to collapse, and as a result, the vertices within each bone group would be preserved.

@lawnjelly
Copy link
Member

lawnjelly commented Nov 7, 2023

Doing LOD on skinned meshes is a fun topic, I'm sure there will be research into this.

Because whether weighted polys can be reduced depends on the poses expected! 😁

At the extreme an animated skeleton with little movement can be decimated as usual. Whereas if just one arm moves a lot, only those polys need be prevented from decimation more.

Simplest scheme is just to not LOD any verts in polys that contain weights. That will preserve the joints but will limit how far poly reduction can take place.

In theory if you have a bunch of poses, you can calculate the max error metrics for use in decimation from these (by software skinning each pose maybe).

Congrats to the AI on getting the simplest. 😉

For a universal "pose agnostic" approach you could measure the deviation of a collapsed vertex in extreme bone positions and use this as a metric. But without knowledge of the poses, it will probably suck somewhat. 🤔

You might be able to use a "poor man's metric". If a vert is weighted 0.5, 0.5 to two bones, then reduce it's likelihood of collapse. If it is weighted 0.9, 0.1 then more likely to collapse, etc.

@fire
Copy link
Member

fire commented Nov 7, 2023

I was investigating using Smooth Skinning Decomposition with Rigid Bones. This algorithm extracts Linear Blend Skinning (LBS) from example meshes and can be used for:

  • Converting animated mesh sequences to LBS.
  • Solving skinning weights from shapes and skeleton poses.
  • Solving bone transformations for a mesh animation given skinning weights.

https://github.com/electronicarts/dem-bones

However, it doesn't directly relate to solving this problem of modify our simplification algorithm to better preserve vertices for an animated mesh.

I also have an implementation of direct delta mush lying around in a game engine, but these are about animation skinning correction and not calculating metric for when to decimate.

Directed Delta Mush is a technique used in 3D animation to smooth deformations in skinned characters. It's particularly useful for correcting artifacts that can occur during skinning, such as volume loss or unwanted creasing.

https://github.com/V-Sekai/unity-mesh-deform-direct-delta-mush

@zeux
Copy link
Contributor

zeux commented Nov 8, 2023

The result from the image doesn't really make sense to me, but I'm not sure what the mesh that the simplifier is being fed here is - eg I'm not sure I understand what the deformation structure here is, maybe a lot of vertices are in the same position and only are moved by the final bone deformation. I think the mesh importer skins vertices before simplification but that code could be getting incorrect (default?) transforms in this case maybe. (see next comment)

If the initial bone transforms here are identity and the expectation is that the mesh would be in tact after applying different transforms at runtime, then one possible solution is to extract a single bone id for the dominant bone per vertex (largest weight), and at the minimum use it during vertex merging, and also maybe pass it as an extra attribute with some weight to the simplifier. The update to meshoptimizer makes it so that the attribute count is fully dynamic so there won't be a need to patch the simplifier source code anymore (right now the max attribute count is baked into the library source).

There's research on skinning aware simplification but it's fundamentally a difficult problem to solve in a reasonable amount of time. But it also feels like the image here is more wrong than anything I've seen with normal skinning (I've tested simplifier mostly with pre-deformed mesh vertices and it worked completely fine!). (see next comment)

One issue that was mentioned in #84384 (btw I don't think that should affect this import one way or the other but not sure) is the unbounded positional error; in effect the simplifier is allowed to produce completely arbitrary result. I do think it would be nice to change this longer term, as it can improve the quality here, but on unskinned meshes I would expect the current approach to be okay if suboptimal - if the deformation is too large, the resulting LOD's error will be large, and it should not be selected for rendering. So I don't think this issue should depend on whether the error is bounded (and I would recommend waiting for bounding the error until Godot fully switches to unmodified meshopt_simplify, so probably 0.21 or thereabouts, as right now just passing a smaller error will not work properly due to differences in metrics that include or exclude attributes in the interface).

@zeux
Copy link
Contributor

zeux commented Nov 8, 2023

the image here is more wrong than anything I've seen with normal skinning

Well, the good news is, I can reproduce this in meshoptimizer's test harness :D so this is not unique to Godot import. And yeah without animation (in the initial position) the simplification looks fine - but that's because the initial pose is a straight cylinder, so the simplifier removes triangles along the cylinder's sides. These collapses are also very low error because, well, the shape doesn't change if you do them.

Of course this all changes once the bones are actually rotated. Something like this is inevitable under bone deformation but this of course is a particularly egregious case.

The only way to solve this particular mesh, other than disabling LOD generation during import :), is to feed some information to the simplifier that would tell it something about the deformation. It could be bone id, some info about transforms, or weights, basically anything that helps differentiate the triangles...

One additional issue though is that because the distance error is very small, and distance error is used for LOD selection in Godot, this alone won't help enough - it's also the case that a combined error will need to be stored for LOD selection to properly reject other LODs.

For that to work, import process would need to store the combined error and the default normal weighting will need to be dramatically adjusted - right now the simplification gets weight 2.0 for normals but that dramatically increases the combined position+normal weighting that simplifier uses. I suspect that to make reasonable LOD selection decisions using the combined error, not just position error, on "regular" meshes, the normal weight will need to be in the 0.01..0.1 range, although I haven't really experimented with this.

I guess one other possibility here is that maybe we can adjust bone rotations during import; there's nowhere really to extract the "proper" values from but maybe if the source file contains animation data, we can just pick a random rotation for each bone from a track? Or maybe just pick a random bone rotation for each bone period :) This alone will probably fix this issue here without further changes to metrics or whatnot, although I think that would be nice to do independently.

@zeux
Copy link
Contributor

zeux commented Nov 8, 2023

Quick example for incorporating the dominant bone index as an attribute -- I switch to attribute-aware simplification half way through the video (sorry for video quality, it took me 3 tries to get a sub-10 MB video that apparently GH attachments are limited to :-/):

Screencast.from.2023-11-07.19-22-56.1.mp4

Per comments above, this alone would not suffice because the resulting attribute error is presently ignored so the mesh will have better quality LODs but will still probably display poorly, but this can likely be a reasonable way to attack this based on this. I would probably experiment with more physically-inspired inputs than the index, for example dominant bone translation or something like this, as this will make more sense when integrated into the overall error, but maybe even ID is workable with carefully tuned weighting.

@fire
Copy link
Member

fire commented Apr 3, 2024

@zeux I did an attempt to do rig aware lods here and it seems to be ok. https://github.com/V-Sekai/godot/tree/lock-primary-base-influence

Needs optimization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants