blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 780 Projects 19 Wiki Activity

Fix: crash when selecting a brush asset from the wrong mode #117233

Merged
Brecht Van Lommel merged 5 commits from brecht/blender:brush-asset-fix-mode-crash into brush-assets-project 2024-01-17 17:31:01 +01:00
Conversation 3 Commits 5 Files Changed 3 +21 -4
3 changed files with 21 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/blender/blenkernel/BKE_paint.hh
View File

@ -207,7 +207,7 @@ void BKE_paint_brush_set(Paint *paint, Brush *br);
* Set the active brush of given paint struct, and store the weak asset reference to it.
* \note Takes ownership of the given `weak_asset_reference`.
*/
void BKE_paint_brush_asset_set(Paint *paint,
bool BKE_paint_brush_asset_set(Paint *paint,
Brush *brush,
AssetWeakReference *weak_asset_reference);

9
source/blender/blenkernel/intern/paint.cc
View File

@ -691,10 +691,17 @@ static void paint_brush_asset_update(Paint &paint,
paint.brush_asset_reference = brush_asset_reference;
}
void BKE_paint_brush_asset_set(Paint *paint,
bool BKE_paint_brush_asset_set(Paint *paint,
Brush *brush,
AssetWeakReference *weak_asset_reference)
brecht marked this conversation as resolved Outdated
Hans Goudey commented 2024-01-17 16:08:26 +01:00
Outdated
Review
Copy Link

ensure -> ensured

`ensure` -> `ensured`
{
/* Should not happen for users if brush assets are properly filtered by mode, but still protect
* against it in case of invalid API usage. */
if (paint->runtime.ob_mode != brush->ob_mode) {
BKE_asset_weak_reference_free(&weak_asset_reference);
return false;
}
BKE_paint_brush_set(paint, brush);
paint_brush_asset_update(*paint, brush, weak_asset_reference);
}

14
source/blender/editors/sculpt_paint/curves_sculpt_ops.cc
View File

@ -18,6 +18,7 @@
#include "BKE_modifier.hh"
#include "BKE_object.hh"
#include "BKE_paint.hh"
#include "BKE_report.h"
#include "WM_api.hh"
#include "WM_message.hh"
@ -1165,7 +1166,7 @@ static void SCULPT_CURVES_OT_min_distance_edit(wmOperatorType *ot)
/* -------------------------------------------------------------------- */
static int brush_asset_select_exec(bContext *C, wmOperator * /*op*/)
static int brush_asset_select_exec(bContext *C, wmOperator *op)
{
/* This operator currently covers both cases: the file/asset browser file list and the asset list
* used for the asset-view template. Once the asset list design is used by the Asset Browser,
@ -1179,8 +1180,17 @@ static int brush_asset_select_exec(bContext *C, wmOperator * /*op*/)
Brush *brush = BKE_brush_asset_runtime_ensure(CTX_data_main(C), brush_asset_reference);
brecht marked this conversation as resolved Outdated
Hans Goudey commented 2024-01-17 16:09:50 +01:00
Outdated
Review
Copy Link

I guess this should check the return argument and return OPERATOR_CANCELLED and give an error

I guess this should check the return argument and return `OPERATOR_CANCELLED` and give an error
Brecht Van Lommel commented 2024-01-17 16:34:22 +01:00
Outdated
Review
Copy Link

It's not a no-op right now, because the brush did get added to the main database. Just not actually selected for painting.

I can try to do the mode check somewhere inside BKE_brush_asset_runtime_ensure to really make it a no-op. It doesn't make a lot of difference in practice since this operator doesn't have OPTYPE_UNDO or OPTYPE_REGISTER, but still more elegant.

It's not a no-op right now, because the brush did get added to the main database. Just not actually selected for painting. I can try to do the mode check somewhere inside `BKE_brush_asset_runtime_ensure` to really make it a no-op. It doesn't make a lot of difference in practice since this operator doesn't have `OPTYPE_UNDO` or `OPTYPE_REGISTER`, but still more elegant.
Hans Goudey commented 2024-01-17 16:37:55 +01:00
Outdated
Review
Copy Link

Ah, good point. Maybe just a RPT_WARNING then

Ah, good point. Maybe just a `RPT_WARNING` then
ToolSettings *tool_settings = CTX_data_tool_settings(C);
/* Either takes ownership of the brush_asset_reference, or frees it. */
BKE_paint_brush_asset_set(&tool_settings->curves_sculpt->paint, brush, brush_asset_reference);
if (!BKE_paint_brush_asset_set(
&tool_settings->curves_sculpt->paint, brush, brush_asset_reference))
{
/* Note brush datablock was still added, so was not a no-op. */
BKE_report(op->reports, RPT_WARNING, "Unable to select brush, wrong object mode");
return OPERATOR_FINISHED;
}
WM_main_add_notifier(NC_SCENE | ND_TOOLSETTINGS, nullptr);
return OPERATOR_FINISHED;
}