Global Top-Bar - Initial Implementation
Needs RevisionPublic

Authored by Julian Eisel (Severin) on Jul 29 2017, 9:17 PM.

Details

Summary

Global Top-Bar - Initial Implementation

This adds the initial implementation of the new global top-bar, as decided on during the 2.8 UI workshop (see https://wiki.blender.org/index.php/Dev:2.8/UI/Workshop_Writeup#Global_Bars).

Main Features/Changes for Users

  • Add horizontal bar at top of all non-temp windows, consisting out of two horizontal sub-bars.
  • Upper sub-bar contains global menus (File, Edit, etc.), tabs for workspaces, render engine and scene selector.
  • Lower sub-bar contains object mode selector, operator redo buttons, screen-layout and render-layer selector.
  • Individual sections of the topbar are individually scrollable
  • Workspace tabs can be double- or ctrl-clicked for renaming
  • Top Bar should scale nicely with DPI

Technical Features/Changes

  • Support for global areas (horizontal-only for now). They are part of the window, not part of the screen-layout.
  • Removes all info editors when reading old files, they'll get an entirely new purpose
  • New space-type "Top Bar". Shouldn't be selectabe through menu (TODO)

TODO

TODO :)

There are quite a few hacks and ugly parts in this, and I wouldn't consider it finished at all. I think it's definitely good and stable enough to demo in 2.8 branch though (e.g. at Siggraph ;) )

Diff Detail

Repository
rB Blender
Branch
topbar
Build Status
Buildable 737
Build 737: arc lint + arc unit

Noticed there are some unwanted changes caused by merges (related to linking & transform orientations mainly). Please ignore, will remove them soon.

Julian Eisel (Severin) retitled this revision from Global Top-Bar, Initial Implementation to Global Top-Bar - Initial Implementation.Jul 29 2017, 9:58 PM
Julian Eisel (Severin) edited the summary of this revision. (Show Details)

I know this is a work in progress and not finished at all, but I'll leave a few unsolicited suggestions if I may; purely from a mere user point of view:

The order of the buttons, pulldown menus, tools etc should try to roughly reflect some sort of hierarchy, like say:
Scene Selector Pulldown Menu > Layers Pulldown Menu > Interaction Mode Pulldown Menu > Operator tools properties
On the top header next to the tabs if possible keep only the ones that may not fit into the same hierarchy like Render Engine selection and Screen Layout which are transversal to all.

Other than that this is looking pretty good, especially if the eliminates the need for that temporary operator properties on the bottom left of the Tool Shelf; it is always wrongly sized and easy to miss.
Would also love if you brought back the old behavior of popping by default the operator properties pop-up directly in the 3D View, like in the old days of pre-2.49.

Sorry for the noise, keep up the great work.

Brecht Van Lommel (brecht) requested changes to this revision.Aug 1 2017, 1:53 AM

Great to see this taking shape, seems to work pretty much as expected in my tests. Would be good to see the to do items, there's a bunch incomplete/glitchy stuff I could list in reviewing this that you are probably aware of. Probably there should be 2.8 tasks created here to track to do's.

For now only some quick code review comments. It's difficult to review some of the places where a lot of code moved. The way things fits together makes sense to me: making the top bar a new space type, that is in a new global area in the window.

source/blender/blenloader/intern/versioning_280.c
328

DPI is per window now, so this can't be correct in general. Maybe it get fixed in some later refresh anyway. I see a bunch of flickering as well when switching workspaces, clicking redo, etc. Not sure if that's a refresh issue related me using high DPI, or if there's some other layout refresh issues.

source/blender/editors/interface/interface_layout.c
3517

This doesn't seem to belong in layout code?

Regardless, I don't think the operator name should be in a button to redo the operator, I think that should be a separate button with a redo icon. Now I expect users will think this is an "apply" button, not a "redo" button.

source/blender/editors/screen/area.c
1564

Might be good to only have ED_area_initialize that checks ED_area_is_global.

2154

Not using the TH_HEADER color here at all anymore seems wrong?

source/blender/editors/screen/screen_edit.c
682–691

Can you add a comment about what this code does?

827–830

Is this really supposed to be if/else, or could both types of updates be needed? It's not very clear me what the different types of updates are, some comments might help. Or maybe some code could be deduplicated?

source/blender/makesdna/DNA_screen_types.h
426

Maybe RGN_FLAG_DYNAMIC_SIZE would better describe the intent of this flag, rather than details about the implementation?

429

It's not clear to me how this flag can work reliably. It's only read in one place, but I would expect sizex to be used in many more places that might break if DPI is applied?

source/blender/windowmanager/intern/wm_subwindow.c
248–249

These got renamed to screen but then don't use WM_window_screen_pixels_x?

This revision now requires changes to proceed.Aug 1 2017, 1:53 AM

Some UI design notes from testing this:

  • The operator redo properties look messy. The roundness of number buttons, lack of clear grouping, .. . Some tweaks will be needed here.
  • I think the second bar in the top bar should be higher, I expect that would look better and distinguish it more from other area headers.
  • I hope we can move the render engine menu into the Render menu, once we hopefully have draw modes in the 3D views again.
  • The screen layouts menu could be made smaller, leaving out the screen name? Maybe screens don't need names anymore.
  • Scene and render layer menus should probably be in the same bar?

