Outliner: Display buttons for library overrides
ClosedPublic
Actions

Authored by Julian Eisel (Severin) on Mar 7 2022, 6:16 PM.

Details

Reviewers

Bastien Montagne (mont29)
Dalai Felinto (dfelinto)

Commits

rBd8e3bcf770c2: Outliner: Display buttons to edit library override properties

Summary

As proposed in T95802, this adds buttons to modify the override in the library override display mode.

Some further usability improvements are planned. E.g. this does not yet expand collections (modifiers, constraints, etc) nicely or group modified properties of a modifier together. The following shows a overridden modifier property and how added modifiers are listed:

I decided to add the right column so that the left column can be scrolled independently, like we do it for the restriction toggles in other display modes. This differs from the Data API mode, which has some issues in my opinion.

Diff Detail

Repository: rB Blender

Event Timeline

Julian Eisel (Severin) requested review of this revision.Mar 7 2022, 6:16 PM

Julian Eisel (Severin) created this revision.

Harbormaster completed remote builds in B20892: Diff 49001.Mar 7 2022, 6:16 PM

Julian Eisel (Severin) edited the summary of this revision. (Show Details)Mar 7 2022, 6:19 PM

Julian Eisel (Severin) added projects: Core, Overrides.

Julian Eisel (Severin) edited the summary of this revision. (Show Details)Mar 7 2022, 6:24 PM

Julian Eisel (Severin) added inline comments.Mar 7 2022, 6:28 PM

source/blender/editors/space_outliner/outliner_draw.cc
1721	These are to be committed separately.
source/blender/editors/space_outliner/outliner_intern.hh
222–224	To be committed separately. Noticed I could just pass a `TreeElement *` without warning and get undefined behavior.
source/blender/editors/space_outliner/outliner_utils.cc
393	Old code was adding 1 pixel for this check, but it doesn't seem to be needed. So I'd just remove it.

Did not check the code in details, but generally seems fine.

One thing we should not forget imho is to remove listing of override properties in the ViewLayer view... Having the 'override' icon there should be enough, no need to overload that view anymore.

Adding Dalai so that he can check this too. ;)

Not sure I like the that separator line is visable.
Don't know if you have plans to remove it or not.
I would suggest adding an icon on the top of the the outliner for a more clear seperation option.

Also you can see that the text and the new buttons are not aligned in the center of each line, And now with the addition of the new buttons its more visible especially that you can cleary see the top part of the blue selected line and not the buttom.

Not sure if it's a lot of work or not tho.
Pretty sure it will require a different patch but I just wanted to share my thoughts as long work is been done on it.

@Julian Eisel (Severin) can you use the property name instead of its RNA path? e.g., Location instead of location. Or is this waiting for the unflat tree approach? (if so can you mention in the patch description?

Also is there a way to remove the blue tint color of those properties? They are all override, I don't think we need to make that distinction here.

@Dalai Felinto (dfelinto) those are both TODO's noted in T95802 yes.

Ok, still missing them:

To use icons for Objects, Images, ...
To show only the "Current File" aggregator (thus removing the need for show "Current File" altogether).

I don't see either as a TODO in T955802. Were those ideas dropped?

Committed some cleanups to master already

Harbormaster completed remote builds in B20955: Diff 49108.Mar 9 2022, 3:36 PM

I expect there will be a bunch of tweaking/fine-tuning of the UI, but that is something for later once the main functionality changes are here. This patch only adds the buttons.

To use icons for Objects, Images, ...

I guess you're talking about the base element, the ones grouping together data-blocks by type? I'm actually not convinced we should add an icon there, it's not adding much other than visual noise (as explained by @Yevgeny Makarov (jenkm) here). I'd rather keep things more "calm" here.

To show only the "Current File" aggregator (thus removing the need for show "Current File" altogether).

I checked with Bastien, and he expected that we would also show library overrides from linked libraries in there, so the Current File should be kept.

I checked with Bastien, and he expected that we would also show library overrides from linked libraries in there, so the Current File should be kept.

Okay that was a week or so ago, we just talked again and he agrees we should remove it. The properties from linked library overrides aren't editable anyway, and it may be confusing to show them. However we should probably show them in the other hierarchy view mode.

I will do that as another patch though, since this changes what's currently in Blender and this patch is only adding the buttons right now.

Anything stopping this patch from being merged?

@Julian Eisel (Severin) not from my side, @Dalai Felinto (dfelinto)?

This revision is now accepted and ready to land.Mar 15 2022, 3:16 PM

Julian said that this is only for the buttons, if that's the case +1

Julian Eisel (Severin) edited the summary of this revision. (Show Details)Mar 15 2022, 6:44 PM

Closed by commit rBd8e3bcf770c2: Outliner: Display buttons to edit library override properties (authored by Julian Eisel (Severin)). · Explain WhyMar 15 2022, 6:49 PM

This revision was automatically updated to reflect the committed changes.

Julian Eisel (Severin) added a commit: rBd8e3bcf770c2: Outliner: Display buttons to edit library override properties.