Page MenuHome
Differential D11977

Tools: Improve toolsystem updating on mode changes
Needs RevisionPublic
Actions

Authored by Julian Eisel (Severin) on Jul 20 2021, 6:55 PM.

Details

Reviewers
Campbell Barton (campbellbarton)
Summary

Here's what the code would do before this change:

  • Object mode change operators did:
    • Call WM_toolsystem_update_from_context_view3d() - activates the default tool if needed.
    • Send a message notifier that would cause toolsystem refreshing, see WM_toolsystem_do_msg_notify_tag_refresh().
  • Area reinitialization - ED_area_init() - did:
    • Set AREA_FLAG_ACTIVE_TOOL_UPDATE causing a WM_toolsystem_update_from_context() call - activates the default tool if needed.
    • Cause toolsystem refreshing.

So both ways would basically do the same, yet do it in rather different & confusing ways.

Idea of this patch is to move AREA_FLAG_ACTIVE_TOOL_UPDATE setting to the toolsystem refreshing, which is called by area reinitialization, undo/redo and WM_toolsystem_do_msg_notify_tag_refresh(). That way we don't have to call WM_toolsystem_update_from_context(_view3d)() in mode change operators anymore and do it all through the message-bus.

I found this while investigating why the default Clip Editor tool (WIP in tracking_tools branch) wouldn't be set when switching modes. With this change you only have to make sure the message-bus is notified when the mode changes and that the area responds to that with a WM_toolsystem_do_msg_notify_tag_refresh() call.
Note that for the sequencer this only happened to work because changing modes would hide/unhide regions, causing ED_area_init() to be called.

Diff Detail

Repository
rB Blender
Branch
temp-toolsystem-mode-update (branched from master)
Build Status
Buildable 15923
Build 15923: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) requested review of this revision.Jul 20 2021, 6:55 PM
Julian Eisel (Severin) created this revision.
Harbormaster completed remote builds in B15923: Diff 39745.Jul 20 2021, 6:55 PM
Julian Eisel (Severin) edited the summary of this revision. (Show Details)Jul 20 2021, 6:56 PM
Julian Eisel (Severin) added a project: User Interface.
Julian Eisel (Severin) added a comment.Jul 20 2021, 7:03 PM

With this, the following patch fixes the default tool initialization when changing modes in the Clip Editor:

P2270 Clip Editor Default Tool Fix (partial)
1diff --git a/source/blender/editors/space_clip/clip_ops.c b/source/blender/editors/space_clip/clip_ops.c
2index f5e4c4d55d9..e51c8925aa7 100644
3--- a/source/blender/editors/space_clip/clip_ops.c
4+++ b/source/blender/editors/space_clip/clip_ops.c
5@@ -76,6 +76,8 @@
6 #include "DEG_depsgraph.h"
7 #include "DEG_depsgraph_build.h"
8
9+#include "WM_message.h"
10+
11 #include "clip_intern.h" /* own include */
12
13 /* -------------------------------------------------------------------- */
14@@ -1604,6 +1606,9 @@ static int mode_set_exec(bContext *C, wmOperator *op)
15
16 WM_event_add_notifier(C, NC_SPACE | ND_SPACE_CLIP, NULL);
17
18+ struct wmMsgBus *mbus = CTX_wm_message_bus(C);
19+ WM_msg_publish_rna_prop(mbus, (ID *)CTX_wm_screen(C), sc, SpaceClipEditor, mode);
20+
21 return OPERATOR_FINISHED;
22 }
23
24diff --git a/source/blender/editors/space_clip/space_clip.c b/source/blender/editors/space_clip/space_clip.c
25index 20a8bbc10c1..b16559e3a45 100644
26--- a/source/blender/editors/space_clip/space_clip.c
27+++ b/source/blender/editors/space_clip/space_clip.c
28@@ -56,6 +56,8 @@
29 #include "GPU_matrix.h"
30
31 #include "WM_api.h"
32+#include "WM_message.h"
33+#include "WM_toolsystem.h"
34 #include "WM_types.h"
35
36 #include "UI_interface.h"
37@@ -1036,6 +1038,20 @@ static void clip_main_region_listener(const wmRegionListenerParams *params)
38 }
39 }
40
41+static void clip_main_region_message_subscribe(const wmRegionMessageSubscribeParams *params)
42+{
43+ struct wmMsgBus *mbus = params->message_bus;
44+ ScrArea *area = params->area;
45+ ARegion *region = params->region;
46+
47+ wmMsgSubscribeValue msg_sub_value_region_tag_refresh = {
48+ .owner = region,
49+ .user_data = area,
50+ .notify = WM_toolsystem_do_msg_notify_tag_refresh,
51+ };
52+ WM_msg_subscribe_rna_anon_prop(mbus, SpaceClipEditor, mode, &msg_sub_value_region_tag_refresh);
53+}
54+
55 /****************** preview region ******************/
56
57 static void clip_preview_region_init(wmWindowManager *wm, ARegion *region)
58@@ -1347,6 +1363,7 @@ void ED_spacetype_clip(void)
59 art->init = clip_main_region_init;
60 art->draw = clip_main_region_draw;
61 art->listener = clip_main_region_listener;
62+ art->message_subscribe = clip_main_region_message_subscribe;
63 art->keymapflag = ED_KEYMAP_GIZMO | ED_KEYMAP_FRAMES | ED_KEYMAP_UI | ED_KEYMAP_GPENCIL |
64 ED_KEYMAP_TOOL;
65

Campbell Barton (campbellbarton) accepted this revision.Jul 21 2021, 9:57 AM
This revision is now accepted and ready to land.Jul 21 2021, 9:57 AM
Campbell Barton (campbellbarton) awarded a token.Jul 21 2021, 9:58 AM
Campbell Barton (campbellbarton) added a comment.EditedJul 21 2021, 10:14 AM

Removing these is OK as long as the tool-system is guaranteed to be updated before events are handled.

Since handling the next event can't activate a gizmo left from the tool of a mode that's no longer active.

For this to work reliably WM_msgbus_handle will need to be called after each event instead of in wm_event_do_notifiers.

While I like the direction this change goes, as far as I can see we can't be sure the tool system will be updated between handling events.

We could test adding WM_msgbus_handle to the event loop as a separate patch.

Campbell Barton (campbellbarton) requested changes to this revision.Jul 21 2021, 10:18 AM
This revision now requires changes to proceed.Jul 21 2021, 10:18 AM

Revision Contents
Changeset List

PathSize
source/
blender/
editors/
object/
8 lines
physics/
2 lines
screen/
1 line
sculpt_paint/
2 lines
4 lines
2 lines
windowmanager/
intern/
2 lines

Diff 39745

View Options

source/blender/editors/object/object_edit.c

Loading...
View Options

source/blender/editors/physics/particle_edit.c

Loading...
View Options

source/blender/editors/screen/area.c

Loading...
View Options

source/blender/editors/sculpt_paint/paint_image.c

Loading...
View Options

source/blender/editors/sculpt_paint/paint_vertex.c

Loading...
View Options

source/blender/editors/sculpt_paint/sculpt.c

Loading...
View Options

source/blender/windowmanager/intern/wm_toolsystem.c

Loading...