Mirror Modifier: Bisecting along mirrorplane
ClosedPublic
Actions

Authored by Benjamin Sauder (kioku) on Nov 19 2018, 9:50 PM.

Details

Reviewers

Campbell Barton (campbellbarton)

Group Reviewers

Modifiers

Summary

Greetings blenderers,

This patch extends the mirror modifier a bit. It adds an option for bisecting along the mirrorplane, and provides a shortcut to mirror across the world origin. This makes the mirror modifier quite a bit more flexible in various scenarios.

Quick Demo Video:

Im quite new to the C side of things, so bear with me if things are done wrongly.

This patch just hooks up the existing bisect implementation before the mirror operation.

bmesh_bisect_plane.c seems the be built for operater use, and I'm not sure if just shutting these calls off with bm->use_toolflags is the right way to go about - but for ex. bmesh_bevel.c is apparently doing it this way too...

The modifier UI has been a bit restructured because of the added options, I also tried to regroup the uv options in a more sane manner.

Thanks for your time, I know you guys are busy..

Diff Detail

Repository: rB Blender

Event Timeline

Benjamin Sauder (kioku) created this revision.Nov 19 2018, 9:50 PM

Campbell Barton (campbellbarton) mentioned this in rB720368e2d0f2: Cleanup: use const mesh arg to BM_mesh_bm_from_me.Nov 20 2018, 3:31 AM

Fix GCC compiler warnings

Nice functionality, some minor improvements needed.

source/blender/bmesh/tools/bmesh_bisect_plane.c
156 ↗	(On Diff #12573)	Checks for `bm->use_toolflags` shouldn't be needed. Just pass in zero `oflag_center` .. etc.
source/blender/modifiers/intern/MOD_mirror.c
98	Might want this to be user editable, not essential for initial patch.
165	It's possible to avoid building an array and delete immediately using `BM_ITER_MESH_MUTABLE`.

This revision now requires changes to proceed.Nov 20 2018, 3:49 AM

Wo!262 (wo262) awarded a token.Nov 20 2018, 11:48 AM

thank you the review!

ill try to adress this soon.

reverted changes to bmesh_bisect_plane.c
changed to using BM_ITER_MESH_MUTABLE instead of the stack

regarding the bisect distance constant - I guess this will only cause issues if things get really tiny? which probably has other sideeffects through blender as well?

(sorry I have a bit of a hard time with phabricator..)

Campbell Barton (campbellbarton) mentioned this in rB19875439b5fb: Modifier: Mirror learns to bisect on plane.Nov 21 2018, 12:03 AM

Thanks for the updates, committed rB19875439b5fb2b7c569bc95f384b5be548a5e45f w/ edits.

Remove world option, it's not related to bisect and we generally avoid relating modifiers to the world coords because it's not useful when instancing groups for eg (if you used this for a character for eg, it would work when at zero location but not when instanced as a collection and moved about).

Fixed a leak (the bisected mesh copy wasn't being freed).

Simplified flip-plane for-loop (flip the plane instead of using two planes w/ a conditional).

This revision is now accepted and ready to land.Nov 21 2018, 12:06 AM

Campbell Barton (campbellbarton) closed this revision.Nov 21 2018, 12:07 AM

May I recommend that this feature should be tested on higher resolution meshes against the unmodified mirror modifier to see what the performance penalty is?

@kursad k (kursadk) - the performance penalty will be quite high, similar to mesh wire-frame or solidify.

In practice this is hard to avoid since cutting a mesh needs a data-structure that supports more advanced operations (a single face cut cold result in many faces for example).

That is undestandable. I will try testing it once I am able to setup building Blender on my new desktop. I am just hoping that the addition wont make the existing modifier slower when the new bisect feature is not enabled, ideally it should but I just want to make sure.

Nahuel Belich (belich) awarded a token.Nov 21 2018, 1:53 AM

@kursad k (kursadk) performance without bisect hasn't changed.

Duarte Farrajota Ramos (duarteframos) awarded a token.Nov 21 2018, 4:21 AM

@Campbell Barton (campbellbarton) thank you for your time updating this patch! learned a thing or two from your changes - I know you have a lot on your plate, so this is really appreciated.

regarding the world axis option, I have a background in doing exactly the type of work you describe, so I was a bit biased on this... but nevermind it doesn't really matter as working with an empty as the 'world-origin' is also straight forward to work with.

@kursad k (kursadk) it doesnt get called if you don't enable it. The trick is do use it before all subdividing operations in the stack.