From 7df977c3edde2136480f63211ecc3d672926db79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Delf=20Neum=C3=A4rker?= Date: Sat, 27 Feb 2021 10:08:51 +0100 Subject: [PATCH] Fix crash during drag if user freed the drag preview --- doc/classes/Control.xml | 2 +- scene/main/viewport.cpp | 65 ++++++++++++++++++++++++++++------------- scene/main/viewport.h | 3 +- 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/doc/classes/Control.xml b/doc/classes/Control.xml index 68ffd6efb45..ac4d2949707 100644 --- a/doc/classes/Control.xml +++ b/doc/classes/Control.xml @@ -693,7 +693,7 @@ - Shows the given control at the mouse pointer. A good time to call this method is in [method get_drag_data]. The control must not be in the scene tree. + Shows the given control at the mouse pointer. A good time to call this method is in [method get_drag_data]. The control must not be in the scene tree. You should not free the control, and you should not keep a reference to the control beyond the duration of the drag. It will be deleted automatically after the drag has ended. [codeblock] export (Color, RGBA) var color = Color(1, 0, 0, 1) diff --git a/scene/main/viewport.cpp b/scene/main/viewport.cpp index 8e2989459bc..4abf019c1c7 100644 --- a/scene/main/viewport.cpp +++ b/scene/main/viewport.cpp @@ -1844,17 +1844,22 @@ Control *Viewport::_gui_find_control_at_pos(CanvasItem *p_node, const Point2 &p_ } } - if (!c) - return NULL; + if (!c || c->data.mouse_filter == Control::MOUSE_FILTER_IGNORE) { + return nullptr; + } matrix.affine_invert(); + if (!c->has_point(matrix.xform(p_global))) { + return nullptr; + } - //conditions for considering this as a valid control for return - if (c->data.mouse_filter != Control::MOUSE_FILTER_IGNORE && c->has_point(matrix.xform(p_global)) && (!gui.drag_preview || (c != gui.drag_preview && !gui.drag_preview->is_a_parent_of(c)))) { + Control *drag_preview = _gui_get_drag_preview(); + if (!drag_preview || (c != drag_preview && !drag_preview->is_a_parent_of(c))) { r_inv_xform = matrix; return c; - } else - return NULL; + } + + return nullptr; } bool Viewport::_gui_drop(Control *p_at_control, Point2 p_at_pos, bool p_just_check) { @@ -2040,9 +2045,10 @@ void Viewport::_gui_input_event(Ref p_event) { gui.drag_data = Variant(); gui.dragging = false; - if (gui.drag_preview) { - memdelete(gui.drag_preview); - gui.drag_preview = NULL; + Control *drag_preview = _gui_get_drag_preview(); + if (drag_preview) { + memdelete(drag_preview); + gui.drag_preview_id = 0; } _propagate_viewport_notification(this, NOTIFICATION_DRAG_END); //change mouse accordingly @@ -2060,9 +2066,10 @@ void Viewport::_gui_input_event(Ref p_event) { _gui_drop(gui.mouse_over, pos, false); } - if (gui.drag_preview && mb->get_button_index() == BUTTON_LEFT) { - memdelete(gui.drag_preview); - gui.drag_preview = NULL; + Control *drag_preview = _gui_get_drag_preview(); + if (drag_preview) { + memdelete(drag_preview); + gui.drag_preview_id = 0; } gui.drag_data = Variant(); @@ -2163,10 +2170,11 @@ void Viewport::_gui_input_event(Ref p_event) { gui.mouse_focus_mask = 0; break; } else { - if (gui.drag_preview != NULL) { + Control *drag_preview = _gui_get_drag_preview(); + if (drag_preview) { ERR_PRINT("Don't set a drag preview and return null data. Preview was deleted and drag request ignored."); - memdelete(gui.drag_preview); - gui.drag_preview = NULL; + memdelete(drag_preview); + gui.drag_preview_id = 0; } gui.dragging = false; } @@ -2254,8 +2262,9 @@ void Viewport::_gui_input_event(Ref p_event) { gui.mouse_over = over; - if (gui.drag_preview) { - gui.drag_preview->set_position(mpos); + Control *drag_preview = _gui_get_drag_preview(); + if (drag_preview) { + drag_preview->set_position(mpos); } if (!over) { @@ -2632,15 +2641,29 @@ void Viewport::_gui_set_drag_preview(Control *p_base, Control *p_control) { ERR_FAIL_COND(p_control->is_inside_tree()); ERR_FAIL_COND(p_control->get_parent() != NULL); - if (gui.drag_preview) { - memdelete(gui.drag_preview); + Control *drag_preview = _gui_get_drag_preview(); + if (drag_preview) { + memdelete(drag_preview); } p_control->set_as_toplevel(true); p_control->set_position(gui.last_mouse_pos); p_base->get_root_parent_control()->add_child(p_control); //add as child of viewport p_control->raise(); - gui.drag_preview = p_control; + gui.drag_preview_id = p_control->get_instance_id(); +} + +Control *Viewport::_gui_get_drag_preview() { + if (!gui.drag_preview_id) { + return nullptr; + } else { + Control *drag_preview = Object::cast_to(ObjectDB::get_instance(gui.drag_preview_id)); + if (!drag_preview) { + ERR_PRINT("Don't free the control set as drag preview."); + gui.drag_preview_id = 0; + } + return drag_preview; + } } void Viewport::_gui_remove_root_control(List::Element *RI) { @@ -3505,7 +3528,7 @@ Viewport::Viewport() { gui.tooltip_control = NULL; gui.tooltip_label = NULL; - gui.drag_preview = NULL; + gui.drag_preview_id = 0; gui.drag_attempted = false; gui.canvas_sort_index = 0; gui.roots_order_dirty = false; diff --git a/scene/main/viewport.h b/scene/main/viewport.h index 32b374c801b..f9d91e4b443 100644 --- a/scene/main/viewport.h +++ b/scene/main/viewport.h @@ -307,7 +307,7 @@ private: Point2 drag_accum; bool drag_attempted; Variant drag_data; - Control *drag_preview; + ObjectID drag_preview_id; float tooltip_timer; float tooltip_delay; List modal_stack; @@ -369,6 +369,7 @@ private: void _gui_force_drag(Control *p_base, const Variant &p_data, Control *p_control); void _gui_set_drag_preview(Control *p_base, Control *p_control); + Control *_gui_get_drag_preview(); bool _gui_is_modal_on_top(const Control *p_control); List::Element *_gui_show_modal(Control *p_control);