Details

Reviewers

Campbell Barton (campbellbarton)
Julian Eisel (Severin)

Maniphest Tasks

T77408: Continued Outliner Improvements Design

Commits

rB095a53bd884b: Cleanup: Remove outliner edit mode context menu entries
rB2110af20f5e6: Outliner: Move mode toggling to left column

Summary

Add a column of icons in the left gutter for controlling the interaction
modes of objects. When an object is in a mode other than object mode,
the mode icon will draw to the left of that object. Any other objects
that could be brought into that mode have a dot drawn to the left.

Clicking the dot will bring the object into the edit mode.

For modes like sculpt that don't allow multiple objects in the mode,
clicking the dot will swap the objects in the mode.

The grease pencil draw mode icon shows green. There is no individual icon for this mode, so it uses the outliner data icon which draws green in the outliner.

Diff Detail

Repository

rB Blender

Branch

outliner-mode-column (branched from master)

Build Status

Buildable 10044
Build 10044: arc lint + arc unit

Event Timeline

Nathan Craddock (natecraddock) requested review of this revision.Aug 19 2020, 9:32 PM

Nathan Craddock (natecraddock) created this revision.

Harbormaster completed remote builds in B9659: Diff 27938.Aug 19 2020, 9:32 PM

Nathan Craddock (natecraddock) mentioned this in T77408: Continued Outliner Improvements Design.Aug 19 2020, 9:36 PM

Nathan Craddock (natecraddock) edited the summary of this revision. (Show Details)

Nathan Craddock (natecraddock) added a project: User Interface.

jorge (jorsh) awarded a token.Aug 20 2020, 2:06 PM

Julian Eisel (Severin) mentioned this in D8638: Outliner: Add properties editor sync on selection.Aug 24 2020, 2:51 PM

Campbell Barton (campbellbarton) mentioned this in T68498: Outliner: Mode Toggling.Aug 27 2020, 6:27 AM

I'm not keen on switching the active object picking, replied to design task T68498#1003511

Note that this patch could be applied without changing active-object behavior for edit/pose mode, other issues noted next.

Usability: I find the behavior of showing the mode-icon on hover confusing, since it's not clear if the object is in the mode or not, for example, if the mode doesn't change - you only know this when the mouse moves away. Even exiting the mode is a little odd since the hover state keeps showing as if it's enabled.

Quirk: when the object is hidden using the Eye icon you can't enter/exit the mode (entering makes sense, exiting could be allowed otherwise hiding the active object locks the mode from the outliner) - this is a corner case, the previous issue with hovering makes it feel more like a glitch though.

Inconsistency: adding armatures to edit-mode doesn't set them active, where adding an object to pose-mode does.

Bug: Currently I'm getting an assertion on redo (where redo isn't properly re-creating the edit-mode state):
- Default scene.
- Duplicate "Cube"
- Select All
- Press Tab
- Using the mode-column, exit edit-mode for the active object.
- Hold Ctrl-Z (undo all steps)
- Hold Ctrl-Shift-Z (redo all steps)
- This assert is raised editmesh_undo.c:747, mesh_undosys_step_decode(), at 'mesh_undosys_poll(C)'

Noting some details, although they don't need to be addressed as part of this patch.

This doesn't seem to have a purpose when in object mode, it just takes space, although hiding it causes the outliner layout to jitter on mode switching too.
Currently, clicking on object-data always toggles edit-mode, this patch removes this functionality which is OK, however there is no longer a way to enter edit-mode.
Users may want to click-drag to add objects to a mode (the vertical layout works this way elsewhere). This should be possible but isn't high priority.

This revision now requires changes to proceed.Aug 27 2020, 7:03 AM

RedMser (RedMser) added a subscriber: RedMser (RedMser).Aug 27 2020, 9:15 AM

In D8641#213024, @Campbell Barton (campbellbarton) wrote:

I'm not keen on switching the active object picking, replied to design task T68498#1003511

As I mentioned in T68498: Outliner: Mode Toggling I'm fine with leaving the behavior similar to master (not changing active) and only changing where mode toggling occurs in the outliner. I've made changes to the patch to remove all activation code I added. Besides, a simple workaround is to simply change the active object in the outliner with the usual selection of the name/icon.

Usability: I find the behavior of showing the mode-icon on hover confusing, since it's not clear if the object is in the mode or not, for example, if the mode doesn't change - you only know this when the mouse moves away. Even exiting the mode is a little odd since the hover state keeps showing as if it's enabled.

