[Bf-codereview] Convex hull bmesh operator (issue 6114060)

ideasman42 at gmail.com ideasman42 at gmail.com
Sat Apr 28 12:59:34 CEST 2012


Some more feedback,

If you want to copy loop data from adjacent faces call this after
creating each face.
  - BM_face_copy_shared()

Not certain this will always give good results though I think it the
case of typical bridge usage it would be good.


http://codereview.appspot.com/6114060/diff/10001/source/blender/bmesh/operators/bmo_hull.c
File source/blender/bmesh/operators/bmo_hull.c (right):

http://codereview.appspot.com/6114060/diff/10001/source/blender/bmesh/operators/bmo_hull.c#newcode105
source/blender/bmesh/operators/bmo_hull.c:105: new =
MEM_callocN(sizeof(HullBoundaryEdge), "HullBoundaryEdge");
another use for mempool? didnt check how often this runs but expect a
fair bit.

http://codereview.appspot.com/6114060/diff/10001/source/blender/bmesh/operators/bmo_hull.c#newcode278
source/blender/bmesh/operators/bmo_hull.c:278: if (v1 > v2)
expect you intend this - but could add comment...

/* verts can be in any order, just pick lower pointer for consistency */

... since the first vert may be after the last vert (when comparing
pointer addresses), if you dont mean this, then better compare index
values.

http://codereview.appspot.com/6114060/diff/10001/source/blender/bmesh/operators/bmo_hull.c#newcode386
source/blender/bmesh/operators/bmo_hull.c:386:
could use INIT_MINMAX & DO_MINMAX here? - think its more
straightforward.

http://codereview.appspot.com/6114060/diff/10001/source/blender/bmesh/operators/bmo_hull.c#newcode512
source/blender/bmesh/operators/bmo_hull.c:512: if (BMO_slot_bool_get(op,
"make_holes"))
better not do string lookups in a loop, this can be passed as an arg to
the function (then no need to pass 'op')

http://codereview.appspot.com/6114060/


More information about the Bf-codereview mailing list