Whinarn / UnityMeshSimplifier

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

Lossless simplification of plane with 2x3 vertices results in empty mesh #59

Closed michaelclockstone closed 2 years ago

michaelclockstone commented 2 years ago

The following Unity test code generates a planar triangle strip with 2x3 vertices and 4 triangles. When Calling SimplifyMeshLossless() an empty mesh with zero vertices is created. Expected result: a quad mesh with 4 vertices and two triangles.

SimplifyMesh_ReproduceBug.zip

Whinarn commented 2 years ago

Hi @michaelclockstone ,

Unfortunately I have not spent much (or any) time on the SimplifyMeshLossless method. It's not customizable at all right now. Essentially the only difference between that method and the SimplifyMesh method is that the "lossless" one has a static error threshold while the regular one increases for each iteration. The problem is that this threshold is not customizable through the options at the moment, and to be honest, with default settings the regular simplifier method will start with a much lower threshold.

I can look into adding more customization here. But in the meantime if you'd like to try, you could modify this line with a lower number: https://github.com/Whinarn/UnityMeshSimplifier/blob/3a2c24e5841c63e0c3bc454949d0fa44fb5f86e1/Runtime/MeshSimplifier.cs#L2204

Currently that threshold is 0.001, but I suggest much lower. These values are really for a case-by-case basis.

I still cannot promise you that it will generate the expected result still though. This algorithm is great for fast simplification, but doesn't produce the best quality in some cases, especially with sharp edges like boxes when you simplify it too much. There's also sometimes artifacts that I haven't been able to figure out why they happen. If it's a problem with the original algorithm, or with my port of it.

If you're not faster than me I can run some tests with your provided sample using a lower threshold, when I find the time. Otherwise, please let me know what you figure out.

Thanks!

michaelclockstone commented 2 years ago

I did that very simple test with only 6 triangles because I had problems with other meshes where many triangles (~50% of the entire mesh) were falsly removed completely, so I created this simple test scenario to do debugging. I also tried SimplifyMes(1) or SimplifyMesh(0.5) but the results had the same issue. I am using Unity 2021.2.14

Whinarn commented 2 years ago

I performed some tests, and it is what I still suspected.

I still cannot promise you that it will generate the expected result still though. This algorithm is great for fast simplification, but doesn't produce the best quality in some cases, especially with sharp edges like boxes when you simplify it too much.

This very much applies here. If you debug the algorithm as it calculates the errors, you will notice that every single triangle gets 0 as error, because it's a flat surface. The original algorithm doesn't take borders into account at all. But you cannot simply get away with preserving borders in this case, because you would end up with no simplification at all. While the problem you are trying to solve in your example is very simple and straightforward, you just want to end up with two triangles. This algorithm doesn't really take these cases into account. It works much better with complicated rounded models such as humans or animals. Everything with sharp edges will quickly deteriorate. While there has been attempts to preserve things like curvature, there's still plenty of cases it doesn't deal well with.

My recommendation is to use this for cases where it does work. For all other cases, another algorithm would be needed. If I had to do this all over again, I would have picked another algorithm, honestly. I chose this one because it was fast. But it's lacking in many other ways. Surely it could be modified further to take more cases into account, but I don't find it worth my time.

Since this library just happened to get popular I decided to at least focus my attention to fixing bugs. But unfortunately I'm not interested in expanding the algorithm much further because it's sadly not an area of interest for me. I'm glad that people have found it useful, but I just don't feel like the right person for this kind of project.

So I apologize, but I don't consider this a bug, but the wrong choice of algorithm for your specific problem. But if you do find that the original algorithm deals with this better than my port, then please prove me wrong. But for now I'll close this.

I do accept PRs though, if you decide to dig deeper into this yourself.

And regarding your other problem, I suggest you open up a new issue, if you want me to investigate what the problem might be there. Unless it ties into what I mentioned above. But I'd need sample models in order to investigate that further.

Thanks!

michaelclockstone commented 2 years ago

Thanks for your detailed answer. I can't think of situation though were the SimplifyMeshLossless() would be of any use if it is not even capable of handling the most simple situation were a lossless algorithm how I understand it could improve poly/vertex count. Maybe the name of function is misleading and it's doing something usefuly in situations that I don't understand. In any case it's definitely not lossless if the output can be an empty meshy without any vertices.

Whinarn commented 2 years ago

I totally understand what you mean. And I do agree that it's weird that it ends up just removing everything for something that should be lossless. Kind of ironic. In more normal game meshes it ends up working fine in the cases I have tested it. The idea I think is to remove triangles for flat surfaces, and the problem is that it just doesn't know when to stop. Or that it should prioritize the corners.

While I can think of solutions for dealing with this, it's not generic and would end up breaking other cases.

I have kind of just accepted that it deals with planes and boxes poorly. But perhaps I should also make that more obvious in readme/docs.

I feel like I cannot completely remove the method, since I'm unsure how widely it's being used.