Page MenuHome

Mesh Edit: preserve Custom Normal vectors in topology operators.
Needs RevisionPublic

Authored by Alexander Gavrilov (angavrilov) on Jun 2 2019, 12:28 PM.

Details

Summary

Custom Loop Normals are normally encoded relative to the default
normals, similar to normal maps, allowing them to naturally follow
mesh deformations. Changes to mesh topology however often result
in nonsensical effects that are not desired.

The Remove Doubles operation especially (now known as Merge By
Distance) is intended as a purely topological operation, and
definitely should not change the vector of the custom normals.

This patch implements that behavior by converting the relative
encoding into an absolute vector layer for the duration of the
operation. It also modifies other Merge types in this way for
consistency, the Rip operator as their inverse counterpart;
and also Dissolve, Connect Path and Knife operators as other
examples more related to topology than shape.

Custom Normals: when forcing smooth fan clnors together, average in 3D.

It seems the code is designed to force all custom normals associated
with a smooth fan to be the same. Not touching on that design decision,
this fixes the code to average in 3D space, because averaging normals
in polar coordinates produces weird results.

Diff Detail

Repository
rB Blender
Branch
keep-clnor-vectors (branched from master)
Build Status
Buildable 3794
Build 3794: arc lint + arc unit

Event Timeline

Bastien Montagne (mont29) requested changes to this revision.Jun 3 2019, 5:23 PM

Custom Normals: when forcing smooth fan clnors together, average in 3D.
It seems the code is designed to force all custom normals associated
with a smooth fan to be the same. Not touching on that design decision,
this fixes the code to average in 3D space, because averaging normals
in polar coordinates produces weird results.

I wonder whether your solution is really better, since you evaluate the clnors using another clnor_space than the one used to create them. Did you actually found a real difference (as in, worse/better) in results here?
Further more, that averaging of 'polar' clnors values is expected to be used only to solve very small divergences due to numerical errors or so, not to fix widely different clnors - this would be a bug and not an expected behavior, as you said, all clnors in a same clnor_fan should have same value, since it defines a single normal… Did you actually encounter such case with existing code?

In fact, I have the feeling that you are using that code in a weird unexpected way, BM_custom_loop_normals_from_vector_layer() seems way to simple to me, think you should check what mesh_normals_loop_custom_set() is doing. It first computes existing clnors fans, then detects the ones that are no more valid and need splitting (because the custom normals in it are not matching anymore), sets relevant edges as sharp to create new fans as needed, then recomputes again all clnor_space fans, and finally sets the new custom normals… That’s how the few modifiers supporting custom normals handle topology changes currently, e.g.

Note that that patch would need to wait for after 2.80 release anyway…

This revision now requires changes to proceed.Jun 3 2019, 5:23 PM

I'm not sure mesh editing operations that are not in any way related to shading should suddenly start setting edges to sharp (there are arguments both way, especially for remove doubles; could be an option, or maybe differ per operator?), so I let it assign 'incorrect' normals and rely on that code to average them out, but in a sensible way.