From 2a1004f7af9dab255d95eea01c4779ffa6af89f0 Mon Sep 17 00:00:00 2001
From: Julian Eisel <julian@blender.org>
Date: Mon, 30 Sep 2024 16:12:24 +0200
Subject: [PATCH] Fix #128335: Activating brush from asset shelf sets tool, but
 not brush

We'd first set the brush, then change tools, which would again set the
brush to what it was before (switching tools tries to remember its last
used brush). Instead, switch the brush after switching the tool.

Also added a brush API check to test if the brush is valid for the
current mode, so that we don't end up switching tools in that case. This
matches previous behavior where this would be checked as part of setting
the brush already, causing the function to early exit before the tool is
changed.
---
 source/blender/blenkernel/BKE_paint.hh          |  8 +++++++-
 source/blender/blenkernel/intern/paint.cc       | 17 +++++++++++------
 .../windowmanager/intern/wm_toolsystem.cc       |  9 ++++++++-
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/source/blender/blenkernel/BKE_paint.hh b/source/blender/blenkernel/BKE_paint.hh
index 27f5f7923d1..509e4cb3329 100644
--- a/source/blender/blenkernel/BKE_paint.hh
+++ b/source/blender/blenkernel/BKE_paint.hh
@@ -213,9 +213,15 @@ Brush *BKE_paint_brush(Paint *paint);
 const Brush *BKE_paint_brush_for_read(const Paint *paint);
 Brush *BKE_paint_brush_from_essentials(Main *bmain, eObjectMode obmode, const char *name);
 
+/**
+ * Check if brush \a brush may be set/activated for \a paint. Passing null for \a brush will return
+ * true.
+ */
+bool BKE_paint_brush_poll(const Paint *paint, const Brush *brush);
+
 /**
  * Activates \a brush for painting, and updates #Paint.brush_asset_reference so the brush can be
- * restored after file read.
+ * restored after file read. No change is done if #BKE_paint_brush_poll() returns false.
  *
  * \return True on success. If \a brush is already active, this is considered a success (the brush
  * asset reference will still be updated).
diff --git a/source/blender/blenkernel/intern/paint.cc b/source/blender/blenkernel/intern/paint.cc
index bfe953845a8..75bf5f30707 100644
--- a/source/blender/blenkernel/intern/paint.cc
+++ b/source/blender/blenkernel/intern/paint.cc
@@ -646,6 +646,14 @@ const Brush *BKE_paint_brush_for_read(const Paint *paint)
   return paint ? paint->brush : nullptr;
 }
 
+bool BKE_paint_brush_poll(const Paint *paint, const Brush *brush)
+{
+  if (paint == nullptr) {
+    return false;
+  }
+  return !brush || (paint->runtime.ob_mode & brush->ob_mode) != 0;
+}
+
 static AssetWeakReference *asset_reference_create_from_brush(Brush *brush)
 {
   if (std::optional<AssetWeakReference> weak_ref = blender::bke::asset_edit_weak_reference_from_id(
@@ -671,8 +679,8 @@ bool BKE_paint_brush_set(Main *bmain,
   BLI_assert(brush == nullptr || blender::bke::asset_edit_id_is_editable(brush->id));
 
   /* Ensure we have a brush with appropriate mode to assign.
-   * Could happen if contents of asset blend was manually changed. */
-  if (brush && (paint->runtime.ob_mode & brush->ob_mode) == 0) {
+   * Could happen if contents of asset blend were manually changed. */
+  if (!BKE_paint_brush_poll(paint, brush)) {
     return false;
   }
 
@@ -694,10 +702,7 @@ bool BKE_paint_brush_set(Main *bmain,
 
 bool BKE_paint_brush_set(Paint *paint, Brush *brush)
 {
-  if (paint == nullptr) {
-    return false;
-  }
-  if (brush && (paint->runtime.ob_mode & brush->ob_mode) == 0) {
+  if (!BKE_paint_brush_poll(paint, brush)) {
     return false;
   }
 
diff --git a/source/blender/windowmanager/intern/wm_toolsystem.cc b/source/blender/windowmanager/intern/wm_toolsystem.cc
index 8c4eaa01364..8b9a64b7f03 100644
--- a/source/blender/windowmanager/intern/wm_toolsystem.cc
+++ b/source/blender/windowmanager/intern/wm_toolsystem.cc
@@ -270,7 +270,8 @@ bool WM_toolsystem_activate_brush_and_tool(bContext *C, Paint *paint, Brush *bru
   const bToolRef *active_tool = toolsystem_active_tool_from_context_or_view3d(C);
   const PaintMode paint_mode = BKE_paintmode_get_active_from_context(C);
 
-  if (!BKE_paint_brush_set(paint, brush)) {
+  if (!BKE_paint_brush_poll(paint, brush)) {
+    /* Avoid switching tool when brush isn't valid for this mode anyway. */
     return false;
   }
 
@@ -288,6 +289,12 @@ bool WM_toolsystem_activate_brush_and_tool(bContext *C, Paint *paint, Brush *bru
     }
   }
 
+  /* Do after switching tool, since switching tool will attempt to restore the last used brush of
+   * that tool (in #toolsystem_brush_activate_from_toolref_for_object_paint()). */
+  if (!BKE_paint_brush_set(paint, brush)) {
+    return false;
+  }
+
   if (active_tool->runtime->brush_type == -1) {
     /* Only update the main brush binding to reference the newly active brush. */
     toolsystem_main_brush_binding_update_from_active(paint);