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

[NicholasBishop at gmail.com]

On 2012/04/29 06:13:32, ideasman42 wrote:
> On 2012/04/28 21:18:42, nicholasbishop wrote:
> > On 2012/04/28 20:36:46, ideasman42 wrote:
> > > LGTM,
> > >
> > > one more general comment is that I think having the operator
delete the
> holes
> > > its self might not be so good.
> > >
> > > Instead the blender operator could do this on faces tagged as
original. -
> This
> > > isnt really bad, just think it can get a bit messy if a lot of
operators
> call
> > > eachother because of functionality users might want. rather have
them low
> > level
> > > and the callers can mix together the operators that make sense in
some
> > context.
> > > In this case the caller is the wmOperator
> >
> > Before, I was doing deletion in the wmOperator, but the code got a
bit more
> > complex than just deleting original faces -- all the code in
> > hull_delete_unused() and hull_make_holes(). Agree it gets trickier
to combine
> > operators now, but on the other hand  the hull operator has flags to
disable
> > these operations too.
> >
> > So, not sure which option is best:
> > a) In the hull bmop, find elements to be deleted and tag them in an
output
> slot,
> > then actually delete in the wm op
> > b) Move both the find-deleteable-elements and deletion to the wm op
> > c) Leave both in the bmop and let other operators disable with slot
booleans

> Leave this up to you, but some things to consider.

> I Don't think the way its doing it now is horrible, off hand (a) looks
most in
> keeping with bmop design, but if for some reasons there are
complications then
> suggest stick with what you have (c).

> b) may be more trouble then its worth, but if it could be done cleanly
then I'd
> consider it - rip tool does this for eg.

Implemented option (a), committed to r46080 and r46081.

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