From 80a46955d8212f01b77215ab12d479d934e305f4 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Thu, 4 Nov 2021 12:10:58 +0100 Subject: Fix T92501: Crash when dragging material assets over 3D View regions Issue was that the context used for dropbox handling and polling didn't match the one used for drawing the dropbox and generating the tooltip text (which would determine the material slot under the cursor, requiring context). The mismatch would happen with overlapping regions. Actually, this patch includes two fixes, each fixing the crash itself: * Store the context from handling & polling and restore it for drawing. * Correct the hovered region lookup for drawing to account for overlayed regions. Note that to properly set up context for drawing, we should also account for the operator context, which isn't done here, see https://developer.blender.org/T92501#1247581. --- source/blender/editors/include/ED_screen.h | 1 + .../editors/interface/interface_dropboxes.cc | 8 +-- source/blender/editors/screen/area_query.c | 45 +++++++++++++ source/blender/windowmanager/WM_types.h | 31 ++++++--- source/blender/windowmanager/intern/wm_dragdrop.c | 77 ++++++++++++---------- 5 files changed, 116 insertions(+), 46 deletions(-) (limited to 'source/blender') diff --git a/source/blender/editors/include/ED_screen.h b/source/blender/editors/include/ED_screen.h index 08b6c02a8d0..ef3ff7874df 100644 --- a/source/blender/editors/include/ED_screen.h +++ b/source/blender/editors/include/ED_screen.h @@ -439,6 +439,7 @@ bool ED_region_panel_category_gutter_calc_rect(const ARegion *region, rcti *r_re bool ED_region_panel_category_gutter_isect_xy(const ARegion *region, const int event_xy[2]); bool ED_region_contains_xy(const struct ARegion *region, const int event_xy[2]); +ARegion *ED_area_find_region_xy_visual(const ScrArea *area, int regiontype, const int event_xy[2]); /* interface_region_hud.c */ struct ARegionType *ED_area_type_hud(int space_type); diff --git a/source/blender/editors/interface/interface_dropboxes.cc b/source/blender/editors/interface/interface_dropboxes.cc index 81a1354cbe7..369efa7c52e 100644 --- a/source/blender/editors/interface/interface_dropboxes.cc +++ b/source/blender/editors/interface/interface_dropboxes.cc @@ -39,12 +39,12 @@ static bool ui_tree_view_drop_poll(bContext *C, wmDrag *drag, const wmEvent *eve return false; } - if (drag->free_disabled_info) { - MEM_SAFE_FREE(drag->disabled_info); + if (drag->drop_state.free_disabled_info) { + MEM_SAFE_FREE(drag->drop_state.disabled_info); } - drag->free_disabled_info = false; - return UI_tree_view_item_can_drop(hovered_tree_item, drag, &drag->disabled_info); + drag->drop_state.free_disabled_info = false; + return UI_tree_view_item_can_drop(hovered_tree_item, drag, &drag->drop_state.disabled_info); } static char *ui_tree_view_drop_tooltip(bContext *C, diff --git a/source/blender/editors/screen/area_query.c b/source/blender/editors/screen/area_query.c index fd4f3964398..30e744ca174 100644 --- a/source/blender/editors/screen/area_query.c +++ b/source/blender/editors/screen/area_query.c @@ -140,6 +140,10 @@ bool ED_region_overlap_isect_xy_with_margin(const ARegion *region, ED_region_overlap_isect_y_with_margin(region, event_xy[1], margin)); } +/** + * \note: This may return true for multiple overlapping regions. If it matters, check overlapped + * regions first (#ARegion.overlap). + */ bool ED_region_contains_xy(const ARegion *region, const int event_xy[2]) { /* Only use the margin when inside the region. */ @@ -188,3 +192,44 @@ bool ED_region_contains_xy(const ARegion *region, const int event_xy[2]) } return false; } + +/** + * Similar to #BKE_area_find_region_xy() but when \a event_xy intersects an overlapping region, + * this returns the region that is visually under the cursor. E.g. when over the + * transparent part of the region, it returns the region underneath. + * + * The overlapping region is determined using the #ED_region_contains_xy() query. + */ +ARegion *ED_area_find_region_xy_visual(const ScrArea *area, + const int regiontype, + const int event_xy[2]) +{ + if (!area) { + return NULL; + } + + /* Check overlapped regions first. */ + LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + if (!region->overlap) { + continue; + } + if (ELEM(regiontype, RGN_TYPE_ANY, region->regiontype)) { + if (ED_region_contains_xy(region, event_xy)) { + return region; + } + } + } + /* Now non-overlapping ones. */ + LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { + if (region->overlap) { + continue; + } + if (ELEM(regiontype, RGN_TYPE_ANY, region->regiontype)) { + if (ED_region_contains_xy(region, event_xy)) { + return region; + } + } + } + + return NULL; +} diff --git a/source/blender/windowmanager/WM_types.h b/source/blender/windowmanager/WM_types.h index 9f427a90353..27c8aa532f2 100644 --- a/source/blender/windowmanager/WM_types.h +++ b/source/blender/windowmanager/WM_types.h @@ -1030,6 +1030,27 @@ typedef char *(*WMDropboxTooltipFunc)(struct bContext *, const int xy[2], struct wmDropBox *drop); +typedef struct wmDragActiveDropState { + /** Informs which dropbox is activated with the drag item. + * When this value changes, the #draw_activate and #draw_deactivate dropbox callbacks are + * triggered. + */ + struct wmDropBox *active_dropbox; + + /** If `active_dropbox` is set, the area it successfully polled in. To restore the context of it + * as needed. */ + struct ScrArea *area_from; + /** If `active_dropbox` is set, the region it successfully polled in. To restore the context of + * it as needed. */ + struct ARegion *region_from; + + /** Text to show when a dropbox poll succeeds (so the dropbox itself is available) but the + * operator poll fails. Typically the message the operator set with + * CTX_wm_operator_poll_msg_set(). */ + const char *disabled_info; + bool free_disabled_info; +} wmDragActiveDropState; + typedef struct wmDrag { struct wmDrag *next, *prev; @@ -1045,15 +1066,7 @@ typedef struct wmDrag { float scale; int sx, sy; - /** Informs which dropbox is activated with the drag item. - * When this value changes, the #draw_activate and #draw_deactivate dropbox callbacks are - * triggered. - */ - struct wmDropBox *active_dropbox; - /* Text to show when the operator poll fails. Typically the message the - * operator set with CTX_wm_operator_poll_msg_set(). */ - const char *disabled_info; - bool free_disabled_info; + wmDragActiveDropState drop_state; unsigned int flags; diff --git a/source/blender/windowmanager/intern/wm_dragdrop.c b/source/blender/windowmanager/intern/wm_dragdrop.c index 37a101cc31d..85378ffd7c7 100644 --- a/source/blender/windowmanager/intern/wm_dragdrop.c +++ b/source/blender/windowmanager/intern/wm_dragdrop.c @@ -51,6 +51,7 @@ #include "BLO_readfile.h" #include "ED_asset.h" +#include "ED_screen.h" #include "GPU_shader.h" #include "GPU_state.h" @@ -217,7 +218,7 @@ void wm_drags_exit(wmWindowManager *wm, wmWindow *win) { bool any_active = false; LISTBASE_FOREACH (const wmDrag *, drag, &wm->drags) { - if (drag->active_dropbox) { + if (drag->drop_state.active_dropbox) { any_active = true; break; } @@ -260,14 +261,14 @@ void WM_drag_data_free(int dragtype, void *poin) void WM_drag_free(wmDrag *drag) { - if (drag->active_dropbox && drag->active_dropbox->draw_deactivate) { - drag->active_dropbox->draw_deactivate(drag->active_dropbox, drag); + if (drag->drop_state.active_dropbox && drag->drop_state.active_dropbox->draw_deactivate) { + drag->drop_state.active_dropbox->draw_deactivate(drag->drop_state.active_dropbox, drag); } if (drag->flags & WM_DRAG_FREE_DATA) { WM_drag_data_free(drag->type, drag->poin); } - if (drag->free_disabled_info) { - MEM_SAFE_FREE(drag->disabled_info); + if (drag->drop_state.free_disabled_info) { + MEM_SAFE_FREE(drag->drop_state.disabled_info); } BLI_freelistN(&drag->ids); LISTBASE_FOREACH_MUTABLE (wmDragAssetListItem *, asset_item, &drag->asset_items) { @@ -306,10 +307,10 @@ static wmDropBox *dropbox_active(bContext *C, wmDrag *drag, const wmEvent *event) { - if (drag->free_disabled_info) { - MEM_SAFE_FREE(drag->disabled_info); + if (drag->drop_state.free_disabled_info) { + MEM_SAFE_FREE(drag->drop_state.disabled_info); } - drag->disabled_info = NULL; + drag->drop_state.disabled_info = NULL; LISTBASE_FOREACH (wmEventHandler *, handler_base, handlers) { if (handler_base->type == WM_HANDLER_TYPE_DROPBOX) { @@ -332,8 +333,8 @@ static wmDropBox *dropbox_active(bContext *C, bool free_disabled_info = false; const char *disabled_hint = CTX_wm_operator_poll_msg_get(C, &free_disabled_info); if (disabled_hint) { - drag->disabled_info = disabled_hint; - drag->free_disabled_info = free_disabled_info; + drag->drop_state.disabled_info = disabled_hint; + drag->drop_state.free_disabled_info = free_disabled_info; } } } @@ -373,7 +374,7 @@ static void wm_drop_update_active(bContext *C, wmDrag *drag, const wmEvent *even return; } - wmDropBox *drop_prev = drag->active_dropbox; + wmDropBox *drop_prev = drag->drop_state.active_dropbox; wmDropBox *drop = wm_dropbox_active(C, drag, event); if (drop != drop_prev) { if (drop_prev && drop_prev->draw_deactivate) { @@ -383,7 +384,9 @@ static void wm_drop_update_active(bContext *C, wmDrag *drag, const wmEvent *even if (drop && drop->draw_activate) { drop->draw_activate(drop, drag); } - drag->active_dropbox = drop; + drag->drop_state.active_dropbox = drop; + drag->drop_state.area_from = drop ? CTX_wm_area(C) : NULL; + drag->drop_state.region_from = drop ? CTX_wm_region(C) : NULL; } } @@ -408,7 +411,7 @@ void wm_drags_check_ops(bContext *C, const wmEvent *event) LISTBASE_FOREACH (wmDrag *, drag, &wm->drags) { wm_drop_update_active(C, drag, event); - if (drag->active_dropbox) { + if (drag->drop_state.active_dropbox) { any_active = true; } } @@ -819,14 +822,14 @@ static void wm_drag_draw_tooltip(bContext *C, wmWindow *win, wmDrag *drag, const int iconsize = UI_DPI_ICON_SIZE; int padding = 4 * UI_DPI_FAC; - const char *tooltip = NULL; - bool free_tooltip = false; - if (drag->active_dropbox) { - tooltip = dropbox_tooltip(C, drag, xy, drag->active_dropbox); - free_tooltip = true; + char *tooltip = NULL; + if (drag->drop_state.active_dropbox) { + tooltip = dropbox_tooltip(C, drag, xy, drag->drop_state.active_dropbox); } - if (!tooltip && !drag->disabled_info) { + const bool has_disabled_info = drag->drop_state.disabled_info && + drag->drop_state.disabled_info[0]; + if (!tooltip && !has_disabled_info) { return; } @@ -855,12 +858,10 @@ static void wm_drag_draw_tooltip(bContext *C, wmWindow *win, wmDrag *drag, const if (tooltip) { wm_drop_operator_draw(tooltip, x, y); - if (free_tooltip) { - MEM_freeN((void *)tooltip); - } + MEM_freeN(tooltip); } - else if (drag->disabled_info) { - wm_drop_redalert_draw(drag->disabled_info, x, y); + else if (has_disabled_info) { + wm_drop_redalert_draw(drag->drop_state.disabled_info, x, y); } } @@ -905,22 +906,32 @@ void wm_drags_draw(bContext *C, wmWindow *win) } bScreen *screen = CTX_wm_screen(C); + /* To start with, use the area and region under the mouse cursor, just like event handling. The + * operator context may still override it. */ ScrArea *area = BKE_screen_find_area_xy(screen, SPACE_TYPE_ANY, UNPACK2(xy)); - ARegion *region = BKE_area_find_region_xy(area, RGN_TYPE_ANY, UNPACK2(xy)); - if (region) { - BLI_assert(!CTX_wm_area(C) && !CTX_wm_region(C)); - CTX_wm_area_set(C, area); - CTX_wm_region_set(C, region); - } + ARegion *region = ED_area_find_region_xy_visual(area, RGN_TYPE_ANY, xy); + /* Will be overridden and unset eventually. */ + BLI_assert(!CTX_wm_area(C) && !CTX_wm_region(C)); wmWindowManager *wm = CTX_wm_manager(C); /* Should we support multi-line drag draws? Maybe not, more types mixed won't work well. */ GPU_blend(GPU_BLEND_ALPHA); LISTBASE_FOREACH (wmDrag *, drag, &wm->drags) { - if (drag->active_dropbox && drag->active_dropbox->draw) { - drag->active_dropbox->draw(C, win, drag, xy); - continue; + if (drag->drop_state.active_dropbox) { + CTX_wm_area_set(C, drag->drop_state.area_from); + CTX_wm_region_set(C, drag->drop_state.region_from); + + /* Drawing should be allowed to assume the context from handling and polling (that's why we + * restore it above). */ + if (drag->drop_state.active_dropbox->draw) { + drag->drop_state.active_dropbox->draw(C, win, drag, xy); + continue; + } + } + else if (region) { + CTX_wm_area_set(C, area); + CTX_wm_region_set(C, region); } wm_drag_draw_default(C, win, drag, xy); -- cgit v1.2.3 From b5162638c58397f32e766882fa101ab683f455a5 Mon Sep 17 00:00:00 2001 From: Charlie Jolly Date: Thu, 4 Nov 2021 13:40:38 +0000 Subject: Fix: Geometry Nodes: Math node smoothmax not working Function arguments were incorrect. Noted during @simonthommes live stream. Reviewed By: JacquesLucke Differential Revision: https://developer.blender.org/D13110 --- source/blender/nodes/NOD_math_functions.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source/blender') diff --git a/source/blender/nodes/NOD_math_functions.hh b/source/blender/nodes/NOD_math_functions.hh index 9443be820d1..86ff8cab3e9 100644 --- a/source/blender/nodes/NOD_math_functions.hh +++ b/source/blender/nodes/NOD_math_functions.hh @@ -193,7 +193,7 @@ inline bool try_dispatch_float_math_fl_fl_fl_to_fl(const int operation, Callback case NODE_MATH_SMOOTH_MIN: return dispatch([](float a, float b, float c) { return smoothminf(a, b, c); }); case NODE_MATH_SMOOTH_MAX: - return dispatch([](float a, float b, float c) { return -smoothminf(-a, -b, -c); }); + return dispatch([](float a, float b, float c) { return -smoothminf(-a, -b, c); }); case NODE_MATH_WRAP: return dispatch([](float a, float b, float c) { return wrapf(a, b, c); }); } -- cgit v1.2.3 From ff4959eeaa5368d79f15d482da6576a1ccba5dca Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Thu, 4 Nov 2021 14:41:22 +0100 Subject: Fix T92649: incorrect copying of anonymous attributes in many places Many modifiers and other places use `CustomData_copy_data` to copy data between different meshes. This function assumes that assumes that the source and destination `CustomData` objects are "compatible" in some way. Usually modifiers use `CustomData_copy` to create a compatible new `CustomData` on the new mesh. The issue was that the optimization I added for anonymous attributes broke this compatibility. It avoided copying some attributes when they are no longer used. This lead to attributes being copied incorrectly. D13083 contains ideas for how this could be fixed more generally. For now I just removed the optimization. Differential Revision: https://developer.blender.org/D13083 --- source/blender/blenkernel/intern/customdata.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'source/blender') diff --git a/source/blender/blenkernel/intern/customdata.c b/source/blender/blenkernel/intern/customdata.c index d86b8163ebc..743d5471aa7 100644 --- a/source/blender/blenkernel/intern/customdata.c +++ b/source/blender/blenkernel/intern/customdata.c @@ -2131,11 +2131,6 @@ bool CustomData_merge(const struct CustomData *source, if (flag & CD_FLAG_NOCOPY) { continue; } - if (layer->anonymous_id && - !BKE_anonymous_attribute_id_has_strong_references(layer->anonymous_id)) { - /* This attribute is not referenced anymore, so it can be treated as if it didn't exist. */ - continue; - } if (!(mask & CD_TYPE_AS_MASK(type))) { continue; } -- cgit v1.2.3 From be4478d1f8b6c75b50c951f02cf0116f78e68d6d Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Thu, 4 Nov 2021 14:44:21 +0100 Subject: Fix T92814: improve automatic linking when inserting Float Curve node This solves the issue in a more general that can also be used to solve similar issues for other nodes in the future. Nodes can specify their "main" socket in their declaration so that we don't have to rely on heuristics. Differential Revision: https://developer.blender.org/D13108 --- .../editors/space_node/node_relationships.cc | 32 +++++++++++++++++++--- source/blender/nodes/NOD_node_declaration.hh | 13 +++++++++ .../nodes/shader/nodes/node_shader_curves.cc | 2 +- 3 files changed, 42 insertions(+), 5 deletions(-) (limited to 'source/blender') diff --git a/source/blender/editors/space_node/node_relationships.cc b/source/blender/editors/space_node/node_relationships.cc index 76aad684b4c..55b547d3195 100644 --- a/source/blender/editors/space_node/node_relationships.cc +++ b/source/blender/editors/space_node/node_relationships.cc @@ -57,6 +57,7 @@ #include "BLT_translation.h" +#include "NOD_node_declaration.hh" #include "NOD_node_tree_ref.hh" #include "node_intern.h" /* own include */ @@ -2182,9 +2183,32 @@ static int get_main_socket_priority(const bNodeSocket *socket) return -1; } -/** Get the "main" socket of a socket list using a heuristic based on socket types. */ -static bNodeSocket *get_main_socket(ListBase *sockets) +/** Get the "main" socket based on the node declaration or an heuristic. */ +static bNodeSocket *get_main_socket(bNodeTree &ntree, bNode &node, eNodeSocketInOut in_out) { + using namespace blender; + using namespace blender::nodes; + + ListBase *sockets = (in_out == SOCK_IN) ? &node.inputs : &node.outputs; + + /* Try to get the main socket based on the socket declaration. */ + nodeDeclarationEnsure(&ntree, &node); + const NodeDeclaration *node_decl = node.declaration; + if (node_decl != nullptr) { + Span socket_decls = (in_out == SOCK_IN) ? node_decl->inputs() : + node_decl->outputs(); + int index; + LISTBASE_FOREACH_INDEX (bNodeSocket *, socket, sockets, index) { + const SocketDeclaration &socket_decl = *socket_decls[index]; + if (nodeSocketIsHidden(socket)) { + continue; + } + if (socket_decl.is_default_link_socket()) { + return socket; + } + } + } + /* find priority range */ int maxpriority = -1; LISTBASE_FOREACH (bNodeSocket *, sock, sockets) { @@ -2561,8 +2585,8 @@ void ED_node_link_insert(Main *bmain, ScrArea *area) } if (link) { - bNodeSocket *best_input = get_main_socket(&select->inputs); - bNodeSocket *best_output = get_main_socket(&select->outputs); + bNodeSocket *best_input = get_main_socket(*snode->edittree, *select, SOCK_IN); + bNodeSocket *best_output = get_main_socket(*snode->edittree, *select, SOCK_OUT); if (best_input && best_output) { bNode *node = link->tonode; diff --git a/source/blender/nodes/NOD_node_declaration.hh b/source/blender/nodes/NOD_node_declaration.hh index 1481e69c00e..e9f2996bb30 100644 --- a/source/blender/nodes/NOD_node_declaration.hh +++ b/source/blender/nodes/NOD_node_declaration.hh @@ -89,6 +89,7 @@ class SocketDeclaration { bool is_multi_input_ = false; bool no_mute_links_ = false; bool is_attribute_name_ = false; + bool is_default_link_socket_ = false; InputSocketFieldType input_field_type_ = InputSocketFieldType::None; OutputFieldDependency output_field_dependency_; @@ -107,6 +108,7 @@ class SocketDeclaration { StringRefNull description() const; StringRefNull identifier() const; bool is_attribute_name() const; + bool is_default_link_socket() const; InputSocketFieldType input_field_type() const; const OutputFieldDependency &output_field_dependency() const; @@ -171,6 +173,12 @@ class SocketDeclarationBuilder : public BaseSocketDeclarationBuilder { return *(Self *)this; } + Self &is_default_link_socket(bool value = true) + { + decl_->is_default_link_socket_ = value; + return *(Self *)this; + } + /** The input socket allows passing in a field. */ Self &supports_field() { @@ -363,6 +371,11 @@ inline bool SocketDeclaration::is_attribute_name() const return is_attribute_name_; } +inline bool SocketDeclaration::is_default_link_socket() const +{ + return is_default_link_socket_; +} + inline InputSocketFieldType SocketDeclaration::input_field_type() const { return input_field_type_; diff --git a/source/blender/nodes/shader/nodes/node_shader_curves.cc b/source/blender/nodes/shader/nodes/node_shader_curves.cc index f8f0ee97eae..7ce5150bf85 100644 --- a/source/blender/nodes/shader/nodes/node_shader_curves.cc +++ b/source/blender/nodes/shader/nodes/node_shader_curves.cc @@ -357,7 +357,7 @@ static void sh_node_curve_float_declare(NodeDeclarationBuilder &b) .max(1.0f) .default_value(1.0f) .subtype(PROP_FACTOR); - b.add_input(N_("Value")).default_value(1.0f); + b.add_input(N_("Value")).default_value(1.0f).is_default_link_socket(); b.add_output(N_("Value")); }; -- cgit v1.2.3