Details

Reviewers

Antonio Vazquez (antoniov)
Campbell Barton (campbellbarton)

Maniphest Tasks

T63125: Can't select individual bones when in weight paint mode for grease pencil

Commits

rBf00cb93dbec7: Fix T63125: Gpencil: bones cannot be selected in weightpaint mode

Summary

Some underlying functionality was not ready for greasepencil:

BKE_modifiers_get_virtual_modifierlist (now introduce dedicated BKE_gpencil_modifiers_get_virtual_modifierlist)
BKE_modifiers_is_deformed_by_armature
checks in drawing code
checks in (pose) selection code

A couple of changes to make this work:

eGpencilModifierType_Armature has to be respected (not only eModifierType_Armature)
OB_MODE_WEIGHT_GPENCIL has to be respected (not only OB_MODE_WEIGHT_PAINT) -- (now use new OB_MODE_ALL_WEIGHT_PAINT)
gpencil_weightmode_toggle_exec now shares functionality from wpaint_mode_toggle_exec -- moved to new ED_object_posemode_set_for_weight_paint

This patch will also set the context member "weight_paint_object" for greasepencil (otherwise some appropriate pose operators wont work when in weightpaint mode)

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Aug 6 2020, 2:53 PM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B9378: Diff 27477.Aug 6 2020, 2:53 PM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Aug 6 2020, 2:54 PM

Philipp Oeser (lichtwerk) added projects: Grease Pencil, Modeling, Modifiers, Animation & Rigging.

@Falk David (filedescriptor) Please, take a look of this patch because you are doing big changes to the selection code gpencil_select.c

Looks good to me. It does not interfere with the editcurve code.

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Aug 7 2020, 10:37 AM

Campbell Barton (campbellbarton) requested changes to this revision.Aug 17 2020, 12:57 PM

Campbell Barton (campbellbarton) added inline comments.

source/blender/blenkernel/intern/modifier.c
659	The grease-pencil modifiers use `GpencilModifierData`, we can't rely on them being compatible with `ModifierData`, there should be a separate function for grease-pencil.
744	clang-tidy will kick out the `else` here :)
source/blender/draw/engines/overlay/overlay_armature.c
117	We can use `(draw_ctx->object_mode & (OB_MODE_WEIGHT_PAINT \| OB_MODE_WEIGHT_GPENCIL)) != 0)` here. I'd prefer this since it's in keeping with flag use elsewhere, see: `OB_MODE_ALL_SCULPT`, `OB_MODE_ALL_MODE_DATA`... etc.
source/blender/editors/gpencil/gpencil_edit.c
601–605	This is copying a fairly large block of code from `paint_vertex.c`, this should be moved into a shared function. Or, if this is not an options because of the modifier type, two functions next to each other, although I think it's worth attempting to generalize them, even if the modifier type check happens inside the modifier loop.

This revision now requires changes to proceed.Aug 17 2020, 12:57 PM

address review by @Campbell Barton (campbellbarton)

introduce BKE_gpencil_modifiers_get_virtual_modifierlist
introduce OB_MODE_ALL_WEIGHT_PAINT
introduce ED_object_mode_weight_armature_posemode_toggle

Harbormaster completed remote builds in B9605: Diff 27857.Aug 18 2020, 3:02 PM

Philipp Oeser (lichtwerk) marked 4 inline comments as done.Aug 18 2020, 3:04 PM

remove comment

Harbormaster completed remote builds in B9606: Diff 27858.Aug 18 2020, 3:07 PM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Aug 18 2020, 3:13 PM

Taking a step back I am honestly not sure if we need the concept of virtual modifiers for gpencil at all?
While this might still hold true for meshes [even there I am unsure if there is a situation where e.g. Lattices or Armatures are deforming outside the modifier stack?], I dont see this happening for gpencil at all...

This might lead to the fact that XXX_modifiers_get_virtual_modifierlist could be simplified/replaced with just looping over "real" modifiers?

