Details

Reviewers

Brecht Van Lommel (brecht)
Germano Cavalcante (mano-wii)
Campbell Barton (campbellbarton)

Maniphest Tasks

T79785: Make XYZ symmetry a property of Objects

Commits

rB5502517c3c12: Unify all XYZ symmetry options using Mesh Symmetry

Summary

This adds XYZ symmetry as a property of meshes and updates all modes to
use the mesh symmetry by default to have a consistent tool behavior
between all modes and when switching objects.

Diff Detail

Repository

rB Blender

Branch

mesh-symmetry (branched from master)

Build Status

Buildable 9571
Build 9571: arc lint + arc unit

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Aug 16 2020, 6:21 PM

Pablo Dobarro (pablodp606) created this revision.

Harbormaster completed remote builds in B9548: Diff 27766.Aug 16 2020, 6:21 PM

Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)Aug 16 2020, 6:22 PM

Pablo Dobarro (pablodp606) added a task: T79785: Make XYZ symmetry a property of Objects.

This updates all paint modes to use the new symmetry flags. @Germano Cavalcante (mano-wii) Is it ok to replace all instances of ME_EDIT_MIRROR_X with the new flags?

Good to see this property being unified.
If the default value of the mesh edit remains the same (without symmetry), I see no problem.

Use ME symmetry flags in Edit Mesh code
Fix UI in object mode

Harbormaster completed remote builds in B9551: Diff 27771.Aug 16 2020, 8:33 PM

Ok, now XYZ symmetry options are shared between vetex paint, texture paint, weight paint, sculpt and edit mode. If everything works correctly, do we want to keep the old flags and RNA properties for something or should I remove and deprecate them?

Pablo Dobarro (pablodp606) added a reviewer: Brecht Van Lommel (brecht).Aug 16 2020, 8:35 PM

Brecht Van Lommel (brecht) added reviewers: Germano Cavalcante (mano-wii), Campbell Barton (campbellbarton).Aug 17 2020, 5:03 PM

For compatibility the property could just keep the name use_mirror_x and add use_mirror_y and use_mirror_z? If we want to change the UI as well to symmetry that's possible for consistency, but the Python API doesn't need to break for this I think.

This will also need version patching to keep the existing flag from edit mode, or it could keep using editflag.

Rename use_symmetry_x to use_mirror_x
Add versioning

Harbormaster completed remote builds in B9571: Diff 27798.Aug 17 2020, 6:00 PM

Overall it looks good to me.

release/scripts/startup/bl_ui/space_view3d_toolbar.py
1093	This `ipaint` seems to be no longer used here
source/blender/makesdna/DNA_mesh_types.h
267	I think it would be nice to remove (plus change the value of others and versioning) or rename these `ME_EDIT_MIRROR_` flags to `ME_FLAG_UNUSED_` since they are no longer used.

This revision is now accepted and ready to land.Aug 17 2020, 10:16 PM

Review Update

Harbormaster completed remote builds in B9582: Diff 27815.Aug 17 2020, 10:30 PM

Looking at the patch, what is the purpose of moving the flags to a new symmetry member, rather than just leaving it in editflag? It seems simpler to just leave things as they were and make sculpting use those flags.

release/scripts/startup/bl_ui/space_view3d_toolbar.py
995	The same mirror X option is now in two places. Previously it was already confusing, but the tooltip for the old option explained that one was for mirroring the brushes. The best solution would be to support Y and Z mirroring everywhere. I haven't checked how complicated that is. If it's too complicated, is there something better we can do here?

This revision now requires changes to proceed.Aug 18 2020, 5:35 PM

Looking at the patch, what is the purpose of moving the flags to a new symmetry member, rather than just leaving it in editflag?

I thought that it may be less confusing to have a separate variable and flags for symmetry options that are intended to be supported in all modes. Right now editflag contains flags that are mode specific, like ME_EDIT_PAINT_VERT_SEL.

release/scripts/startup/bl_ui/space_view3d_toolbar.py
995	I'm not familiar with the weight paint tools and with the transform tools, so I'm not sure why it is not supported. I've only seen the Y and Z symmetry flags being used once in the transform code. Maybe we can leave the current editmesh flag for X symmetry where Y and Z is not supported and use the new flags for the rest of cases.

I personally don't see much issue in having the flags in editflag, but either way is fine.

This revision is now accepted and ready to land.Aug 19 2020, 7:43 PM

Initial code review.

release/scripts/startup/bl_ui/space_view3d_toolbar.py
1104–1107	The `mesh = context.object.data` should be assigned.
source/blender/blenloader/intern/versioning_290.c
492–493	These flags could be cleared at the same time, so we can re-use them later. (their declaration should then have `/* cleared */` after it, so we know reusing is safe).
source/blender/makesdna/DNA_mesh_types.h
269–270	These should be named `ME_EDITFLAG_UNUSED_`, as we already have `ME_FLAG_UNUSED_` for `Mesh.flag`.

This revision now requires changes to proceed.Aug 20 2020, 7:08 AM

This causes changed behavior in weight paint mode where both sculpt symmetry and weight mirror are enabled at once, this makes it impossible to paint onto symmetrically named groups.

Attached video, behavior from master on the left, this patch is on the right.

We could have an option for weight paint to make symmetry apply to Symmetry: [Brush | Vertex Group] since it seems highly unlikely we want to enable both at once.

Also noticed weight-paint will use Y and Z symmetry, if they have been set in other mode.

Review Update

Harbormaster completed remote builds in B9693: Diff 27985.Aug 20 2020, 4:47 PM

@Campbell Barton (campbellbarton) In that case, wouldn't it make more sense to add a separate flag for using vertex group symmetry and keep these flags only for brush symmetry to be consistent with the rest of the modes? Should that change be part of this patch?

