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

feat: add remove mesh by blend shape component #275

Merged
merged 11 commits into from
Aug 7, 2023

Conversation

nil-vr
Copy link
Contributor

@nil-vr nil-vr commented Jul 31, 2023

Added by anatawa12: Closes #43

This PR adds a new component for removing mesh by blend shapes (#43). It's based on the "Remove Mesh in Box" and "Freeze BlendShapes" components. If one of the checked blend shapes translates all of the vertices of a triangle by more than "Tolerance" then the triangle is removed from the mesh.

It might also make sense to remove these blend shapes. However, they are still used by triangles that are only partially affected by the blend shapes. Maybe FreezeBlendShapesProcessor could be called at the end of RemoveMeshByBlendShapeProcessor, but FreezeBlendShapesProcessor uses FreezeBlendShapes and RemoveMeshByBlendShapeProcessor doesn't have one.

Copy link
Owner

@anatawa12 anatawa12 left a comment

Choose a reason for hiding this comment

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

Thank you for implementing RemoveMeshByBlendShape.

If one of the checked blend shapes translates all of the vertices of a triangle by more than "Tolerance" then the triangle is removed from the mesh.

I think removing triangle if any of the vertices is translated.
In RemoveMeshInBox, it's intended to remove polygon in box.
On the other hand, I think RemoveMeshByBlendShape should remove transformed vertices.

In addition, can you add to changelog?

Runtime/RemoveMeshByBlendShape.cs Outdated Show resolved Hide resolved
Runtime/RemoveMeshByBlendShape.cs Outdated Show resolved Hide resolved
Editor/RemoveMeshByBlendShapeEditor.cs Outdated Show resolved Hide resolved
Editor/RemoveMeshByBlendShapeEditor.cs Outdated Show resolved Hide resolved
@nil-vr
Copy link
Contributor Author

nil-vr commented Aug 4, 2023

This still doesn't remove the blend shapes that are removed, so a separate freeze blend shapes behavior should be used.

Having used it on some avatars, I wonder if this is the right behavior and editor. Maybe it'd be better if deleting and freezing were in the same behavior. Even if this behavior were to remove the blend shapes, they would still appear in the freeze blend shapes editor.

@anatawa12
Copy link
Owner

Maybe it'd be better if deleting and freezing were in the same behavior.

I think we should remove the blendshapes in RemoveMeshByBlendShape component.
I think we can pull some of FreezeBlendShapesProcessor into an static method and use the static method is one good way to implement.

Even if this behavior were to remove the blend shapes, they would still appear in the freeze blend shapes editor.

In SkinnedMeshRenderer, It's expected because AAO is non-desctructive avatar modification tool. If you mean you can find in other AAO components, we have to remove blendshape name in MeshInfoComputer to tell other AAO components that it's removed.

@nil-vr
Copy link
Contributor Author

nil-vr commented Aug 6, 2023

RemoveMeshByBlendShapeProcessor now calls a static method from FreezeBlendShapeProcessor to remove the blend shapes without needing to also select the same blend shapes in a separate FreezeBlendShapesComponent.

It isn't smart enough to handle the case where another blend shape transforms a subset of the vertices transformed by the deleted blend shapes. For example, Mamehinata has Shoulder_OFF and Shoulder_OFF_left. If you delete the vertices affected by Shoulder_OFF, Shoulder_OFF is deleted from the skinned mesh renderer, but a useless Shoulder_OFF_left remains. Maybe there could be a "Remove unused blend shapes" feature that would clean everything up using similar logic to find blend shapes that transform no vertex by more than some tolerance value.

However, for my workflow it doesn't matter. I freeze every blend shape on that mesh anyway.

Copy link
Owner

@anatawa12 anatawa12 left a comment

Choose a reason for hiding this comment

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

It looks good for me. Thanks a lot!

I think freezing meaningless BlendShapes is out of scope of this component. we can add some pass in Automatic Configuration component to remove such a BlendShapes in the feature.

@anatawa12 anatawa12 changed the base branch from master to remove-mesh-by-blend-shape August 7, 2023 07:30
@anatawa12 anatawa12 merged commit 1e63345 into anatawa12:remove-mesh-by-blend-shape Aug 7, 2023
5 of 6 checks passed
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.

remove mesh based on blendshape
2 participants