Page MenuHome

Fix T57852: Mesh X Mirror option not working
AbandonedPublic

Authored by Philipp Oeser (lichtwerk) on Oct 31 2018, 5:23 PM.

Diff Detail

Repository
rB Blender
Branch
xmirror (branched from blender2.8)
Build Status
Buildable 2348
Build 2348: arc lint + arc unit

Event Timeline

reason was that relevant code was never called [TransformContainer->is_active was never true]
that was corrected but actually I see ne reason to only do it for the active object?

also found these TODOs (multi object editing)

/* TODO(campbell): xform, get mirror from each object. */
/* TODO(campbell): xform: We need support for many mirror objects at once! */

and tried to address these as well

now it checks each participating mesh (not just the active objects) for the setting and if found on any will enable mirror.
later this will be checked again for each container.

@Campbell Barton (campbellbarton) : not sure if you had something more involved in mind?

First we should consider how this should behave.

However this is done AFAICS it's rather awkward.

We could go with this: If any of the meshes have mirror enabled, transform will mirrored.

This is reasonable since meshes are mirrored or not, it doesn't make sense to mirror all.

OTOH - it does mean users may do some operation without being sure if one of the many objects is mirrored (simply moving all vertices could mess things up in this case).

So we *might* want a top-level tool setting for this that overrides each meshes options, but don't think this is needed just now.

Regarding this patch, had a look at this and have some suggestions:

  • Move mirror option to TransDataContainer (no need for spesific inline checks like - ((Mesh *)objects[i]->data)->editflag & ME_EDIT_MIRROR_X
  • Then we should be able to avoid BKE_view_layer_array_from_objects_in_edit_mode_unique_data loop which has some potential to get out of sync with transform objects.
  • Don't check the active object (if you do need to for some reason - it's always first in the array).

@Campbell Barton (campbellbarton) : thx for having a look

Ultimately this should be a top-level tool setting: +1

I wasnt really sure what you meant:

However this is done AFAICS it's rather awkward. We could go with this: If any of the meshes have mirror enabled, transform will mirrored. This is reasonable since meshes are mirrored or not, it doesn't make sense to mirror all.

This sounds like the current behavior of this patch:

  • generally enable mirror if any of the meshes have mirror enabled [t->flag |= T_MIRROR; t->mirror = 1;]
  • check again each individual mesh (or TransformContainer) and respect the setting (it has downsides, yes, but I think the pros outweight the cons here)

regarding suggestions:

  • Move mirror option to TransDataContainer +1
  • avoid BKE_view_layer_array_from_objects_in_edit_mode_unique_data: will check again
  • Don't check the active object +1 [thats what I meant by the code comment " 'is_active' is practically unused anywhere - could be removed even? "]

If OK, I will update regarding suggestions...

  • moved mirror options into TransDataContainer
  • dont check active object
  • avoid BKE_view_layer_array_from_objects_in_edit_mode_unique_data
Philipp Oeser (lichtwerk) retitled this revision from Fix (unreported) Mesh X Mirror option not working to Fix T57852: Mesh X Mirror option not working.Dec 14 2018, 3:28 PM
Philipp Oeser (lichtwerk) planned changes to this revision.Dec 18 2018, 9:27 AM
  • will have to respect "mirror" arg for transform macro operators again (broke with last version if the patch)
  • investigate why in 2.8 bpy.ops.transform.translate(value=(0, 0.680408, 0), constraint_axis=(False, True, False), constraint_matrix=(1, 0, 0, 0, 1, 0, 0, 0, 1), constraint_orientation='GLOBAL', mirror=True, proportional='DISABLED', proportional_edit_falloff='SMOOTH', proportional_size=1) throws error
  • investigate why generally (also in 2.79) bpy.ops.transform.translate "mirror" option has no effect really [also we never show this in the redo UI]?
  • investigate if (for multi-object-editing) we could show mesh options as being "shared/multiple" (can be confusing)