RedMser (RedMser) added a subscriber: RedMser (RedMser).Aug 21 2020, 3:26 AM

Having a separate flag seems fine, I think that should be part of this patch.

Use a separate property for Vertex Group L/R mirroring

Harbormaster completed remote builds in B9899: Diff 28343.Aug 31 2020, 7:01 PM

@Campbell Barton (campbellbarton) Ok, now weight paint should work as expected. There are separate options to control the brush symmetry (using the unified symmetry flags like in any other mode) and the L/R vertex groups symmetry

Pablo Dobarro (pablodp606) mentioned this in T80402: Improve workflow with multiple objects.Sep 2 2020, 10:27 PM

Julien Kaspar (JulienKaspar) added a subscriber: Julien Kaspar (JulienKaspar).Sep 10 2020, 5:38 PM

Lopo Isaac (lopoIsaac) awarded a token.Sep 15 2020, 10:07 AM

@Campbell Barton (campbellbarton) Is this patch ok to commit now?

Seems fine, only minor issues noted.

source/blender/makesdna/DNA_mesh_types.h
270–271	We could leave these for future support of Y/Z vertex-group mirror (leave them commented out).
source/blender/makesrna/intern/rna_mesh.c
3287–3291	Naming is a bit odd here, why not call: `use_mirror_vertex_group_x` ?

This revision is now accepted and ready to land.Sep 18 2020, 3:27 AM

Metin Seven (MetinSeven) awarded a token.Sep 18 2020, 7:58 PM

Closed by commit rB5502517c3c12: Unify all XYZ symmetry options using Mesh Symmetry (authored by Pablo Dobarro (pablodp606)). · Explain WhySep 18 2020, 8:00 PM

This revision was automatically updated to reflect the committed changes.

Pablo Dobarro (pablodp606) marked an inline comment as done.

Pablo Dobarro (pablodp606) added a commit: rB5502517c3c12: Unify all XYZ symmetry options using Mesh Symmetry.

David Friedli (hlorus) mentioned this in T80580: Mirror options, when activated by shortcuts, do not update interface.Sep 26 2020, 1:20 AM

Diogo Valadares Reis dos Santos (DiogoX2) mentioned this in T81461: Weight paint crashing too often with blur brush + symmetry.Oct 6 2020, 2:36 PM

Hans Goudey (HooglyBoogly) mentioned this in T82805: Undo in Sculpt Mode.Nov 18 2020, 5:48 PM

Sybren A. Stüvel (sybren) mentioned this in T84518: XYZ symmetry options: unclear release notes.Jan 8 2021, 1:48 PM

Unify all XYZ symmetry options using Mesh Symmetry
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 27798

release/scripts/startup/bl_ui/space_view3d.py

release/scripts/startup/bl_ui/space_view3d_toolbar.py

source/blender/blenloader/intern/versioning_290.c

source/blender/draw/intern/draw_cache_impl_mesh.c

source/blender/editors/mesh/editmesh_tools.c

source/blender/editors/object/object_remesh.c

source/blender/editors/object/object_vgroup.c

source/blender/editors/physics/particle_edit.c

source/blender/editors/sculpt_paint/paint_cursor.c

source/blender/editors/sculpt_paint/paint_image_proj.c

source/blender/editors/sculpt_paint/paint_mask.c

source/blender/editors/sculpt_paint/paint_vertex.c

source/blender/editors/sculpt_paint/paint_vertex_weight_ops.c

source/blender/editors/sculpt_paint/paint_vertex_weight_utils.c

source/blender/editors/sculpt_paint/sculpt.c

source/blender/editors/sculpt_paint/sculpt_automasking.c

source/blender/editors/sculpt_paint/sculpt_boundary.c

source/blender/editors/sculpt_paint/sculpt_intern.h

source/blender/editors/sculpt_paint/sculpt_mask_expand.c

source/blender/editors/sculpt_paint/sculpt_pose.c

source/blender/editors/sculpt_paint/sculpt_transform.c

source/blender/editors/transform/transform_generics.c

source/blender/makesdna/DNA_mesh_types.h

source/blender/makesrna/intern/rna_mesh.c

Unify all XYZ symmetry options using Mesh SymmetryClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 27798

release/scripts/startup/bl_ui/space_view3d.py

release/scripts/startup/bl_ui/space_view3d_toolbar.py

source/blender/blenloader/intern/versioning_290.c

source/blender/draw/intern/draw_cache_impl_mesh.c

source/blender/editors/mesh/editmesh_tools.c

source/blender/editors/object/object_remesh.c

source/blender/editors/object/object_vgroup.c

source/blender/editors/physics/particle_edit.c

source/blender/editors/sculpt_paint/paint_cursor.c

source/blender/editors/sculpt_paint/paint_image_proj.c

source/blender/editors/sculpt_paint/paint_mask.c

source/blender/editors/sculpt_paint/paint_vertex.c

source/blender/editors/sculpt_paint/paint_vertex_weight_ops.c

source/blender/editors/sculpt_paint/paint_vertex_weight_utils.c

source/blender/editors/sculpt_paint/sculpt.c

source/blender/editors/sculpt_paint/sculpt_automasking.c

source/blender/editors/sculpt_paint/sculpt_boundary.c

source/blender/editors/sculpt_paint/sculpt_intern.h

source/blender/editors/sculpt_paint/sculpt_mask_expand.c

source/blender/editors/sculpt_paint/sculpt_pose.c

source/blender/editors/sculpt_paint/sculpt_transform.c

source/blender/editors/transform/transform_generics.c

source/blender/makesdna/DNA_mesh_types.h

source/blender/makesrna/intern/rna_mesh.c

Unify all XYZ symmetry options using Mesh Symmetry
ClosedPublic
Actions

Revision Contents
Changeset List