Page MenuHome

GSoC Outliner Branch
ClosedPublic

Authored by Nathan Craddock (Zachman) on Aug 1 2019, 6:44 AM.
Tags
None
Tokens
"Party Time" token, awarded by Zino."Yellow Medal" token, awarded by dgsantana."Yellow Medal" token, awarded by amonpaike."Yellow Medal" token, awarded by symstract."Love" token, awarded by koloved."Evil Spooky Haunted Tree" token, awarded by xrg."Burninate" token, awarded by billreynish."Like" token, awarded by erickblender."Mountain of Wealth" token, awarded by EitanSomething.

Details

Summary

Here is a summary of the major changes to the outliner in the branch

Selection syncing

  • Added a outliner_sync.c file for syncing. of anything I have changed I feel like this has the potential of needing more work
  • Set the selection syncing flag from any operator that selects in outliner, object, or bone selection.
  • I have also left a couple of print debug messages in the syncing code to help as I code.

Outliner Selection

  • Extended box select to work with click+drag
  • Added walk/keyboard navigation with left and right arrows for open/close element subtree
  • Added range select to item activate
  • Allowed selection of iconrow icons again
  • Added a menu to select objects in aggregated iconrow icons

Interaction

  • Allow click+drag on disclosure triangles to open/close subtrees

Drag and drop

  • Allow setting and clearing parents for all selected elements
    • This required many changes for the popup menu for armatures, curves, and lattices. It might be best to not support those parenting options from the outliner.

Fixes

  • Fixed page up/down from scrolling past the bounds
  • Fixed show active from scrolling past bounds
    • Also modified to open outliner to all instances of active object
  • Icon drawing for constraints and sequencer (needs more icons to be drawn for correct colors)

Diff Detail

Repository
rB Blender
Branch
soc-2019-outliner
Build Status
Buildable 4469
Build 4469: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Nathan Craddock (Zachman) marked 7 inline comments as done.Aug 2 2019, 1:23 AM
source/blender/blenkernel/intern/layer.c
569–570

Okay, I'm not so familier with the collection system so looking at this I had no idea what the reasoning was for the change.

Just note that comments to make it clear whats going on could help.

source/blender/editors/interface/interface_eyedropper_datablock.c
71

Ah in that case the existing name makes sense, although could call it cursor_area - or comment that it's the area under the cursor.

source/blender/makesdna/DNA_outliner_types.h
64

makes sense, could call this TSE_ACTIVE_WALK then?
With short comment noting what you've said above.

Nathan Craddock (Zachman) marked 8 inline comments as done.Aug 3 2019, 10:26 PM
Nathan Craddock (Zachman) added inline comments.
source/blender/editors/space_outliner/outliner_sync.c
254–260

I believe this will work fine, as outliners_mark_dirty will tag all outliners in the .blend as dirty, meaning the global dirty state can be cleared. I have tested with multiple outliners and syncing works well between them.

source/blender/makesdna/DNA_space_types.h
271–272

I think SO_SYNC_SELECT fits in here, as that is the flag to toggle synced selection on or off. I will move it if needed though.

Nathan Craddock (Zachman) updated this revision to Diff 16867.EditedAug 6 2019, 5:04 AM

Refactored and cleaned up synced selection code, now stores global sync state in windowmanager.

There are still a few small things to clean up in selection syncing, but it should be better overall now.

I did make both the outliner dirty state and the window manager dirty state flags. This is because selections can occur without an active outliner, so the types of selections that have occurred since the last sync cannot be overwritten by another selection type (object selection followed by sequence selection for example).

Renamed various flags, variables, and functions for clarity. Added comments.

Nathan Craddock (Zachman) marked 7 inline comments as done.Aug 6 2019, 5:06 AM
  • Merge branch 'master' into soc-2019-outliner
  • Rename variables, including 'outliner' in window manager struct & flag
  • Check outliner dirty flag before syncing selection
  • Remove duplicate unused flag.
  • Don't flag everything as dirty ED_outliner_select_sync_from_outliner.
  • Revert change in previous update (need to check exact use of dirty flag)
  • Avoid getting scene & view layer for each call to outliner_sync_selection_from_outliner

Generally looks good to me, there are some issues with layer handling I'm not familier with, @Brecht Van Lommel (brecht) & @Dalai Felinto (dfelinto) - could you check on this?

(see layer_collection_hidden)

source/blender/editors/space_outliner/outliner_sync.c
93

This should use |= incase other flags have already been set dirty.

325–331

We should check flag types.

335–337

These could check which flags are dirty.

Nathan Craddock (Zachman) marked 3 inline comments as done.Aug 6 2019, 6:50 PM
Nathan Craddock (Zachman) marked an inline comment as done.
  • Preserve existing dirty flags when flagging outliners
  • Outliner Merged object search: Simplify memory free callback
  • Use sync select dirty flag to restrict syncing types
  • Prevent getting active data during syncing
  • Reorganize code to better show to and from outliner syncing
  • Add comments and organize ED_outliner functions
  • Restrict sync types on outliner select
  • Rename sync select set type functions for more clarity
  • Fix crash on editbone syncing
  • Tag depsgraph on pose bone sync only if changed
  • Only notify on syncing specific types of data
