Page MenuHome
Differential D9849

fix preserving user selected exclude-state of nested collections.
Needs ReviewPublic
Actions

Authored by Dirk Wallenstein (d-egg) on Dec 14 2020, 2:59 PM.

Details

Reviewers
Szymon Ulatowski (szulat)
Brecht Van Lommel (brecht)
Hans Goudey (HooglyBoogly)
Maniphest Tasks
T84305: Outliner: when excluding collections from view over more than one nesting level state gets lost (plus fix)
Summary

Exclude from view-layer state is not preserved across multiple nesting levels in the outliner

In the example:

  • turn off widget1
  • toggle theme off and on again
  • turn on widget1
  • notice that the collection exclude-state is gone.

collection-on-off-setting-nested-lost.blend917 KBDownload

Rewrite the implementation of setting the corresponding flags by focusing on maintaining the synced state of those two flags and trying to make complex logic a bit more intuitive.

Regarding the behavior: excluding from view layer works like a one time ad-hoc curtain thrown over the nested items. When unveiling the explicitly chosen state gets restored.

Diff Detail

Repository
rB Blender
Branch
collectionstate (branched from master)
Build Status
Buildable 11780
Build 11780: arc lint + arc unit

Event Timeline

Dirk Wallenstein (d-egg) requested review of this revision.Dec 14 2020, 2:59 PM
Dirk Wallenstein (d-egg) created this revision.
Dirk Wallenstein (d-egg) added a comment.Dec 14 2020, 3:08 PM

Subject: [PATCH] Outliner: Rewrite exclude flag toggling

There are two exclude flags. One describes the current display state,
and the other tracks the user-chosen state for that element. Toggling
the directly controlled element does either hide all elements beneath,
or restore the previous state of those recursive elements.

IOW: excluding from view layer works like an ad-hoc curtain over all
nested elements. While un-excluding unveils recursively up to children
that the user chose to deliberately exclude. All else would be another
view layer.

Guarantee to keep flags in sync by extracting setting those into
a separate function.

Dirk Wallenstein (d-egg) added projects: User Interface, BF Blender (2.92).Dec 14 2020, 5:31 PM
Dirk Wallenstein (d-egg) added a reviewer: User Interface.Dec 14 2020, 5:47 PM
Dirk Wallenstein (d-egg) added reviewers: Szymon Ulatowski (szulat), Brecht Van Lommel (brecht).Dec 14 2020, 6:48 PM

This reworks f3433fcd3bf87c9405fb05c96fde036eb658e8aa but keeps that behavior. Adding mentioned developers.

Dirk Wallenstein (d-egg) added a comment.Dec 16 2020, 10:06 AM
This comment was removed by Dirk Wallenstein (d-egg).
Dirk Wallenstein (d-egg) added a comment.Dec 16 2020, 10:38 AM

Demo for the problem it fixes

Dirk Wallenstein (d-egg) updated this revision to Diff 32071.Dec 16 2020, 5:41 PM

The new function is certainly not required

Harbormaster completed remote builds in B11780: Diff 32071.Dec 16 2020, 5:41 PM
Dirk Wallenstein (d-egg) mentioned this in T84305: Outliner: when excluding collections from view over more than one nesting level state gets lost (plus fix).Jan 1 2021, 10:35 AM
Robert Guetzkow (rjg) added a task: T84305: Outliner: when excluding collections from view over more than one nesting level state gets lost (plus fix).Jan 1 2021, 10:56 AM
Hans Goudey (HooglyBoogly) requested changes to this revision.Jan 26 2021, 7:36 PM
Hans Goudey (HooglyBoogly) edited projects, added Pipeline, Assets & I/O; removed BF Blender (2.92).
Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).

Thanks for the patch! The current behavior can be annoying, I agree.

Could you remove the detailed steps to reproduce the problem from the patch description? Since they're already in the bug report there's no need to duplicate them here. A higher-level description of the problem would be better for a commit message.

Two small code style things:

  1. Please use clang format on the patch.
  2. Comments should always begin with a capital letter and end with a period. Also I think it's more common not to put them on the same lines as the "if" and "else" statements.
This revision now requires changes to proceed.Jan 26 2021, 7:36 PM
Dirk Wallenstein (d-egg) updated this revision to Diff 33202.Jan 27 2021, 11:49 AM

I slimmed down the comments a bit more. I understand that comments can get
stale, and they might describe stuff that is obvious to people familiar with
the source. I might not have the sense for that. Feel free to truncate more.

Oh, clang-format was easier than I thought. I didn't expect that the entire
source is actually in that format. :)

Harbormaster completed remote builds in B12413: Diff 33202.Jan 27 2021, 11:49 AM
Erick Tukuniata (erickblender) awarded a token.Jan 27 2021, 12:12 PM
Hans Goudey (HooglyBoogly) resigned from this revision.Feb 1 2021, 7:42 AM
In D9849#256071, @Dirk Wallenstein (d-egg) wrote:

I slimmed down the comments a bit more. I understand that comments can get
stale, and they might describe stuff that is obvious to people familiar with
the source. I might not have the sense for that. Feel free to truncate more.

Oh, clang-format was easier than I thought. I didn't expect that the entire
source is actually in that format. :)

Thanks! Yeah, clang format is great. I'll leave the rest of the review to others since I don't have much experience in this area.

Bastien Montagne (mont29) removed a reviewer: User Interface.Feb 16 2021, 6:38 PM
Bastien Montagne (mont29) edited projects, added Core; removed Pipeline, Assets & I/O.

Revision Contents
Changeset List

PathSize
source/
blender/
blenkernel/
intern/
42 lines

Diff 32071

View Options

source/blender/blenkernel/intern/layer.c

Loading...