release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
52	Suggest to rename `filter` to `exclude_filter`, it's a bit more clear what kind of filter this is then. This should also have a default value: `exclude_filter={},`
source/blender/editors/sculpt_paint/sculpt.c
5810	This permanently changes the brush from a detail to a draw brush, I don't think that's the right behavior? I guess what really needs to happen is that toggling dynamic topology causes some kind of tool system refresh, which then leads to the detail brush being unavailable and the tool system switching to the default (draw) brush.

This revision now requires changes to proceed.Jul 31 2019, 1:13 AM

Rename filter to exclude_filter
Remove wrong brush update from sculpt.c
Fix tool switching after disabling dyntopo

Harbormaster completed remote builds in B4248: Diff 16766.Aug 1 2019, 2:56 PM

Pablo Dobarro (pablodp606) marked 2 inline comments as done.Aug 1 2019, 2:57 PM

Brecht Van Lommel (brecht) requested changes to this revision.Aug 2 2019, 12:55 PM

Brecht Van Lommel (brecht) added inline comments.

source/blender/editors/sculpt_paint/sculpt.c
5809–5810	This isn't quite correct since there can be multiple workspaces. Or the operator could be executed in a context where there is no user interface / workspace / area, and crash. @Campbell Barton (campbellbarton), do you know how to do this kind of refresh?

This revision now requires changes to proceed.Aug 2 2019, 12:55 PM

Campbell Barton (campbellbarton) added inline comments.Aug 7 2019, 6:44 PM

source/blender/editors/sculpt_paint/sculpt.c
5809–5810	Currently there isn't a way to handle context changes to sub-modes, this needs to be supported though, think this patch could just not handle this, or - be put on-hold until support is added.

Brecht Van Lommel (brecht) added inline comments.Aug 7 2019, 11:23 PM

source/blender/editors/sculpt_paint/sculpt.c

5809–5810

I don't know exactly what happens now when you switch dynamic topology with the detail tool, without this code. If there's no weird errors and users just need to switch to another brush it's not that bad and we can leave out this code.

Otherwise we can loop over all the active workspace and areas.

for (wmWindow *win = wm->windows.first; win; win = win->next) {
  WorkSpace *workspace = WM_window_get_active_workspace(win);
  bScreen *screen = WM_window_get_active_screen(screen);

  for (ScrArea *sa = screen->areabase.first; sa; sa = sa->next) {
    if (sa->spacetype == SPACE_VIEW3D) {
      bToolKey tkey;
      WM_toolsystem_key_from_context(view_layer, sa, &tkey);
      WM_toolsystem_ref_set_by_id(C, workspace, &tkey, "builtin_brush.Draw", false);
    }
  }
}

Pablo Dobarro (pablodp606) added inline comments.Aug 7 2019, 11:30 PM

source/blender/editors/sculpt_paint/sculpt.c
5809–5810	The workspace won't have any active tool and the topbar will be empty until you select another tool. It does not crash, but most users are probably going to consider that a bug

Brecht Van Lommel (brecht) added inline comments.Aug 13 2019, 6:18 AM

source/blender/editors/sculpt_paint/sculpt.c
5809–5810	Right, then changing the tool in all workspaces as in my code snippet seems best?

Regarding

bToolKey tkey;
WM_toolsystem_key_from_context(view_layer, sa, &tkey);
WM_toolsystem_ref_set_by_id(C, workspace, &tkey, "builtin_brush.Draw", false);

It'd be best to check if the tkey->mode matches the current mode before setting the tool - or even that the active objects match.
Otherwise it will try set the tool in a mode which doesn't make sense, while I didn't try this, the best case is it will fail silently.

William Reynish (billreynish) awarded a token.Sep 22 2019, 3:34 PM

AnityEx (AnityEx) awarded a token.Mar 16 2020, 4:24 PM

Konstantina Anagnostou (Constantina32) awarded a token.Mar 19 2020, 4:27 PM

Hans Goudey (HooglyBoogly) added a project: User Interface.Jun 25 2020, 4:31 PM