From 03018c2f56855c6c9006f7feace63febdef7bc52 Mon Sep 17 00:00:00 2001 From: Stefano Facchini Date: Fri, 20 Oct 2017 17:10:39 +0200 Subject: Refactor SPDesktop::setEventContext to allow for unsetting the current tool Passing the empty string as toolName has the effect of unsetting and freeing the current tool. This will be used in a future commit. --- src/desktop.cpp | 44 ++++++++++++++++++++++---------------------- src/desktop.h | 5 +---- src/ui/tools-switch.cpp | 2 +- 3 files changed, 24 insertions(+), 27 deletions(-) (limited to 'src') diff --git a/src/desktop.cpp b/src/desktop.cpp index 1264e0184..61a64fe22 100644 --- a/src/desktop.cpp +++ b/src/desktop.cpp @@ -228,7 +228,7 @@ SPDesktop::init (SPNamedView *nv, SPCanvas *aCanvas, Inkscape::UI::View::EditWid controls = (SPCanvasGroup *) sp_canvas_item_new (main, SP_TYPE_CANVAS_GROUP, NULL); // Set the select tool as the active tool. - set_event_context2("/tools/select"); + setEventContext("/tools/select"); // display rect and zoom are now handled in sp_desktop_widget_realize() @@ -656,34 +656,34 @@ SPDesktop::change_document (SPDocument *theDocument) } /** - * Replaces the currently active tool with a new one. + * Replaces the currently active tool with a new one. Pass the empty string to + * unset and free the current tool. */ -void SPDesktop::set_event_context2(const std::string& toolName) +void SPDesktop::setEventContext(const std::string& toolName) { - Inkscape::UI::Tools::ToolBase* old_tool = event_context; - - if (old_tool) { - if (toolName.compare(old_tool->pref_observer->observed_path) != 0) { - //g_message("Old tool: %s", old_tool->pref_observer->observed_path.c_str()); - //g_message("New tool: %s", toolName.c_str()); - old_tool->finish(); - delete old_tool; + if (event_context) { + if (toolName.compare(event_context->pref_observer->observed_path) != 0) { + event_context->finish(); + delete event_context; } else { _event_context_changed_signal.emit(this, event_context); return; } } - - Inkscape::UI::Tools::ToolBase* new_tool = ToolFactory::createObject(toolName); - new_tool->desktop = this; - new_tool->message_context = new Inkscape::MessageContext(this->messageStack()); - event_context = new_tool; - new_tool->setup(); - - // Make sure no delayed snapping events are carried over after switching tools - // (this is only an additional safety measure against sloppy coding, because each - // tool should take care of this by itself) - sp_event_context_discard_delayed_snap_event(event_context); + + if (toolName.empty()) { + event_context = nullptr; + } else { + event_context = ToolFactory::createObject(toolName); + event_context->desktop = this; + event_context->message_context = new Inkscape::MessageContext(this->messageStack()); + event_context->setup(); + + // Make sure no delayed snapping events are carried over after switching tools + // (this is only an additional safety measure against sloppy coding, because each + // tool should take care of this by itself) + sp_event_context_discard_delayed_snap_event(event_context); + } _event_context_changed_signal.emit(this, event_context); } diff --git a/src/desktop.h b/src/desktop.h index 330d95607..0ddf3805b 100644 --- a/src/desktop.h +++ b/src/desktop.h @@ -310,10 +310,7 @@ public: void change_document (SPDocument *document); - void set_event_context2(const std::string& toolName); - - //void set_event_context (GType type, const gchar *config); - //void push_event_context (GType type, const gchar *config, unsigned int key); + void setEventContext(const std::string& toolName); void set_coordinate_status (Geom::Point p); SPItem *getItemFromListAtPointBottom(const std::vector &list, Geom::Point const &p) const; diff --git a/src/ui/tools-switch.cpp b/src/ui/tools-switch.cpp index d87bcc51d..e2803ccfd 100644 --- a/src/ui/tools-switch.cpp +++ b/src/ui/tools-switch.cpp @@ -157,7 +157,7 @@ tools_switch(SPDesktop *dt, int num) dt->_tool_changed.emit(num); } - dt->set_event_context2(tool_names[num]); + dt->setEventContext(tool_names[num]); /* fixme: This is really ugly hack. We should bind and unbind class methods */ /* First 4 tools use guides, first is undefined but we don't care */ dt->activate_guides(num < 5); -- cgit v1.2.3 From 2adf86d21d42f7d225e0d2188109fbd031989cf4 Mon Sep 17 00:00:00 2001 From: Stefano Facchini Date: Fri, 20 Oct 2017 22:40:38 +0200 Subject: Unset the current tool early on shutdown. Before this commit, when the application is terminated we call the ::finish() method for the current tool, only after deleting the selection and other objects. But it may happen that the tool's finish() assume that the selection is still alive, making the application crash. (For instance, with the Bezier tool when the path is still not closed). Instead, unset the current tool early, when the Desktop object is removed from the application, before calling its ::destroy() method. --- src/desktop.cpp | 8 +------- src/inkscape.cpp | 2 ++ src/widgets/desktop-widget.cpp | 3 +-- 3 files changed, 4 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/desktop.cpp b/src/desktop.cpp index 61a64fe22..f798079fd 100644 --- a/src/desktop.cpp +++ b/src/desktop.cpp @@ -322,7 +322,6 @@ SPDesktop::init (SPNamedView *nv, SPCanvas *aCanvas, Inkscape::UI::View::EditWid // canvas_debug = sp_canvas_item_new (main, SP_TYPE_CANVAS_DEBUG, NULL); } - void SPDesktop::destroy() { _destroy_signal.emit(this); @@ -331,6 +330,7 @@ void SPDesktop::destroy() delete snapindicator; snapindicator = NULL; } + if (temporary_item_list) { delete temporary_item_list; temporary_item_list = NULL; @@ -356,12 +356,6 @@ void SPDesktop::destroy() g_signal_handlers_disconnect_by_func(G_OBJECT (main), (gpointer) G_CALLBACK(sp_desktop_root_handler), this); g_signal_handlers_disconnect_by_func(G_OBJECT (drawing), (gpointer) G_CALLBACK(_arena_handler), this); - if (event_context) { - event_context->finish(); - delete event_context; - event_context = NULL; - } - delete layers; if (layer_manager) { diff --git a/src/inkscape.cpp b/src/inkscape.cpp index e7e93929b..b30d06168 100644 --- a/src/inkscape.cpp +++ b/src/inkscape.cpp @@ -864,6 +864,8 @@ Application::remove_desktop (SPDesktop * desktop) g_error("Attempted to remove desktop not in list."); } + desktop->setEventContext(""); + if (DESKTOP_IS_ACTIVE (desktop)) { signal_deactivate_desktop.emit(desktop); if (_desktops->size() > 1) { diff --git a/src/widgets/desktop-widget.cpp b/src/widgets/desktop-widget.cpp index 571f920bf..f9c8e4ac6 100644 --- a/src/widgets/desktop-widget.cpp +++ b/src/widgets/desktop-widget.cpp @@ -736,10 +736,9 @@ static void sp_desktop_widget_dispose(GObject *object) g_signal_handlers_disconnect_by_func (G_OBJECT (dtw->canvas), (gpointer) G_CALLBACK (sp_desktop_widget_event), dtw); g_signal_handlers_disconnect_by_func (G_OBJECT (dtw->canvas_tbl), (gpointer) G_CALLBACK (canvas_tbl_size_allocate), dtw); - dtw->layer_selector->setDesktop(NULL); dtw->layer_selector->unreference(); - INKSCAPE.remove_desktop (dtw->desktop); // clears selection too + INKSCAPE.remove_desktop(dtw->desktop); // clears selection and event_context dtw->modified_connection.disconnect(); dtw->desktop->destroy(); Inkscape::GC::release (dtw->desktop); -- cgit v1.2.3 From 93d72cb93dab69306a4dcb525e09e8524798639f Mon Sep 17 00:00:00 2001 From: Stefano Facchini Date: Wed, 25 Oct 2017 13:56:42 +0200 Subject: SPMeshArray: fix a crash when dragging control points It used to work with the old GSList code because objects where prepended instead of appended, so they were freed in the "right" order. But actually it's enough to delete the row objects, the children are then automatically deleted. --- src/sp-mesh-array.cpp | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/sp-mesh-array.cpp b/src/sp-mesh-array.cpp index a8d9e589c..905ad97eb 100644 --- a/src/sp-mesh-array.cpp +++ b/src/sp-mesh-array.cpp @@ -935,27 +935,18 @@ void SPMeshNodeArray::write( SPMeshGradient *mg ) { mg_array = mg; } - // First we must delete reprs for old mesh rows and patches. - std::vector descendant_objects; - std::vector descendant_reprs; - for (auto& row: mg_array->children) { - descendant_reprs.push_back(row.getRepr()); - descendant_objects.push_back(&row); - for (auto& patch: row.children) { - descendant_reprs.push_back(patch.getRepr()); - descendant_objects.push_back(&patch); - for (auto& stop: patch.children) { - descendant_reprs.push_back(stop.getRepr()); - descendant_objects.push_back(&stop); - } - } + // First we must delete reprs for old mesh rows and patches. We only need to call the + // deleteObject() method, which in turn calls sp_repr_unparent. Since iterators do not play + // well with boost::intrusive::list (which ChildrenList derive from) we need to iterate over a + // copy of the pointers to the objects. + std::vector children_pointers; + for (auto& row : mg_array->children) { + children_pointers.push_back(&row); } - for (auto i:descendant_objects) + for (auto i : children_pointers) { i->deleteObject(); - - for (auto i:descendant_reprs) - sp_repr_unparent(i); + } // Now we build new reprs Inkscape::XML::Node *mesh = mg->getRepr(); -- cgit v1.2.3