So, after thinking about this more, my conclusion is that I think the X Mirror option should be moved away from the mesh datablock and into the mode. Here's why:

  • To the user it is really a tool setting, and we display it as such. We don't store tool settings per object
  • It makes it consistent with other parts of Blender. In the other modes where we have Mirror options (eg Sculpt Mode, Texture Paint etc) we store these in the mode, not per mesh
  • This makes it possible to support multi-object Edit Mode for this feature, which becomes a mess if it's stored per mesh
  • If you have 100 objects, you avoid having to enable X Mirror 100 times.

For these reasons I think we should move this to the mode, for simplicity, consistency and practicality. Then later we would also add Y Mirror and Z Mirror to make it fully consistent with the other modes.

So, after thinking about this more, my conclusion is that I think the X Mirror option should be moved away from the mesh datablock and into the mode. Here's why:

  • To the user it is really a tool setting, and we display it as such. We don't store tool settings per object
  • It makes it consistent with other parts of Blender. In the other modes where we have Mirror options (eg Sculpt Mode, Texture Paint etc) we store these in the mode, not per mesh
  • This makes it possible to support multi-object Edit Mode for this feature, which becomes a mess if it's stored per mesh
  • If you have 100 objects, you avoid having to enable X Mirror 100 times.

For these reasons I think we should move this to the mode, for simplicity, consistency and practicality. Then later we would also add Y Mirror and Z Mirror to make it fully consistent with the other modes.

If you weight paint and forget to enable the symmetry option, it's annoying to have to manually fix this up (even though we have some tools to flip weights).

If you accidentally perform edits on a mesh that isn't symmetrical - you can move vertices you didn't intend to.

In both cases you risk making edits and saving - without realizing you're performing edits that are/aren't impacting the opposite side.

From a tool-design perspective it's much neater to make this a single option, but from a user perspective the content you're editing is either symmetrical or not.

Would like to get feedback from artists if having this as a tool setting would help.


Note, there was a proposal to show mirror status while transforming - this would help avoid accidentally having the wrong setting. Although mirror option also impacts non-transform tools, so it's not a complete solution.

  • If you have 100 objects, you avoid having to enable X Mirror 100 times.

you can ALT+click the option when having multiple objects selected, this will be respected for all objects then...

  • If you have 100 objects, you avoid having to enable X Mirror 100 times.

you can ALT+click the option when having multiple objects selected, this will be respected for all objects then...

Or (more discoverable) RMB-Copy-to-selected.