Nathan Craddock (Zachman) marked 2 inline comments as done.Aug 7 2019, 7:37 AM

I have added the checks to only sync certain types of data both to and from the outliner depending on what is selected.

To increase code legibility and to reduce the number of CTX function calls I created two structs. One to hold the active data when syncing to the outliner, and the other to hold the syncing types. The sync_select_dirty flag cannot be used alone to determine which types of data to sync because the outliner does not always show the elements that need to be synced. This is most obvious when an outliner is in sequences view and a 3D view selection is made. Then the outliner would need to ensue the dirty flag for objects is not lost because no objects are shown in sequences view.

When syncing from the outliner the only determining factor of what types to sync is if the outliner is in sequences view or not. This could easily be changed to also include the current edit mode.

I also made many organization changes to better group the syncing code.

source/blender/editors/space_outliner/space_outliner.c
134

This was added to redraw the outlier when the active scene camera changes. I couldn't find a more specific way to trigger the redraw, but I think this may redraw in more cases than expected.

  • Merge branch 'master' into soc-2019-outliner
  • Pass outliner type as a pointer
  • Merge branch 'master' into soc-2019-outliner
  • Pass active data as pointer

Just some minor cleanup I noticed as I combined the many commits into
a few smaller commits in the test-soc-2019-outliner-sync branch

Dalai Felinto (dfelinto) requested changes to this revision.Aug 12 2019, 10:15 PM

My test file:

Allow click+drag on disclosure triangles to open/close subtrees

It should behave the same way as the Properties Editor. So it is either opening or closing depending on the initial state of the first click+dragged element. I tried with collections and It should either be in open or close mode. At the moment seems to do both based on mouse direction, seems very confusing in my opinion.

Added walk/keyboard navigation with left and right arrows for open/close element subtree

I find strange that not only it opens but it also navigate to the first child and it stops until it hits a leaf. It we want to keep navigating (not only opening/closing - though not sure why we want that) shouldn't it also keep going to the next leaf after you keep pressing right?

Outliner Selection

When selecting an object in the outliner that belongs to multiple collections, the object is not selected. For example the "2" in my test file.

Added a menu to select objects in aggregated iconrow icons

It shows the same element multiple times if in more than one closed nested collection (e.g., close "Numbers" collection, click in the object icon and you will see "1" multiple times.

This required many changes for the popup menu for armatures, curves, and lattices. It might be best to not support those parenting options from the outliner

The options shown (e.g., to parent something to an armature) seems different when doing it from the outliner. For example, the "Bone Relative" option is only in the viewport.

This revision now requires changes to proceed.Aug 12 2019, 10:15 PM

Allow click+drag on disclosure triangles to open/close subtrees. It should behave the same way as the Properties Editor.

Okay, I'll fix this. It isn't based on the mouse direction entirely, but the direction does have an effect. I can see how this is confusing.

I find strange that not only it opens but it also navigate to the first child

I will remove the walking in and out of subtree on left/right key press. I was basing the behavior on my file manager in linux, and from feedback I received.

Selecting an object in the outliner that belongs to multiple collections, the object is not selected.

I am aware of this, and am about to push a fix to address the issue. :)

It shows the same element multiple times if in more than one closed nested collection.

Oh, hadn't noticed that. Will fix!

The options shown (e.g., to parent something to an armature) seems different when doing it from the outliner

I was basing this on the popup menu that existed before, which also did not contain all of the parenting options. I could add them, however I'm thinking it would be best to remove the popup entirely and only support simple object parenting. @Campbell Barton (campbellbarton) mentioned to me earlier that it would be okay to remove the popop as well.

On the chat @Dalai Felinto (dfelinto) mentioned

It is ok if merged with pending issues, but it would be nice if at least we agree on them being issues

I agree that these issues need to be resolved, and am fine if some of the issues are solved in commits after merge. I should be able to fix most really quickly too.

On the chat @Dalai Felinto (dfelinto) mentioned

It is ok if merged with pending issues, but it would be nice if at least we agree on them being issues

I agree that these issues need to be resolved, and am fine if some of the issues are solved in commits after merge.

It depends on the severity of the issues. But if they are something we would accept as a bug in the tracker, we should not commit until they are fixed.

Brecht Van Lommel (brecht) requested changes to this revision.EditedAug 13 2019, 6:49 PM

This is great work. I only looked at the implementation superficially, but it seems fine overall.

  • There is little contrast between the light blue highlight and white text for active objects, which makes it hard to read. Can we add more contrast here? The selected highlight color could be made darker if needed to distinguish from active.
  • "Sync Selection" should be a checkbox, and not be at the top of the popover. It's not the thing you change most often, probably relatively rare. Maybe put right below Sort Alphabetically?
  • For selection syncing, I think outliners should be able to have different selection. Now they are always synced. Can we only update other outliners selection when selection changes in the 3D viewport?

