[Bf-codereview] Add BMesh and WM symmetrize operators (issue 6618059)

brechtvanlommel at gmail.com brechtvanlommel at gmail.com
Fri Oct 12 12:47:24 CEST 2012


This is really quite cool, I was expecting a simple vertex coordinates
symmetrize but it can actually mirror topology! Worked well in testing.

Some comments below.


https://codereview.appspot.com/6618059/diff/1/source/blender/bmesh/intern/bmesh_opdefines.c
File source/blender/bmesh/intern/bmesh_opdefines.c (right):

https://codereview.appspot.com/6618059/diff/1/source/blender/bmesh/intern/bmesh_opdefines.c#newcode1188
source/blender/bmesh/intern/bmesh_opdefines.c:1188: * Description is
TODO
Guess this todo and the one a few lines lower is not difficult to fix?

https://codereview.appspot.com/6618059/diff/1/source/blender/bmesh/operators/bmo_symmetrize.c
File source/blender/bmesh/operators/bmo_symmetrize.c (right):

https://codereview.appspot.com/6618059/diff/1/source/blender/bmesh/operators/bmo_symmetrize.c#newcode38
source/blender/bmesh/operators/bmo_symmetrize.c:38: #define
SYMM_VERT_THRESHOLD 0.001f
I think these values are quite high, mirror for transform and particles
uses 0.00002f (meshtools.c MOC_THRESH). For Sintel's head model I
remember Campbell set it to this low value because it didn't work with
small detail. Not sure if it should be user adjustable but probably it
should be smaller?

https://codereview.appspot.com/6618059/diff/1/source/blender/bmesh/operators/bmo_symmetrize.c#newcode174
source/blender/bmesh/operators/bmo_symmetrize.c:174: BLI_assert(r);
I have a feeling there is a corner case where r = 0 and lambda is used
uninitialized. I didn't try to recreate it, but it probably exists.

https://codereview.appspot.com/6618059/diff/1/source/blender/bmesh/operators/bmo_symmetrize.c#newcode196
source/blender/bmesh/operators/bmo_symmetrize.c:196: if
(BLI_ghash_haskey(symm->vert_symm_map, e->v1))
This could call ghash_lookup without ghash_haskey first, will just
return NULL.

https://codereview.appspot.com/6618059/diff/1/source/blender/editors/mesh/editmesh_tools.c
File source/blender/editors/mesh/editmesh_tools.c (right):

https://codereview.appspot.com/6618059/diff/1/source/blender/editors/mesh/editmesh_tools.c#newcode5444
source/blender/editors/mesh/editmesh_tools.c:5444: ot->prop =
RNA_def_enum(ot->srna, "direction", direction_items, 0,
It would be slightly more clear if 0 is replaced with
BMO_SYMMETRIZE_NEGATIVE_X here.

https://codereview.appspot.com/6618059/


More information about the Bf-codereview mailing list