Changing the dot to the mode icon on hover was some feedback from @Julian Eisel (Severin) (And I believe the studio artists), but it is the cause of the click+drag issue you mentioned. If we leave it as a dot (I'm perfectly okay with this) then there are no hover quirks and you can click+drag to toggle modes.

Quirk: when the object is hidden using the Eye icon you can't enter/exit the mode (entering makes sense, exiting could be allowed otherwise hiding the active object locks the mode from the outliner) - this is a corner case, the previous issue with hovering makes it feel more like a glitch though.

Fixed. Hidden objects can now be removed from the mode.

Inconsistency: adding armatures to edit-mode doesn't set them active, where adding an object to pose-mode does.

Fixed, another case where the activation logic was half-finished. Now neither modes activate the object when brought into the mode.

Bug: Currently I'm getting an assertion on redo (where redo isn't properly re-creating the edit-mode state

After addressing some of your comments I don't get an assert, but the redo doesn't work perfectly. I will look into this more.

This doesn't seem to have a purpose when in object mode, it just takes space, although hiding it causes the outliner layout to jitter on mode switching too.

The gutter already exists in the view layer mode in master, are you referring to the extra space in scenes mode? This patch also gives the option to hide the column completely to give more space for those who don't need outliner mode toggling.

Currently, clicking on object-data always toggles edit-mode, this patch removes this functionality which is OK, however there is no longer a way to enter edit-mode.

This could be addressed with a context menu operator. One exists but it is currently broken (and has been for a long time I think)

Remove all mode toggle activation code

Harbormaster completed remote builds in B9853: Diff 28248.Aug 27 2020, 6:40 PM

Which exact icons to use and how they behave is something we can discuss still. But let's not have that distract us from the more general design aspects.

Works well, some suggestions inline.

source/blender/editors/space_outliner/outliner_draw.c

1903–1904

This could be moved to UI_interface_icons.c.

1935–2000

The button declarations could be de-duplicated:

/* Draw icons for adding and removing objects from the current interation mode */
static void outliner_draw_mode_column_toggle(uiBlock *block,
                                             TreeViewContext *tvc,
                                             TreeElement *te,
                                             TreeStoreElem *tselem,
                                             const bool lock_object_modes)
{
  const int active_mode = tvc->obact->mode;
  bool draw_active_icon = true;

  if (tselem->type == 0 && te->idcode == ID_OB) {
    Object *ob = (Object *)tselem->id;

    /* When not locking object modes, objects can remain in non-object modes. For modes that do not
     * allow multi-object editing, these other objects should still show be viewed as not in the
     * mode. Otherwise multiple objects show the same mode icon in the outliner even though only
     * one object is actually editable in the mode. */
    if (!lock_object_modes && ob != tvc->obact && !(tvc->ob_edit || tvc->ob_pose)) {
      draw_active_icon = false;
    }

    if (ob->type == tvc->obact->type) {
      int icon;
      const char *tip;

      if (draw_active_icon && ob->mode == tvc->obact->mode) {
        icon = outliner_get_mode_icon(active_mode);
        tip = TIP_("Remove from the current mode");
      }
      else {
        /* Not all objects have particle systems */
        if (active_mode == OB_MODE_PARTICLE_EDIT && !psys_get_current(ob)) {
          return;
        }
        icon = ICON_DOT;
        tip = TIP_("Add to the current mode");
      }

      uiBut *but = uiDefIconBut(block,
                                UI_BTYPE_ICON_TOGGLE,
                                0,
                                icon,
                                0,
                                te->ys,
                                UI_UNIT_X,
                                UI_UNIT_Y,
                                NULL,
                                0.0,
                                0.0,
                                0.0,
                                0.0,
                                tip);
      UI_but_func_set(but, outliner_mode_toggle_fn, tselem, NULL);
      UI_but_flag_enable(but, UI_BUT_DRAG_LOCK);
    }
  }
}

This revision is now accepted and ready to land.Aug 28 2020, 7:37 AM

Remove all mode toggle activation code
Cleanup: Move mode icon function
Cleanup: De-duplicate code
Rebase on master

Harbormaster completed remote builds in B9862: Diff 28276.Aug 28 2020, 4:52 PM

Campbell Barton (campbellbarton) accepted this revision.Aug 31 2020, 10:40 AM

After talking to an artist here, I'd suggest two design changes:

In Edit and Pose Mode, swap the edited object with a click in the column, only Ctrl+click would add it to the mode. That way the behavior is consistent across modes, and selection in the Outliner. The tooltip should have a hint for the Ctrl+click option. We could change this later if multi-object editing becomes supported in paint modes. Meanwhile it's better to keep it consistent.
Do not select items when clicking the mode toggle. This is a bit confusing (why would it do this), distracting and it feels like a broken syncing. There shouldn't be a technical need for this.

Also:

I think we should gray out the mode toggle icon for linked objects and add a disabled hint to the tooltip. See UI_block_lock_set()/UI_block_lock_clear().

source/blender/editors/space_outliner/outliner_draw.c
1933	Typo: "suoport" -> "support"

This revision now requires changes to proceed.Sep 2 2020, 12:12 PM

I forgot to add:
@Campbell Barton (campbellbarton) if we swap the active object for editing by default, rather than adding objects to edit mode, we're back at changing the active item for the user. But it's essentially a shortcut for entering object mode, selecting an object and entering the mode again. And the user still explicitly says which item will be activated. So I think this is fine, but would like your opinion.

Cleanup: Fix typo
Don't select on mode toggle
Change object by default for mode toggle
Disable buttons for library data

Harbormaster completed remote builds in B9955: Diff 28484.Sep 3 2020, 3:43 AM

I planned on fixing what you mentioned yesterday, but I ended up reinstalling my OS instead after ruining my bootloader :) I've addressed everything now, but I have a few questions.