Although normally setting a mesh as symmetrical is something you set early on when creating the mesh. Not a setting you toggle for all objects (there are cases you might want to change it for many objects, but think this is very rare - eg, loaded in 100 symmetrical characters, if you do this you might already be using scripts to automate things - so changing a single setting holding alt or via RMB menu isn't a bottleneck).

If you weight paint and forget to enable the symmetry option, it's annoying to have to manually fix this up (even though we have some tools to flip weights).
If you accidentally perform edits on a mesh that isn't symmetrical - you can move vertices you didn't intend to.

Sure, but the reverse could be true too. You've enabled X Mirror and switch to a different object and expect that it still applies. But it's now been turned off because it's a different object. And because it's inconsistent, users wouldn't expect it to be per object, leading to mistakes. Better to keep it simple, predictable and consistent. Do you also want to move Sculpt Mode tool settings to be per object? I think not.

If we were to follow the logic of the per-object X-mirror tool setting to its conclusion, *all* tool settings like snapping, orientation etc should be per object. It seems arbitrary that only the X Mirror in Edit Mode (Not paint or sculpt) is per object.

Would like to get feedback from artists if having this as a tool setting would help.

maybe @Julien Kaspar (JulienKaspar) would like to join the conversation? [also left a note in T57852 for anyone interested to post opinions here -- if that helps...]

@William Reynish (billreynish)
In my experience I find it very annoying with the symmetry options set to the mode and not the objects.
I pretty much always have objects that I sculpt on that I want symmetry to be active on and others where I need to disable it again.
This is a constant back & forth of turning a setting on /off.
But even if it's a per mode setting, it's very easy to not notice that any of these options are on or off and like @Campbell Barton (campbellbarton) said:

In both cases you risk making edits and saving - without realizing you're performing edits that are/aren't impacting the opposite side.
From a tool-design perspective it's much neater to make this a single option, but from a user perspective the content you're editing is either symmetrical or not.

I think the per mode setting is easier to understand and work with. There is some unpredictability in the per object way. I currently prefer the per object way because it removes this constant on/off toggling.
To go off topic a bit: I think with these issues fixed I would be fine with per mode settings:

  • It's not obvious enough that the options are enabled or disabled in the interface.
  • Because of this: Usually you notice it too late if the settings are not as intended.
  • Changing the settings requires to go off to the side or corner of the window and toggle specific options. Often this evolves many clicks for opening/switching tabs and opening popups.

It's important for me that it's obvious if options like Symmetry, X-mirror and also other settings like AutoMerge Editing are enabled or disabled.
Currently if the Tool Settings are not open on the Properties Editor on the side, these settings are hidden in a popup menu in the corner of the window.
These options can be very destructive to the mesh so they should be clearly shown. Icons on the Top Bar or some indicators in the 3D Viewport would fix this.
Also the user can't add shortcuts to these settings. Quick Favorites is one solution but shortcuts can be the fastest way to toggle these settings.

Yes, I think that is a fair solution. We could add an indicator when X, Y or Z Mirror is enabled in the viewport in any mode that supports symmetry - much like we do when Auto-Key is enabled, inside Object or Pose Mode.

lucas veber (lucky3) added a comment.EditedJan 10 2019, 5:39 PM

Since you ask for feedback from artists, I'd support the per-object X-Mirror, as Julien Kaspar explained when working with several objects in the scene, it's the easiest way to keep it enabled for some objects and disabled for others. It may seem counter-intuitive but actually it's way more useful. I'm a 3dsMax and Maya user too, and I still prefer the way it's implemented in Blender.

This argument could be made for literally any tool setting. You might want to use different orientations when transforming different objects, or use different pivots, etc. Having this inconsistent behaviour for one specific tool setting in only one specific mode is not good practice.

This argument could be made for literally any tool setting.

I have to disagree sorry. Enabling or disabling symmetry on meshes is something different, you generally keep it enabled or disabled from the start to the end.
But when modeling, it's often necessary to change many times the pivot point mode, proportional edit tool size, snap...

If you are animating, you could certainly argue that it could be useful to use a certain pivot or orientation with a certain object or bone. You might also want to use a certain transform tool with certain bones always. You don't ever want to move a wheel, but you always want to rotate it. So X-Mirror is no different. It's just a tool setting.

In any case, it most certainly makes no logical sense that the mirror option is per object in Edit Mode, and a normal tool setting in Sculpt Mode.

We should handle this in a way that makes it predictable and consistent. These kinds of exceptions and weirdness builds up over time.

lucas veber (lucky3) added a comment.EditedJan 10 2019, 6:47 PM

If you are animating, you could certainly argue that it could be useful to use a certain pivot or orientation with a certain object or bone. You might also want to use a certain transform tool with certain bones always. You don't ever want to move a wheel, but you always want to rotate it. So X-Mirror is no different. It's just a tool setting.

Not really, at least from my experience when animating I often change the pivot mode.
About the wheel example, the transform locks are used to this end (e.g. the translation channels can be locked).
Why not starting a poll thread on Blender Artists? It's probably the best way to know what the majority of users think about it.

If you are animating, you could certainly argue that it could be useful to use a certain pivot or orientation with a certain object or bone. You might also want to use a certain transform tool with certain bones always. You don't ever want to move a wheel, but you always want to rotate it. So X-Mirror is no different. It's just a tool setting.

This relates to the content of the model in a way other tool settings don't.

It's not unusual to create a mesh which you intend to be symmetrical, where every future edit should be applied symmetrically - and failing to do so causes you to have to manually fix it.

Or the reverse, accidentally leaving symmetry options on for a mesh that you never wan't to apply symmetrical edits too.

In any case, it most certainly makes no logical sense that the mirror option is per object in Edit Mode, and a normal tool setting in Sculpt Mode.
We should handle this in a way that makes it predictable and consistent. These kinds of exceptions and weirdness builds up over time.

+1, but this is only an argument for consistency, we could move them out of the tool-settings too.

Note, I suspect this isn't such an issue for sculpting because people tend to have 1 high poly sculpt model per file, so they aren't switching between objects as often - where they would run into the problem with symmetry options being applied differently, as you would for a low/medium-poly file that contains many objects.

I agree that it can be hard to tell whether it's enabled or not (and as Julien mentioned, highly destructive), but I think a visualisation of the setting would solve that issue, and result in a more consistent design.
If anything, having it per-object (as it is right now in 2.79) often results in messed up geometry for me, as I move from a mesh with it disabled to one where it's enabled, and I don't immediately notice.
A transparent line or plane on the mirror point is a pretty traditional way of visually presenting a mirror, but a cursor indicator (faint x, y or z?) could work as well. I personally prefer a line superimposed on the mesh over a transparent plane, mostly because it's less distracting.
Another option, potentially on top of the previous ones: having the cursor location visually mirrored on the other side as a dot.

@Campbell Barton (campbellbarton) As an extra note on your note:

Note, I suspect this isn't such an issue for sculpting because people tend to have 1 high poly sculpt model per file, so they aren't switching between objects as often - where they would run into the problem with symmetry options being applied differently, as you would for a low/medium-poly file that contains many objects.

That's actually not the case. Because of a real lack of transform and masking options/tools in Sculpt Mode it's very much necessary and/or more comfortable to keep the model as many separate objects.
Only once it's done is it usually boolean-ed together with some more multires sculpting.
But even then there are often multiple parts for the model unless it's a really simple prop/character.

Btw: It's maybe also worth noting that dyntopo is a per object settings, because it absolutely needs to be. So we can't make everything consistent anyway.

Sculpt Mode and Edit Mode are just two different ways of altering your mesh. Sculpt Mode can be used on objects inside scenes with unlimited objects, and users may jump from one mode to the other regularly while modeling.

Both Sculpt Mode and Edit Mode can be used for both high res and low res meshes - there’s no delineation there.

There’s no logical reason why symmetry is a mode-wide tool setting in Sculpt Mode, while being this inconsistent per-object tool setting in Edit Mode. They really should work exactly the same. At some point we should also add Z and Y symmetry to Edit Mode to make it truly consistent with Sculpt Mode.

@William Reynish (billreynish)
What about the mirror options in pose mode and armature edit mode? I just asked around at the studio and having these mode specific can be catastrophic for example rigging.
Having this happen accidentally is something that needs a lot of time to fix it or can even completely ruins the work if it happens unnoticed.

The general feedback was that this becomes easier to manage when it's object specific since you know what you set it as. If you have to switch and manage it constantly then you need to check more often if it is on/off and mistakes can happen more frequently.

I think there's a general problem in the right corner of the top bar that there are going to be options that are object specific and others that are mode specific.
They cannot all be unified since this is definitely going to hurt the workflows of certain people.

@Julien Kaspar (JulienKaspar):

The reverse is also true. If you enable Symmetry, and switch to a different object, you'll accidentally then start to manipulate it without Symmetry, because the tool setting was forgotten, because it is now a different mesh. That is equally as annoying. This is my point: There are cases where you may want to use it only with certain objects, and there are other cases where you won't. And that applies to any tool setting.

If Symmetry is really going to be per object, we should do so consistently in all modes, and display it in the ObData Properties.

But, the most consistent solution is simply to make it a mode-wide tool setting, and make it work the same in Edit Mode, Sculpt Mode and the paint modes. There's no difference in use-case, and no logical reason why they should work differently in the various modes.

@William Reynish (billreynish)
I think I'm fine with either way. But just a heads up:
First address these issues:

It's important for me that it's obvious if options like Symmetry, X-mirror and also other settings like AutoMerge Editing are enabled or disabled.
Currently if the Tool Settings are not open on the Properties Editor on the side, these settings are hidden in a popup menu in the corner of the window.
These options can be very destructive to the mesh so they should be clearly shown. Icons on the Top Bar or some indicators in the 3D Viewport would fix this.
Also the user can't add shortcuts to these settings. Quick Favorites is one solution but shortcuts can be the fastest way to toggle these settings.

Then make the switch in behaviour. Otherwise people are going to make these mirror/symmetry mistakes constantly and they are going to lose it :D

Yes I think that’s fair - we could do this consistently when symmetry is enabled in any mode.

Sculpt Mode and Edit Mode are just two different ways of altering your mesh. Sculpt Mode can be used on objects inside scenes with unlimited objects, and users may jump from one mode to the other regularly while modeling.
Both Sculpt Mode and Edit Mode can be used for both high res and low res meshes - there’s no delineation there.
There’s no logical reason why symmetry is a mode-wide tool setting in Sculpt Mode, while being this inconsistent per-object tool setting in Edit Mode. They really should work exactly the same. At some point we should also add Z and Y symmetry to Edit Mode to make it truly consistent with Sculpt Mode.

I was only pointing out why having symmetry setting shared may not have come up as an annoyance for sculpting common workflows, in a way it might for weight painting for eg - not justifying the difference or arguing it should be kept this way.

@Julien Kaspar (JulienKaspar):
The reverse is also true. If you enable Symmetry, and switch to a different object, you'll accidentally then start to manipulate it without Symmetry, because the tool setting was forgotten, because it is now a different mesh. That is equally as annoying. This is my point: There are cases where you may want to use it only with certain objects, and there are other cases where you won't. And that applies to any tool setting.

This is a false equivalence, pick any arbitrary object in a scene and edit it. The chance that you want to change the symmetry setting from the previous time you edited it is low.

You keep ignoring the fact that there are pros and cons either way in this case - in a way thats different to other tool settings.

If for example we have XYZ mirroring, one object is mirrored on the X axis, one on the Y... it's going to be annoying to have to remember to change this each time you make edits. And the chance you want to use the current setting on the other object is near zero.

If Symmetry is really going to be per object, we should do so consistently in all modes, and display it in the ObData

This seems reasonable.

But, the most consistent solution is simply to make it a mode-wide tool setting, and make it work the same in Edit Mode, Sculpt Mode and the paint modes. There's no difference in use-case, and no logical reason why they should work differently in the various modes.

You're valuing perceived consistency above typical workflows and down sides of making users have to re-consider these options when they switch between editing different objects.

You're also ignoring that this is an unpopular suggestion (based on the poll ~ which I take with a grain of salt :) ), as if everyone who is skeptical of making this a tool option just doesn't like change and no other reason.

@Julien Kaspar (JulienKaspar):
The reverse is also true. If you enable Symmetry, and switch to a different object, you'll accidentally then start to manipulate it without Symmetry, because the tool setting was forgotten, because it is now a different mesh. That is equally as annoying. This is my point: There are cases where you may want to use it only with certain objects, and there are other cases where you won't. And that applies to any tool setting.
If Symmetry is really going to be per object, we should do so consistently in all modes, and display it in the ObData Properties.
But, the most consistent solution is simply to make it a mode-wide tool setting, and make it work the same in Edit Mode, Sculpt Mode and the paint modes. There's no difference in use-case, and no logical reason why they should work differently in the various modes.

As a user who have used the symmetry for a long time I can ensure, Symmetry should, with no doubt be per-object and NEVER a global setting since some objects can or not be symmetric and a complete model generally needs both.
But I agree with unifying the the edit mesh settings with sculpt settings (Making sculpt settings per-object).

And also, I find really annoying the fact that sculpt symmetry is a global setting, it really should be per object.

An example is when you are sculpting a character and want to block the hair, so you have to disable symmetry then you come back to the face and forget that symmetry is disabled, with the concentration in the sculpt, you dont notice that you have sculpted only half of the face and have to do it again, argh!

But I agree with unifying the the edit mesh settings with sculpt settings (Making sculpt settings per-object).

I think this have to be deeply thought.

You can use radial symmetry for sculpting with an odd number of strokes.
Current symmetry in sculpt mode may make sense per brush stroke. There is an ability of an occasional symmetric operation on a model that is not intended to be symmetrical.
That is the whole point of Topology Mirror option.
Topology is similar on left and right but mesh is not supposed to be globally symmetrical.

We want to exploit the ability to sculpt on several objects with 2.8.
What does multi-editing imply in terms of symmetry ?
There should be be ability to make occasional symmetric operation on a selection of multiple objects that is not intended to be symmetrical.
There should have ability to define an axe or center of symmetry that is common to selection, global scene origin or 3Dcursor, or origin of active object or a custom and specified center (numerical values, object name).

If operation is made by object, it makes sense to have result per object.
If operation is made on a selection that is expected to be treated as a whole, it makes sense to have a global result.
I think that both situations should be supported. And it means 2 settings.

I hope that an unification would not impoverish Blender.
Personally, I would be happy if it means radial symmetry while working in edit mode. That may be funny.
But if we loose radial symmetry, this evolution would just have been stupid.

Radial may be quite hard to implement on edit mode objects are rarely radially symmetric.
We don't need to unify everything, only the axial symmetry, radial symmetry is a sculpt specific feature.

And about how to deal with multiple objects, we could compute the symmetry operations per-object, so on sculpt mode, if the stroke passes through a mesh with symmetry it would be computed locally as if it there were just this object selected, and we can do it for every object independently, both in sculpt and edit mode.

Brecht Van Lommel (brecht) requested changes to this revision.Jan 14 2019, 4:22 AM

My preference is for this to be a mesh setting. But the UI can be improved to clarify that.

At this point I think it's important to commit a fix for this bug for the basic case, since it's one of the most reported broken things in 2.8. Let's go ahead with getting this patch committed as is. In a second step we can do design changes and UI improvements, whatever is decided.

source/blender/editors/transform/transform.c
2132 ↗(On Diff #12902)

If we have multiple objects with each their own setting, this operator property doesn't work so well anymore.

We could keep it, behaving like this:

  • If the properties is set (RNA_property_is_set), it overrides the mesh setting on all meshes. If it is not set, the mesh setting is followed.
  • The operator never writes its value, only reads it.

Committed with edits. rB65b82e09d94e77f7d8fb079f24fe82aee8e2509e

"mirror" is now only used as an override that disables.

  • When enabled, read from the mesh x-mirror option.
  • When disabled, never mirror.

We were only ever using this for tools like extrude that need to temporarily disable mirror when transforming, so loosing the ability to force-enable mirroring isn't a problem.

You're valuing perceived consistency above typical workflows and down sides of making users have to re-consider these options when they switch between editing different objects.

I don't want to stir things up, but I really think this is what's happening here. While acknowledging the amount and quality of work that @William Reynish (billreynish) is putting in, I think the approach may be too often driven by desire of consistency when really sometimes established workflows and ease of use should have priority. This has impaired development before and I think there should be some concessions made. All in all @William Reynish (billreynish) included you're all doing tremendous work so no hard feelings at all.

@Hadrien Brissaud (hadrien) would rather criticism be kept constructive and not personal since most decisions are agreed on by more than one individual.

Please post in the user-feedback section of devtalk and list your concerns in detail about changes made for consistency: https://devtalk.blender.org (post the link here for reference).