Decide future of "mesh-smooth" component
diarmidmackenzie opened this issue · 3 comments
We've been reviewing all the components in this repo, and retiring a few that no longer seem valuable - see #395.
It's not clear what to do with 'mesh-smooth'.
As per this comment, it seems to still be compatible with the latest three.js, but we have no examples showing what it does, and from my testing I've not been able to get it to do much of any use.
When applied to a gltf, it seems to either have no effect, or (in one case) make the whole mesh go black with no materials - not very helpful. See this glitch example.
Looking back to when it was introduced, it seems it was primarily tested with json models.
#165
The original proposal was a parameter on the json-model loader.
#163
That PR was rejected, and instead a generic component was proposed on the basis that "this would work for any file format, too". But is it actually useful for any other fomat?
The JSON model loader has subsequently been removed
Only pause for hesitation before deleting the 'mesh-smooth' component is this comment from @donmccurdy at the end of #163 "Actually I just ran into a use case for this with a glTF model, too." - though no detail on what this use case was...
It's such a simple component, I don't see any real loss from removing it, and I'd rather not clutter the repo with stuff where we don't understand the purpose or have any working examples. If that causes anyone any problems, they can explain their use case & we can re-instate.
@vincentfretin - I propose we remove it as per your previous suggestion. If you're OK with that I'll do a PR.
Let's remove it for 7.0.0. If someone depends on this component, they can copy the previous code in their project or contribute a better one as a separate repository.
Just some background — Knowing more now than I did then, it's true that the mesh-smooth component is format-agnostic. It doesn't generalize to all (most?) models though, because it relies on an assumption that vertices have already been welded. A somewhat more robust implementation would:
- remove existing normals
- weld vertices with some error tolerance
- compute vertex normals as before
That's a more expensive set of steps, and will still fail if vertices on adjacent faces have UV seams or vertex colors that prevent welding.
tl;dr — I think it's totally reasonable to remove the component, and I support that and any other reorganization you all choose to do here!
Thanks @donmccurdy for your feedback!