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

NicholasBishop at gmail.com NicholasBishop at gmail.com
Sat Apr 28 05:24:26 CEST 2012


Thanks for reviewing :) New patch attached, comments below:

> Tried on this file and it gives a problem at the top monkeys head. The
problem
> happens when triangulated too.

> http://www.graphicall.org/ftp/ideasman42/2_monkey_head_error.blend
Fixed now, along with a lot of corner cases. The hull code has gotten
quite a bit longer, but is much more robust.

> Know this is not really the purpose but it works quite well as a
bridge tool for
> convex edge loops, could consider..
> - copying UV's and materials from connected faces
Hull output now searches for an adjacent face to use as an example, so
materials etc. get copied. Wasn't sure about UVs; since these are new
faces do I need to do anything there?

> - removing selected faces (so as to create a tube between 2 surfaces).
Added new "make holes" option that does this.

> - 2 triangles between 2 edges make into quads.
> ... Or we could just make our bridge tool better..
Added an optional call to the join_triangles bmop. Gave the hull
operator all of the join_triangles properties; UI is a bit cluttered but
output looks good.


http://codereview.appspot.com/6114060/diff/1/source/blender/bmesh/operators/bmo_hull.c#newcode50
> source/blender/bmesh/operators/bmo_hull.c:50: t =
> MEM_callocN(sizeof(HullTriangle), "HullTriangle");
> (while performance probably isn't horrible) - could use mempool here.
Done.


http://codereview.appspot.com/6114060/diff/1/source/blender/bmesh/operators/bmo_hull.c#newcode176
> source/blender/bmesh/operators/bmo_hull.c:176: const int
hull_flag_interior_vert
> = 1;
> trivial - but why not use enum?
> (thats the convention at least defines or enum, though have been using
enums
> more - recently)
Done (dunno why I didn't do that in the first place...)


http://codereview.appspot.com/6114060/diff/1/source/blender/editors/mesh/editmesh_tools.c#newcode4303
> source/blender/editors/mesh/editmesh_tools.c:4303:
EDBM_update_generic(C, em,
> TRUE);
> need to call selection flush here too, running this in face mode
leaves them
> unselected.
Added a call to EDBM_selectmode_flush(em), is that correct?

Thanks again for reviewing.

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


More information about the Bf-codereview mailing list