In D8641#214594, @Julian Eisel (Severin) wrote:

I think we should gray out the mode toggle icon for linked objects and add a disabled hint to the tooltip. See UI_block_lock_set()/UI_block_lock_clear().

I attempted to do this, but couldn't get it working. I disabled the button for now when objects are linked.

There is one big issue left, which is the undo/redo steps. Because we are changing active, we have to push an extra undo step. I'm not aware of how to skip/merge it into one step for undo.

Another small problem is there is no dedicated mode icon for grease pencil draw

Additionally, when in gpencil draw, vertex/weight paint, or sculpt, (painting modes) clicking the icon of the active object causes a crash.

Use UI_but_disable for library data

Harbormaster completed remote builds in B9966: Diff 28507.Sep 3 2020, 6:25 PM

Noted inline how to fix the Grease Pencil crash. Didn't check yet on the undo one.

source/blender/editors/space_outliner/outliner_select.c
118	I would remove this `if`. Otherwise the behavior is confusing with this option disabled: Clicking the icon without Ctrl only adds to the mode, you have to press Ctrl to be able to remove an object from the mode.
119	To fix the crash with GPencil objects, call `WM_toolsystem_update_from_context_view3d()` here. This should always be done when the mode changes. The mode toggle operators call it, but we don't call them when just exiting the mode. Maybe we should, but this works as well.

This revision now requires changes to proceed.Sep 3 2020, 9:26 PM

Fix: Crash on exiting gpencil draw

Harbormaster completed remote builds in B9972: Diff 28518.Sep 4 2020, 6:17 AM

Mode toggle undo

Harbormaster completed remote builds in B10044: Diff 28642.Sep 8 2020, 5:59 AM

Almost ready! I just noticed one small issue:

If you Ctrl+click the icon to add multiple objects to edit mode, and click on another one then (without Ctrl), it removes the previous active one from the mode, but keeps all others in the mode. That is not what I would expect, if Ctrl is not held it should always remove all other items from the mode I think.

With that addressed, I think this is ready. @Campbell Barton (campbellbarton) do you want to have another look?

This revision now requires changes to proceed.Sep 8 2020, 12:21 PM

Actually I get a failed assert now with undo:

Duplicate the default cube few times
Enter Edit Mode with one
Swap the Edit Mode object through the Outliner
Undo twice

Fix: Return all objects to edit mode

Harbormaster completed remote builds in B10053: Diff 28651.Sep 8 2020, 3:20 PM

In D8641#216083, @Julian Eisel (Severin) wrote:

Almost ready! I just noticed one small issue:

If you Ctrl+click the icon to add multiple objects to edit mode, and click on another one then (without Ctrl), it removes the previous active one from the mode, but keeps all others in the mode. That is not what I would expect, if Ctrl is not held it should always remove all other items from the mode I think.

Fixed this. I'm still unsure of how to fix the assert without adding an additional undo step (between activation and setting the mode).

Generally fine, with one issue.

Currently there is an assertion on undoing when clicking on objects to switch the active object and edit-mode at once.