Philipp Oeser (lichtwerk) mentioned this in T63125: Can't select individual bones when in weight paint mode for grease pencil .Aug 20 2020, 1:54 PM

deactivate code relating to unsupported virtual modifiers

Harbormaster completed remote builds in B9938: Diff 28455.Sep 2 2020, 5:33 PM

@Campbell Barton (campbellbarton), @Antonio Vazquez (antoniov): I consider this ready for review, mind checking on it again?

The one open question for me is if it is helpful to leave in code relating virtual modifiers for greasepencil.
We could just loop over greasepencil_modifiers, but the way it is now is much more in line with what meshes support (and what gpencil - in theory - should support as well? [unless we want to remove the whole parent_type in Object Relations for GPencil?])

So for now I left the code in (#if 0), let me know what you think.

Tested, this works well.

source/blender/editors/object/object_modes.c
358	The NULL check isn't needed, same below.

This revision is now accepted and ready to land.Sep 3 2020, 2:11 PM

Campbell Barton (campbellbarton) added inline comments.Sep 3 2020, 2:17 PM

source/blender/editors/object/object_modes.c
349	The term `toggle` here is misleading, could use `set`, or call `ED_object_posemode_set_for_weight_paint`, using similar prefix to (ED_object_posemode_enter).

address review comments

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Sep 3 2020, 2:56 PM

Harbormaster completed remote builds in B9958: Diff 28489.Sep 3 2020, 2:56 PM

Closed by commit rBf00cb93dbec7: Fix T63125: Gpencil: bones cannot be selected in weightpaint mode (authored by Philipp Oeser (lichtwerk)). · Explain WhySep 3 2020, 3:09 PM

This revision was automatically updated to reflect the committed changes.

Philipp Oeser (lichtwerk) added a commit: rBf00cb93dbec7: Fix T63125: Gpencil: bones cannot be selected in weightpaint mode.

Fix T63125: Gpencil: bones cannot be selected in weightpaint mode
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 28490

source/blender/blenkernel/BKE_gpencil_modifier.h

source/blender/blenkernel/intern/gpencil_modifier.c

source/blender/blenkernel/intern/modifier.c

source/blender/draw/engines/overlay/overlay_armature.c

source/blender/draw/intern/draw_manager.c

source/blender/editors/armature/pose_select.c

source/blender/editors/gpencil/gpencil_edit.c

source/blender/editors/include/ED_object.h

source/blender/editors/object/object_modes.c

source/blender/editors/screen/screen_context.c

source/blender/editors/screen/screen_ops.c

source/blender/editors/sculpt_paint/paint_vertex.c

source/blender/editors/space_view3d/view3d_select.c

source/blender/editors/space_view3d/view3d_view.c

source/blender/makesdna/DNA_gpencil_modifier_types.h

source/blender/makesdna/DNA_object_enums.h

Fix T63125: Gpencil: bones cannot be selected in weightpaint modeClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 28490

source/blender/blenkernel/BKE_gpencil_modifier.h

source/blender/blenkernel/intern/gpencil_modifier.c

source/blender/blenkernel/intern/modifier.c

source/blender/draw/engines/overlay/overlay_armature.c

source/blender/draw/intern/draw_manager.c

source/blender/editors/armature/pose_select.c

source/blender/editors/gpencil/gpencil_edit.c

source/blender/editors/include/ED_object.h

source/blender/editors/object/object_modes.c

source/blender/editors/screen/screen_context.c

source/blender/editors/screen/screen_ops.c

source/blender/editors/sculpt_paint/paint_vertex.c

source/blender/editors/space_view3d/view3d_select.c

source/blender/editors/space_view3d/view3d_view.c

source/blender/makesdna/DNA_gpencil_modifier_types.h

source/blender/makesdna/DNA_object_enums.h

Fix T63125: Gpencil: bones cannot be selected in weightpaint mode
ClosedPublic
Actions

Revision Contents
Changeset List