瀏覽代碼

Merge pull request #46471 from nmrkr/3.2-drag-preview-crash-after-free

[3.2] Fix crash during drag if user freed the drag preview
Rémi Verschelde 4 年之前
父節點
當前提交
1717e16c9e
共有 3 個文件被更改,包括 47 次插入23 次删除
  1. 1 1
      doc/classes/Control.xml
  2. 44 21
      scene/main/viewport.cpp
  3. 2 1
      scene/main/viewport.h

+ 1 - 1
doc/classes/Control.xml

@@ -693,7 +693,7 @@
 			<argument index="0" name="control" type="Control">
 			<argument index="0" name="control" type="Control">
 			</argument>
 			</argument>
 			<description>
 			<description>
-				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]
 				[codeblock]
 				export (Color, RGBA) var color = Color(1, 0, 0, 1)
 				export (Color, RGBA) var color = Color(1, 0, 0, 1)
 
 

+ 44 - 21
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();
 	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;
 		r_inv_xform = matrix;
 		return c;
 		return c;
-	} else
-		return NULL;
+	}
+
+	return nullptr;
 }
 }
 
 
 bool Viewport::_gui_drop(Control *p_at_control, Point2 p_at_pos, bool p_just_check) {
 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<InputEvent> p_event) {
 				gui.drag_data = Variant();
 				gui.drag_data = Variant();
 				gui.dragging = false;
 				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);
 				_propagate_viewport_notification(this, NOTIFICATION_DRAG_END);
 				//change mouse accordingly
 				//change mouse accordingly
@@ -2060,9 +2066,10 @@ void Viewport::_gui_input_event(Ref<InputEvent> p_event) {
 					_gui_drop(gui.mouse_over, pos, false);
 					_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();
 				gui.drag_data = Variant();
@@ -2163,10 +2170,11 @@ void Viewport::_gui_input_event(Ref<InputEvent> p_event) {
 								gui.mouse_focus_mask = 0;
 								gui.mouse_focus_mask = 0;
 								break;
 								break;
 							} else {
 							} 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.");
 									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;
 								gui.dragging = false;
 							}
 							}
@@ -2254,8 +2262,9 @@ void Viewport::_gui_input_event(Ref<InputEvent> p_event) {
 
 
 		gui.mouse_over = over;
 		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) {
 		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->is_inside_tree());
 	ERR_FAIL_COND(p_control->get_parent() != NULL);
 	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_as_toplevel(true);
 	p_control->set_position(gui.last_mouse_pos);
 	p_control->set_position(gui.last_mouse_pos);
 	p_base->get_root_parent_control()->add_child(p_control); //add as child of viewport
 	p_base->get_root_parent_control()->add_child(p_control); //add as child of viewport
 	p_control->raise();
 	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<Control>(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<Control *>::Element *RI) {
 void Viewport::_gui_remove_root_control(List<Control *>::Element *RI) {
@@ -3505,7 +3528,7 @@ Viewport::Viewport() {
 
 
 	gui.tooltip_control = NULL;
 	gui.tooltip_control = NULL;
 	gui.tooltip_label = NULL;
 	gui.tooltip_label = NULL;
-	gui.drag_preview = NULL;
+	gui.drag_preview_id = 0;
 	gui.drag_attempted = false;
 	gui.drag_attempted = false;
 	gui.canvas_sort_index = 0;
 	gui.canvas_sort_index = 0;
 	gui.roots_order_dirty = false;
 	gui.roots_order_dirty = false;

+ 2 - 1
scene/main/viewport.h

@@ -307,7 +307,7 @@ private:
 		Point2 drag_accum;
 		Point2 drag_accum;
 		bool drag_attempted;
 		bool drag_attempted;
 		Variant drag_data;
 		Variant drag_data;
-		Control *drag_preview;
+		ObjectID drag_preview_id;
 		float tooltip_timer;
 		float tooltip_timer;
 		float tooltip_delay;
 		float tooltip_delay;
 		List<Control *> modal_stack;
 		List<Control *> modal_stack;
@@ -369,6 +369,7 @@ private:
 
 
 	void _gui_force_drag(Control *p_base, const Variant &p_data, Control *p_control);
 	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);
 	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);
 	bool _gui_is_modal_on_top(const Control *p_control);
 	List<Control *>::Element *_gui_show_modal(Control *p_control);
 	List<Control *>::Element *_gui_show_modal(Control *p_control);