Thanks for the quick review @Brecht Van Lommel (brecht)!

Would be good to see the to do items, there's a bunch incomplete/glitchy stuff I could list in reviewing this that you are probably aware of.

Indeed, I should really write down the to-do items. I'm on a little vacation the next few days, so not sure if I'll find the time. Will definitely try to.

Probably there should be 2.8 tasks created here to track to do's.

Not sure if that's really needed, I can just list them here. I think most are rather quick to resolve anyway. Not against it, just wondering if this wouldn't be overkill.

  • The operator redo properties look messy. The roundness of number buttons, lack of clear grouping, .. . Some tweaks will be needed here.
  • I think the second bar in the top bar should be higher, I expect that would look better and distinguish it more from other area headers.
  • I hope we can move the render engine menu into the Render menu, once we hopefully have draw modes in the 3D views again.

Agree with all of these.

  • The screen layouts menu could be made smaller, leaving out the screen name? Maybe screens don't need names anymore.

Yes, I hope we can get rid of user visible names for screen-layouts as well. We could give it a try once 2.8 is more usable and ready for serious testing.

  • Scene and render layer menus should probably be in the same bar?

The upper bar contains global/scene/window properties right now, while the lower one contains per-workspace properties and the operator redo settings. I think that's a reasonable separation.
The scene is per window and the active render-layer per workspace currently, so having both in the same bar would break that separation.

source/blender/blenloader/intern/versioning_280.c
328

It's indeed not correct, but we'd have to call WM_window_set_dpi or duplicate its code here to make it correct. However, the area size and coordinates are also set in screen_test_scale before drawing. So we can actually remove the code here that sets area vertices and size, it's redundant anyway (meaning we don't need dpi_fac here).

source/blender/editors/interface/interface_layout.c
3517

I'd say the entire function should be moved to interface_templates.c, doesn't belong to layout code at all if you ask me.

Re button text - For now I mostly followed the design from T50845, I guess we should discuss such things there. I definitely see your point though.

source/blender/editors/screen/area.c
1564

They used to be quite different, but now they are almost identical. So +1 after all.

2154

Right. Think that was a quick test that got committed accidentally. Will remove.

source/blender/editors/screen/screen_edit.c
827–830

The else block is basically a partial version of ED_screen_refresh that just ensures area/region sizes are updated. It's needed for the regions in the top-bar that are supposed to fit to layout size (think of layout or DPI changes).
Actually I think we could add something like bScreen.do_refresh_sizes to avoid unnecessary executions of this. Should also help making clear what this does.

source/blender/makesdna/DNA_screen_types.h
429

Other places might break, but for now it isn't an issue for us since sizex is mainly used for region resizing which isn't possible in global areas currently.
I'm not really happy about this flag in general. Would like to spend some more time on investigating if we can avoid it. For the time being I should probably add a XXX noting that this should only be a temporary solution and that it might cause issues.

source/blender/windowmanager/intern/wm_subwindow.c
248–249

Eeek right, variable should use term 'screen'.

This comment was removed by Galen Beals (galenb).

The order of the buttons, pulldown menus, tools etc should try to roughly reflect some sort of hierarchy, like say:
Scene Selector Pulldown Menu > Layers Pulldown Menu > Interaction Mode Pulldown Menu > Operator tools properties
On the top header next to the tabs if possible keep only the ones that may not fit into the same hierarchy like Render Engine selection and Screen Layout which are transversal to all.

There actually is some hierarchy in the current alignment of items:

  • The first bar contains items that are global/per-window/per-scene. The Blender icon-button and the menus are global, the workspace tabs and the scene switch are per window. The render engine menu is supposed to be moved to the render-layer settings or at least into the properties editor to my knowledge, so its current position is just temporary.
  • The lower bar contains per-workspace settings (object mode, screen-layout, render-layer) and the global last operator settings.

So the hierarchy is that the upper bar contains items that are not per workspace, the lower bar per workspace ones. The last operator settings are an exception to this rule because of space usage, but think this exception is fine.
I guess this hierarchy becomes more clear and makes more sense when using it. I'd expect this to be the best or most reasonable structure, but maybe others disagree :)

Would also love if you brought back the old behavior of popping by default the operator properties pop-up directly in the 3D View, like in the old days of pre-2.49.

Not sure which exact behavior your referring to here, but I don't think we're going to do something like this soon. We have our non-overlapping UI paradigm currently, which I'm quite happy about. However, sounds like something an add-on could do.

Thanks for the feedback :)

Excellent! Looks really great.

If I may add too: The tabs replicate the function of the layouts menu. It seems like there only needs to me a way to remove layouts and you're good to go with just tabs. couldn't You easily get rid of the layout menu and gain some space?

The tabs actually represent workspaces, which are new in 2.8. They are there for configuring Blender for different workflows within a project (modelling, animating, rendering, compositing, ...). Previously, that kinda was the purpose of screen-layouts, but workspaces go much further than they do.
We figured there is still a need for multiple screen-layouts within those workspaces. So actually, each workspace can contain it's own list of screen-layouts now. That's why there are both, workspace tabs and the screen-layout switch.


General Note

I notice we start having some general UI design discussion here. Of course that's fine, but we should move it to T50845. That way we can stick to the code side of things here :)