Whinarn / UnityMeshSimplifier

Mesh simplification for Unity.
MIT License
1.76k stars 277 forks source link

Bug in Skinned Mesh Combiner #27

Closed bawar9 closed 4 years ago

bawar9 commented 4 years ago

Hi, I came across a problem when checking "combine mesh" option during LOD generation. The combined meshes seemed to be totally invalid. I have also noticed that this doesn't happen with every model. Also what I've noticed is that it only combines Skinned Renderers in an invalid way. Static Renderers are combined correctly. I attach some images below that show you 2 models after combining meshes and before combining meshes.

Model 1(Barbarian Before Combining Meshes) Before1Combining

Model 1(Barbarian After Combining Meshes) AfterCombining1

Model 2(Troll Before Combining Meshes) BeforeCombining2

Model 2(Troll After Combining Meshes) AfterCombining2

Here is a video that demonstrates this as well.

https://youtu.be/hQbkqB0P1S8

And here is the UnityPackage with the MeshSimplifier and the afore mentioned 3d models.

https://filebin.net/c0b5quc8gjbrm3py/MeshCombineBug.unitypackage?t=uextr26c

Whinarn commented 4 years ago

Hi @bawar9,

I have tested both models that you provided above. The first one was caused by a bindpose mismatch. My logic for dealing with it was untested since I have not had any models where this happened. But after playing around, I believe that I have a fix. I believe that it should still be possible to solve in a better way, but it appears to solve the problem experienced. You can try it out with the fix/mesh-combine-bindpose-mismatch branch and confirm it yourself. If it's satisfactory for you, then I will include it in a new release.

For your second model, I'm not yet sure what the problem is. But you'll also have a second problem with it since you have a static mesh attached to one of the bones. The mesh combination logic will not deal with this properly, since that would require it to modify the original game object hierarchy, which I have tried to avoid in order to prevent breaking anything in a worst case scenario. I'm not sure how I'd want to deal with that, but for now I wouldn't recommend to use the mesh combination for such scenarios.

I'll get back to you if I advance any more.

bawar9 commented 4 years ago

Hi @Whinarn, Thank you for the quick fix. I can confirm that the fix works for the first model (Barbarian). For the second model I hope you can help me and figure out a fix. Although I can program stuff but I have got very basic knowledge in 3d geometry and rendering so I don't really understand 3d meshes in depth and honestly speaking I couldn't really find any good structured resource to gain some knowledge on the subject. I really appreciate your work and the efforts put in this project, also I understand that you might be doing this in your spare time. I wish I could join hands with you on this, but as I said I don't have the knowledge required to do so. I saw some other reported issues on this project, I think I have a solution for the following two:

https://github.com/Whinarn/UnityMeshSimplifier/issues/19 https://github.com/Whinarn/UnityMeshSimplifier/issues/16

I would love to contribute the changes I made in the original algorithm that might fix the above two issues, but don't really understand how to push changes to this repository.

Please do let me know if I can help in any way on solving the second problem.

Whinarn commented 4 years ago

@bawar9 If you wish to contribute, the easiest would be if you fork this repository and make your changes in a branch and then create a pull request into this repository to let me review it.

bawar9 commented 4 years ago

@Whinarn Thank you, I'll be contributing very soon :)

Whinarn commented 4 years ago

Closing this issue now that both problems will be resolved by next version.