UI: Optional Complex Layout for Workspace Status #120595
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#120595
Loading…
Reference in New Issue
No description provided.
Delete Branch "Harley/blender:WorkspaceStatus"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Optionally allow complex layout instead of just plain text when using
ED_workspace_status_text.
While in a modal operation there are few ways to display information. Some operators will show an automatic display of modal keymaps in the status bar with icons. But this can be overwritten by a simple string in the status bar instead. And they can also choose to display simple strings in the area header.
This PR also allows adding complex layout items with icons, text, custom spacing, etc. None of the existing uses of the status bar changes. But this PR gives us additional tools to add nice items and make everything look more consistent and uniform.
This PR mostly just implements these additional tools. But in order to test and show what these can do there are a few sample changes;
When you open up menus that are "spacebar to search" it will show the status bar with an icon like this:
Menus that are "type to search" will show this:
The mesh operator "Inset faces" currently shows everything in the header like this:
This PR make it show just the changing values in the header:
With keymap items on the status bar. Binary states that are ON are indicated by inverting the key icon:.
This PR makes similar changes to "Edge Slide"
"Knife" tool width comparison, best viewed full screen. PR above, current below:
Enlargement of a section - which also shows the "2x" used to specify double-clicking:
And note that the only operators this PR actually changes are those shown above. This doesn't make everything magically better. This PR would only commit the code that implements the tools needed to do this. Changes to "Knife", etc would be separate and evaluated separately.
@pablovazquez
This approach works for this purpose. I'm assuming the changes for "Inset Faces" shown above are what we are looking for?
@JulianEisel
I think (or just imagined) that you mentioned an approach like this in our last meeting.
These operations need to set this text at some point in time, and at some later point it is turned into layout when the status bar needs to draw. Currently we have a char array in the workspace struct that anyone can write to or clear.
This PR just replaces that simple string with a listbase of small structs containing just enough information to hold the data, otherwise everything works roughly as now. When statusbar needs to draw it checks if this list contains anything, and if it does turns them into layout. I made it so all existing uses of
ED_workspace_status_text
continue to work as it does now, but we get extra routines to make more complex arrangements with icons, spaces, etc.WIP: UI: Optional Complex Layout for Workspace Statusto UI: Optional Complex Layout for Workspace Status@brecht
I'm hoping the first description is clear, but this looks to give me all the tools I need to make modal operator status displays more consistent between area header and status bar, including showing fancier layout with icons on the status bar.
This PR contains both the changes to add these tools but also test changes which make this look more complex than it is.
In a nutshell the current plain text "status_string" in Workspace is replaced with a ListBase of structs that contain just enough to define one item. These are converted to layout when the status bar is redrawn (interface_templates.cc). Some helper routines in
area.cc
, but otherwise that is it."interface_region_menu_popup.cc", "editmesh_inset.cc", "transform_mode_edge_slide.cc", "transform_mode_vert_slide.cc" are changes just to illustrate how it works.
UI: Optional Complex Layout for Workspace Statusto WIP: UI: Optional Complex Layout for Workspace StatusHmmm... one thing missing here is that I see some operators would need to supplement what we get from
WM_window_modal_keymap_status_draw
. This the case with Edge and Vertex Slide.But I see that "Knife" is overriding that. Might make sense to keep things similar to now where the automatic display can be overridden rather than supplemented.
First comparison between this (above) and current (below) for "Knife":
WIP: UI: Optional Complex Layout for Workspace Statusto UI: Optional Complex Layout for Workspace StatusThanks for tackling this, looks nice.
@ -20,0 +22,4 @@
int icon;
std::string text;
float space_factor;
};
Conceptually, it would be nice if this could directly edit a
uiLayout
rather than this intermediate data structure. But that is tied closely to auiBlock
the creation of which is tied to the drawing code, and it doesn't seem practical. So this seems like a reasonable solution to me.@ -452,0 +455,4 @@
void ED_workspace_status_begin(bContext *C);
/* Adds a new item to the status bar. */
void ED_workspace_status_item(bContext *C,
const std::string text = {},
Not sure what the purpose of is of giving the text a default value, I would rather expected
ED_workspace_status_space
to be used for that case.Indeed, looking at this it showed times when
ED_workspace_status_icons
should have been used.@ -452,0 +459,4 @@
const int icon = 0,
const float space_factor = 0.0f);
void ED_workspace_status_item(bContext *C, std::string text, const int icon, const bool enabled);
I'm not sure about overloading a function like this where one is a float and the other a boolean, it seems error prone. I would rather expect this to be a single function with 5 parameters, or two functions with different names.
Yes, I think the larger error here is that the base function that creates a new WorkSpaceStatusItem should probably be a local static function. The others are more like formatted output instead. This overload confuses these.
@ -452,0 +466,4 @@
/* Adds one or more icons to the status bar. */
void ED_workspace_status_icons(bContext *C, const int icon);
void ED_workspace_status_icons(bContext *C, const int icon1, const int icon2);
Can this just be a single function?
@ -866,0 +964,4 @@
bContext *C, std::string text, wmOperatorType *ot, const int propvalue, const bool enabled)
{
text += ": ";
text += enabled ? "✔" : "🚫";
I'm not sure about using the
🚫
symbol here. I think has meaning "prohibited" rather than "off". I think "✗" maybe be the better opposite of "✔"?It does look "prohibited". But the "✗" (in the various versions we have of it in Unicode) looks a lot like a keypress X. For next update I'll probably make it ✔ & ✖ so we can look at it. Thinner version of these are too small. One matching pair that looks okay is ✅ & ❎, but the "x" one can look "on"? Eh, this editor makes these look unlike how we see them in place, will make some captures later. Don't want to spend too much time on this yet as we can always make and use icons for this too.
@ -802,2 +794,4 @@
ED_area_status_text(t->area, str);
wmOperator *op = WM_window_modal_operator(t->context);
It's not ideal to get the operator from a global context.
Can you store
op->type
inEdgeSlideParams
ininitEdgeSlide
perhaps?@ -804,0 +801,4 @@
ED_workspace_status_begin(t->context);
ED_workspace_status_opmodal(t->context, TIP_("Confirm"), op->type, TFM_MODAL_CONFIRM);
Some text uses
TIP_
, while other text usesIFACE_
. I would expect it to use one or the other consistently?This was changed recently (!117234), and the decision was to use
IFACE_
for the status bar.@ -139,2 +139,2 @@
/** Info text from modal operators (runtime). */
char *status_text;
/** Info from modal operators to display in status bar (runtime). WorkSpaceStatusItem. */
ListBase status;
I think we are trying to move away from using
ListBase
and useVector
instead. That would involve creating aWorkspaceRuntime
structure that contains this vector. And then having aWorkspaceRuntime *runtime;
here.I noticed a few things in the Knife Tool options, but maybe it’s too early and they should be discussed in the future tool PR:
Oh wow, that is a mistake in old existing code.
UI_icon_from_event_type
returns the "drag" version of the mouse for all events except "click" and "press". So that means we get drag for KM_ANY, KM_RELEASE, KM_DBL_CLICK, etc. Will be fixed in this PR but I should probably fix that outside of this.Done!
The specific changes to "Knife" will probably be in a separate PR. I've noticed existing problems in the Knife display that are carrying over into this.
UI: Optional Complex Layout for Workspace Statusto WIP: UI: Optional Complex Layout for Workspace StatusWIP: UI: Optional Complex Layout for Workspace Statusto UI: Optional Complex Layout for Workspace Status@brecht
Sorry, I have digressed a bit as it was getting a bit messy. Using a class for this makes the usage of it much simpler. The constructor can clear any existing status, redraw of the status bar can be triggered when it goes out of scope, the variable names are nicer, and my IDE code completes well.
I haven't had good luck with that yet, just because of the complication of this being a C header. I get quite far along then it blows up. I might have to ask @HooglyBoogly for advice.
Anticipating that we won't be happy with any of the options for using text characters to indicate binary settings, it looks like I can probably make inverted versions of (most) the event icons:
This would work for everything but the mice and I can't think of a time when they are used like this. If so I could probably also show those inverted on a background field.
About the only tricky thing is to communicate this state to the icon drawing code. Perhaps a new uiBut drawflag? And maybe get lazy and turn that into a negative desturation value and treat that as inverted? There is probably a nice way to do it.
It should be possible to define the runtime struct in
BKE_workspace.hh
, and only have the forward declaration inDNA_workspace.h
?Yes, it just needed some namespace voodoo that I had to copy from Hans.
@brecht
I updated this PR so that we can see what it is like to have the Boolean states indicated with inverted event icons. It is certainly a lot shorter, which is a good thing on its own. Makes for more of a messy change though.
I updated this PR and the captures in the first comment, but here is a good capture:
@blender-bot build
@ -18,2 +18,4 @@
struct WorkSpaceLayout;
struct WorkSpaceStatusItem {
struct WorkSpaceStatusItem *next, *prev;
Remove this, only needed for listbase.
@ -20,0 +28,4 @@
namespace blender::bke {
struct WorkSpaceRuntime {
blender::Vector<WorkSpaceStatusItem *> status = {};
Don't use pointers, use
blender::Vector<WorkSpaceStatusItem>
.Don't do unnecessary
= {}
, that's automatic.@ -20,0 +22,4 @@
int icon = 0;
std::string text;
float space_factor;
bool inverted;
If you're going to initialize some members by default, might as well do all of them.
Checkout
From your project repository, check out a new branch and test the changes.