I can look into a solution for this, for now it might be best to do an undo push (even two undo pushes) - just to avoid the assert (which leads to other issues - as the undo data isn't being properly restored).

Julian Eisel (Severin) added inline comments.Sep 10 2020, 1:29 PM

source/blender/editors/space_outliner/outliner_draw.c
1961	Discussed this with Campbell - for now we can just keep the button undo enabled and accept that it does two undo steps. The important bit to start with is that it doesn't crash/assert in simple cases. The issue here is really more general and not a fault of you patch. Campbell can then look into ways to group the undo steps into one.

Julian Eisel (Severin) added a task: T77408: Continued Outliner Improvements Design.Sep 10 2020, 2:48 PM

Use two undo steps

Harbormaster completed remote builds in B10117: Diff 28738.Sep 10 2020, 3:43 PM

Updated to what @Julian Eisel (Severin) and @Campbell Barton (campbellbarton) agreed on. I used a XXX comment to describe the double undo step (hope that's okay)

Julian Eisel (Severin) accepted this revision.Sep 10 2020, 3:44 PM

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

Closed by commit rB2110af20f5e6: Outliner: Move mode toggling to left column (authored by Nathan Craddock (natecraddock)). · Explain WhySep 10 2020, 5:03 PM

This revision was automatically updated to reflect the committed changes.

Nathan Craddock (natecraddock) added a commit: rB2110af20f5e6: Outliner: Move mode toggling to left column.

Nathan Craddock (natecraddock) added a commit: rB095a53bd884b: Cleanup: Remove outliner edit mode context menu entries.

Philipp Oeser (lichtwerk) mentioned this in T80757: Unable to pose using the outliner.Sep 14 2020, 9:56 AM

Hi all!

I tried following the conversation here, and afaict the discussion was mainly about the left column (no issues with that).
But what is left now for the object data icons in the Outliner hierarchy [the green ones]?
That doesnt seem very practical?

they dont activate the parent
they dont select the parent
even worse: they clear selection for the parent (if it was selected)
if you compare this to any other nested datablock icons, this seems very inconsistent, no? (they all activate & select):
why would you want interactivity on selection if its sole purpose is deselection?
we could not make this interactive at all, but why not bring this in line with the others?

A main reason for this change (which should've been noted in the commit message) is to free the object-data icons to be available for D8638/T63991. Clicking the data-block will then show the corresponding context in attached Properties editors.

I made the decision on Tuesday to not merge that, because it did not go through a proper testing & feedback phase before bcon3. Unfortunately that indeed has the trade-off you describe -- we could probably disable the interactive feedback if users get confused by that.

Deselecting on click is probably a bug.

Philipp Oeser (lichtwerk) mentioned this in T81478: Selection mesh in Object tree in outliner not switching to Edit Mode as in 2.7~2.90.Nov 4 2020, 5:46 PM

Outliner: Add mode toggling column
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 28642

release/scripts/startup/bl_ui/space_outliner.py

source/blender/blenloader/intern/versioning_290.c

source/blender/editors/include/UI_interface_icons.h

source/blender/editors/interface/interface_icons.c

source/blender/editors/space_outliner/outliner_dragdrop.c

source/blender/editors/space_outliner/outliner_draw.c

source/blender/editors/space_outliner/outliner_edit.c

source/blender/editors/space_outliner/outliner_intern.h

source/blender/editors/space_outliner/outliner_select.c

source/blender/editors/space_outliner/outliner_tools.c

source/blender/editors/space_outliner/space_outliner.c

source/blender/makesdna/DNA_space_types.h

source/blender/makesrna/intern/rna_space.c

Outliner: Add mode toggling columnClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 28642

release/scripts/startup/bl_ui/space_outliner.py

source/blender/blenloader/intern/versioning_290.c

source/blender/editors/include/UI_interface_icons.h

source/blender/editors/interface/interface_icons.c

source/blender/editors/space_outliner/outliner_dragdrop.c

source/blender/editors/space_outliner/outliner_draw.c

source/blender/editors/space_outliner/outliner_edit.c

source/blender/editors/space_outliner/outliner_intern.h

source/blender/editors/space_outliner/outliner_select.c

source/blender/editors/space_outliner/outliner_tools.c

source/blender/editors/space_outliner/space_outliner.c

source/blender/makesdna/DNA_space_types.h

source/blender/makesrna/intern/rna_space.c

Outliner: Add mode toggling column
ClosedPublic
Actions

Revision Contents
Changeset List