For selection syncing, I think outliners should be able to have different selection. Now they are always synced. Can we only update other outliners selection when selection changes in the 3D viewport?

I think this makes sense, and would remove some confusion with how selection and objects in multiple collections behave on syncing. This will not be a difficult change to make.

  • Outliner: Deselect data elements on selection sync
  • Outliner: Use interaction mode to restrict syncing
  • Outliner: Fix selection syncing of linked items
  • Outliner: Fix selection syncing for edit bones
  • Outliner: Comments cleanup
Campbell Barton (campbellbarton) requested changes to this revision.Aug 14 2019, 10:40 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/space_outliner/outliner_sync.c
163–171

It would be better to use a set in this case (since list lookups get much slower as the size increases), see GSet data structure (BLI_gset_add use).

This revision now requires changes to proceed.Aug 14 2019, 10:40 AM
  • Outliner: Use GSet for select sync from outliner
  • Merge branch 'master' into soc-2019-outliner
  • Outliner: Sync select toggle tweaks
  • Outliner: Don't sync outliner selection to other outliners
  • Outliner: Openclose fixes
  • Outliner: Remove parent set popup menu for specific types
  • Outliner: Activate on range select with no active element
  • Outliner: Clear dirty flag after sync from outliner
  • Outliner: Don't allow duplicates for merge search
  • Outliner: increase contrast of selected and active highlights
Nathan Craddock (Zachman) marked an inline comment as done.Aug 15 2019, 5:18 AM

All issues previously mentioned should now be resolved.

Added a menu to select objects in aggregated iconrow icons

It shows the same element multiple times if in more than one closed nested collection (e.g., close "Numbers" collection, click in the object icon and you will see "1" multiple times.

The popup now only shows the first instance if an object is linked in multiple collections. Selection from the popup will select the first instance.

The options shown (e.g., to parent something to an armature) seems different when doing it from the outliner. For example, the "Bone Relative" option is only in the viewport.

I removed the popup based on previous discussion with @Campbell Barton (campbellbarton). The popup added more complexity than is necessary to the outliner. A simple parenting to object behavior makes more sense.

"Sync Selection" should be a checkbox, and not be at the top of the popover. It's not the thing you change most often, probably relatively rare. Maybe put right below Sort Alphabetically?

I placed the checkbox below Sort Alphabetically. When the outliner is in Sequences display mode the filter popover is not shown. in this case the toggle is drawn with an icon.

Edit: turns out this is 2.80 master behavior. looking into when this happened.


Raising this issue here since I only just noticed it. Clicking on mesh data currently enters edit-mode,

This means it's hard to select mesh data-block without changing the mode (you can with workarounds like doing a small box select, but it feels weak to be forced to do that).

Would prefer old behavior be kept, where clicking on the icon changes the mode, and regular selection only selects.

When "selection syncing" is disabled I would expect the outliner pretty much like it is now in 2.80. For instance, clicking in the object should still make it active and select it.
Is this change intentional?

When "selection syncing" is disabled I would expect the outliner pretty much like it is now in 2.80. For instance, clicking in the object should still make it active and select it. Is this change intentional?

Yes this is intentional, though it would not be difficult to revert the change. In my original PDF proposal I planned to never sync selections when synced selection is disabled. This fits with the design proposal that already existed T63988: Outliner: Synced Selection. Additionally I chatted with @William Reynish (billreynish) and he encouraged this behavior after he tested my branch.

I see it useful to keep selection contained to the outliner when synced selection is disabled. I don't really see a need to disable synced selection, unless you wanted to only manage data from the outliner, and in that case it would be helpful to manage collections, parents, etc. without modifying the selection state.

@Dalai Felinto (dfelinto) I think what @Nathan Craddock (Zachman) has done is correct.

In current master, we already have a half-baked one-way synced selection system. Selecting items in the Outliner selects them in the 3D View but not the other way around.

With this update we have proper two way synced selection as a toggle. Rather than the old half-baked thing, we have two options: Off and On. Off means off, so there should be no syncing happening when sync selection is off.

  • Merge branch 'master' into soc-2019-outliner

It's no longer possible to toggle object selection.

In master, shift clicking on an object toggles the selected state.
In this branch the shortcut has changed, holding Ctrl & clicking on an object should be able to de-select it.

This happens with/without sync select enabled.

It's no longer possible to toggle object selection.

Looks like this is related to the commit 9dab57a9f829 in master to only activate on name or icons. A ctrl+click on the name or icon will behave the same as in the 3D view; toggling active and selected then deselected with two clicks. Clicking on the row outside the name and icon only selects and leads to the inability to toggle selection.

I think it would be best to wait and address the issues with mode switching as discussed in T68498: Outliner: Modes & activating cameras or scenes and D5497: Outliner: hold Alt to activate/edit data. The current code to handle activation and mode toggling is essentially the same, so separating the behavior would require a larger change. The simplest solution I see is to revert 9dab57a9f829 and address mode switching more thoroughly.

  • Merge branch 'master' into soc-2019-outliner
  • Outliner: Includes cleanup
This revision is now accepted and ready to land.Aug 20 2019, 2:12 AM