UI - Outliner Visual Fixes. Collapsed Icons, Grease Pencil
ClosedPublic
Actions

Authored by Harley Acheson (harley) on Jun 14 2019, 1:18 AM.

Details

Reviewers

Brecht Van Lommel (brecht)
William Reynish (billreynish)
Antonio Vazquez (antoniov)
Dalai Felinto (dfelinto)

Commits

rB83661a5cfe2e: Outliner - Replacing GP Layer Icon
rB55c379daea68: UI: Icons - Adding ICON_OUTLINER_DATA_GP_LAYER Icon
rB34644f520a46: UI: Outliner - GP Icon Changes
rBe76b223ea3e1: Outliner - Notify on GP Layer Change

Summary

Multiple Icons Collapsed into a Single Icon

When a mesh item is expanded it is nice that we show an active material differently from the rest. But when that row is collapsed, so that one icon is used to represent multiple items (with a number), there is never a time when that icon should show as active.

So for example in the following image. The top portion, expanded, looks great. But when collapsed, as shown on the bottom, The material icon should not show as highlighted. Both items are not active, just one of the group, so showing the highlight adds nothing but visual confusion

Grease Pencil Layer Icons

Grease Pencil layers are showing active layers with a pencil icon, and the rest with a "dot" icon. This breaks with the way that we show active items elsewhere. This patch makes all layers shown as "pencil" but highlights active layer with a highlighted background.

Also when collapsing a row containing multiple grease pencil layers they did not collapse to a single icon like other Outliner items. Instead it shows all icons no matter how many.

The following image shows these issues. The current behavior is shown on top, with "Stroke.001" showing multiple icons on the row, all looking active. And all a "dot" and one pencil. "Suzanne" is expanded and shows three layers, all looking active with dots and a pencil.

The bottom shows how that looks after this patch is applied. "Stroke.001" properly collapsing and showing "5". Suzanne expanded shows three layers with one active.

Note that there is an additional problem with Grease pencil layers in that clicking on a layer in Outliner does not make that layer show as active in Properties. This patch does not fix that, nor hinder it.

Diff Detail

Repository: rB Blender

Event Timeline

Harley Acheson (harley) created this revision.Jun 14 2019, 1:18 AM

Eitan Traurig (EitanSomething) awarded a token.Jun 14 2019, 1:21 AM

This looks correct to me. Nice one. IMO this is more a fix than an actual UI change, so could go into 2.80.

This revision is now accepted and ready to land.Jun 14 2019, 8:26 AM

@Dalai Felinto (dfelinto) Could you look at this? it's related with the mail I have sent to you.

I really like the idea of collapse duplicated icons in one.

Antonio Vazquez (antoniov) accepted this revision.Jun 14 2019, 10:29 AM

Antonio Vazquez (antoniov) awarded a token.

Antonio Vazquez (antoniov) added a reviewer: Antonio Vazquez (antoniov).

Antonio Vazquez (antoniov) added inline comments.Jun 14 2019, 10:33 AM

source/blender/editors/space_outliner/outliner_draw.c
2154	You can remove these comments now (very old ones)

@Harley Acheson (harley) About update of active layer when click in outliner, the active layer changes but it's missing to call the update of the properties panel, and you need put mouse over to update. If you know what flag is missing in the WM message, just add it.

I see the benefit of this patch for materials, grease pencil layers, uv data, ...
However it removes the information we have of where is my active object among the ocean of collapsed collections. Same for active collection and possibility (haven't thought it through) other cases.

source/blender/editors/space_outliner/outliner_draw.c
2639	Don't assume you can cast from GP flags to eOLDrawState. - active = (gpl->flag & GP_LAYER_ACTIVE); + active = (gpl->flag & GP_LAYER_ACTIVE) ? OL_DRAWSEL_ACTIVE : OL_DRAWSEL_NONE;
2688	Pay attention to the building warnings: outlinder_draw.c:2688:24: warning: declaration of ‘active’ shadows a previous local [-Wshadow] eOLDrawState active = merged->active[index]; outliner_draw.c:2616:16: note shadowed declaration is here eOLDrawState active = OL_DRAWSEL_NONE;
2690	Comments should always start with a capital letter and end with a full stop: - /* don't highlight collapsed multiple (icon with number) / - / Don't highlight collapsed multiple (icon with number). / But more than that, comments should try to explain the why, the how is the code itself in this case. A better comment would be: / Don't highlight collapsed multiple (icon with number) since not all the collapsed elements are active. */ That said we don't want this rule to everything. It is only useful to things that are active among the other elements that are nested with it. Not for things that are active among a higher context. In different words, for objects and collections it is really important to know which collection has the active object/collection.
2815	See note above about comments (and yes, I know the original comment was violating that).
2817	That is more personal than anything, but I would probably bring this down to one line: - bGPDlayer gpl = te->directdata; - if (gpl->flag & GP_LAYER_ACTIVE) { + if (((bGPDlayer )te->directdata)->flag & GP_LAYER_ACTIVE) {

This revision now requires changes to proceed.Jun 14 2019, 2:48 PM

Changes from last revision:

Reverts to current behavior of continuing to highlight active icon even when among a group that are collapsed together. I still like the idea in theory, but definitely needs more thinking through.

Properties Editor now updates grease pencil layers as they are selected in Outliner.

Addressed all other issues mentioned in reviews.

I will leave to @Antonio Vazquez (antoniov) to have the final say on it, but since this is now GP only and doesn't break the previous behaviour, I'm fine with it

This revision is now accepted and ready to land.Jun 14 2019, 6:07 PM

@Dalai Felinto (dfelinto) - Does that mean that if @Antonio Vazquez (antoniov) approves that I should just commit?

Don't want to get beaten upon by Brecht or Campbell. I might be able to take Brecht but Campbell looks ripped. LOL

For the final patch commit I would split the notifier changes and the outliner draw changes since they seem unrelated.

Ahh.. that makes a lot of sense to have the actual commits smaller and more atomic. Thanks!

Since Dalai approved it's fine to commit, no need for me to approve all commits.

Closed by commit rBe76b223ea3e1: Outliner - Notify on GP Layer Change (authored by Harley Acheson (harley)). · Explain WhyJun 14 2019, 6:50 PM

This revision was automatically updated to reflect the committed changes.

One remark - GP Layer must use hollow pencil icon - T13. Linear pictograms are for ObData. Filled are used for Objects or to highlight any type of importance or selection.

@Andrzej Ambroz (jendrzych) - One remark - GP Layer must use hollow pencil icon

Hey Andrzej,

Sorry, did not notice that. I just used the icon that was already being used there (I guess in error) for active GP layer, which is mapped to ICON_GREASEPENCIL

I know you have that "hollow pen" icon in the source SVG, but I don't think it is currently mapped for use in the source. I'm pretty sure it is the one marked as "blank" between OUTLINER_DATA_LIGHTPROBE (271) and OUTLINER_DATA_GREASEPENCIL (273)