From 3631c7532dab289cde84b6c9d2e05eee0addddad Mon Sep 17 00:00:00 2001 From: knutux Date: Thu, 6 Apr 2006 12:49:42 +0000 Subject: [PATCH] two crashes while editing nodes (one of those is reported as bug 1453558). * fixed by making SPKnotHolder a referenced class and adding ref/unref in knot_click_handler (which sometime change attributes and knot handler is destroyed before calling knotholder_update_knots); * fixed another crash by adding ref/unref to sp_knot_handler * also added signal disconnecting --- src/knot-holder-entity.h | 4 +++ src/knot.cpp | 13 +++++-- src/knot.h | 2 ++ src/knotholder.cpp | 75 +++++++++++++++++++++++++++++++--------- src/knotholder.h | 7 +++- 5 files changed, 82 insertions(+), 19 deletions(-) diff --git a/src/knot-holder-entity.h b/src/knot-holder-entity.h index 1e9001e67..bbc0c5634 100644 --- a/src/knot-holder-entity.h +++ b/src/knot-holder-entity.h @@ -29,6 +29,10 @@ struct SPKnotHolderEntity { /** Connection to \a knot's "moved" signal. */ guint handler_id; + /** Connection to \a knot's "clicked" signal. */ + guint _click_handler_id; + /** Connection to \a knot's "ungrabbed" signal. */ + guint _ungrab_handler_id; /** * Called solely from knot_moved_handler. diff --git a/src/knot.cpp b/src/knot.cpp index 01ccca693..95cadfdb7 100644 --- a/src/knot.cpp +++ b/src/knot.cpp @@ -195,6 +195,7 @@ static void sp_knot_init(SPKnot *knot) knot->shape = SP_KNOT_SHAPE_SQUARE; knot->mode = SP_KNOT_MODE_XOR; knot->tip = NULL; + knot->_event_handler_id = 0; knot->fill[SP_KNOT_STATE_NORMAL] = 0xffffff00; knot->fill[SP_KNOT_STATE_MOUSEOVER] = 0xff0000ff; @@ -228,6 +229,12 @@ static void sp_knot_dispose(GObject *object) gdk_pointer_ungrab (GDK_CURRENT_TIME); } + if (knot->_event_handler_id > 0) + { + g_signal_handler_disconnect(GTK_OBJECT (knot->item), knot->_event_handler_id); + knot->_event_handler_id = 0; + } + if (knot->item) { gtk_object_destroy (GTK_OBJECT (knot->item)); knot->item = NULL; @@ -280,6 +287,7 @@ static int sp_knot_handler(SPCanvasItem *item, GdkEvent *event, SPKnot *knot) g_assert(knot != NULL); g_assert(SP_IS_KNOT(knot)); + g_object_ref(G_OBJECT(knot)); tolerance = prefs_get_int_attribute_limited("options.dragtolerance", "value", 0, 0, 100); gboolean consumed = FALSE; @@ -424,6 +432,7 @@ static int sp_knot_handler(SPCanvasItem *item, GdkEvent *event, SPKnot *knot) break; } + g_object_unref(G_OBJECT(knot)); return consumed; } @@ -454,8 +463,8 @@ SPKnot *sp_knot_new(SPDesktop *desktop, const gchar *tip) "mode", SP_KNOT_MODE_XOR, NULL); - gtk_signal_connect(GTK_OBJECT(knot->item), "event", - GTK_SIGNAL_FUNC(sp_knot_handler), knot); + knot->_event_handler_id = gtk_signal_connect(GTK_OBJECT(knot->item), "event", + GTK_SIGNAL_FUNC(sp_knot_handler), knot); return knot; } diff --git a/src/knot.h b/src/knot.h index 795eeb8e3..5693417fd 100644 --- a/src/knot.h +++ b/src/knot.h @@ -62,6 +62,8 @@ struct SPKnot { gchar *tip; + gulong _event_handler_id; + //TODO: all the members above should eventualle become private, accessible via setters/getters inline void setSize (guint i) {size = i;} inline void setShape (guint i) {shape = (SPKnotShapeType) i;} diff --git a/src/knotholder.cpp b/src/knotholder.cpp index 0ea3ed98a..82d49d223 100644 --- a/src/knotholder.cpp +++ b/src/knotholder.cpp @@ -27,6 +27,9 @@ class SPDesktop; static void knot_clicked_handler (SPKnot *knot, guint state, gpointer data); static void knot_moved_handler(SPKnot *knot, NR::Point const *p, guint state, gpointer data); static void knot_ungrabbed_handler (SPKnot *knot, unsigned int state, SPKnotHolder *kh); +static void sp_knot_holder_class_init(SPKnotHolderClass *klass); + +void sp_knot_holder_dispose(GObject *object); #ifdef KNOT_HOLDER_DEBUG @@ -36,6 +39,41 @@ static void sp_knot_holder_debug(GtkObject *object, gpointer data) } #endif +static GObjectClass *parent_class; + +/** + * Registers SPKnotHolder class and returns its type number. + */ +GType sp_knot_holder_get_type() +{ + static GType type = 0; + if (!type) { + GTypeInfo info = { + sizeof(SPKnotHolderClass), + NULL, /* base_init */ + NULL, /* base_finalize */ + (GClassInitFunc) sp_knot_holder_class_init, + NULL, /* class_finalize */ + NULL, /* class_data */ + sizeof (SPKnotHolder), + 16, /* n_preallocs */ + NULL, + NULL + }; + type = g_type_register_static (G_TYPE_OBJECT, "SPKnotHolder", &info, (GTypeFlags) 0); + } + return type; +} + +/** + * SPKnotHolder vtable initialization. + */ +static void sp_knot_holder_class_init(SPKnotHolderClass *klass){ + GObjectClass *object_class = (GObjectClass *) klass; + parent_class = (GObjectClass*) g_type_class_peek_parent(klass); + object_class->dispose = sp_knot_holder_dispose; +} + SPKnotHolder *sp_knot_holder_new(SPDesktop *desktop, SPItem *item, SPKnotHolderReleasedFunc relhandler) { Inkscape::XML::Node *repr = SP_OBJECT(item)->repr; @@ -44,7 +82,7 @@ SPKnotHolder *sp_knot_holder_new(SPDesktop *desktop, SPItem *item, SPKnotHolderR g_return_val_if_fail(item != NULL, NULL); g_return_val_if_fail(SP_IS_ITEM(item), NULL); - SPKnotHolder *knot_holder = g_new(SPKnotHolder, 1); + SPKnotHolder *knot_holder = (SPKnotHolder*)g_object_new (SP_TYPE_KNOT_HOLDER, 0); knot_holder->desktop = desktop; knot_holder->item = item; g_object_ref(G_OBJECT(item)); @@ -62,22 +100,25 @@ SPKnotHolder *sp_knot_holder_new(SPDesktop *desktop, SPItem *item, SPKnotHolderR return knot_holder; } -void sp_knot_holder_destroy(SPKnotHolder *kh) -{ - if (kh) { - g_object_unref(G_OBJECT(kh->item)); - while (kh->entity) { - SPKnotHolderEntity *e = (SPKnotHolderEntity *) kh->entity->data; - /* unref should call destroy */ - g_object_unref(G_OBJECT(e->knot)); - g_free(e); - kh->entity = g_slist_remove(kh->entity, e); - } - - g_free(kh); +void sp_knot_holder_dispose(GObject *object) { + SPKnotHolder *kh = G_TYPE_CHECK_INSTANCE_CAST((object), SP_TYPE_KNOT_HOLDER, SPKnotHolder); + + g_object_unref(G_OBJECT(kh->item)); + while (kh->entity) { + SPKnotHolderEntity *e = (SPKnotHolderEntity *) kh->entity->data; + g_signal_handler_disconnect(GTK_OBJECT (e->knot), e->_click_handler_id); + g_signal_handler_disconnect(GTK_OBJECT (e->knot), e->_ungrab_handler_id); + /* unref should call destroy */ + g_object_unref(G_OBJECT(e->knot)); + g_free(e); + kh->entity = g_slist_remove(kh->entity, e); } } +void sp_knot_holder_destroy(SPKnotHolder *kh) { + g_object_unref(G_OBJECT(kh)); + } + void sp_knot_holder_add( SPKnotHolder *knot_holder, SPKnotHolderSetFunc knot_set, @@ -130,8 +171,8 @@ void sp_knot_holder_add_full( sp_knot_set_position(e->knot, &dp, SP_KNOT_STATE_NORMAL); e->handler_id = g_signal_connect(G_OBJECT(e->knot), "moved", G_CALLBACK(knot_moved_handler), knot_holder); - g_signal_connect(G_OBJECT(e->knot), "clicked", G_CALLBACK(knot_clicked_handler), knot_holder); - g_signal_connect(G_OBJECT(e->knot), "ungrabbed", G_CALLBACK(knot_ungrabbed_handler), knot_holder); + e->_click_handler_id = g_signal_connect(G_OBJECT(e->knot), "clicked", G_CALLBACK(knot_clicked_handler), knot_holder); + e->_ungrab_handler_id = g_signal_connect(G_OBJECT(e->knot), "ungrabbed", G_CALLBACK(knot_ungrabbed_handler), knot_holder); #ifdef KNOT_HOLDER_DEBUG g_signal_connect(ob, "destroy", sp_knot_holder_debug, "SPKnotHolder::knot"); @@ -163,6 +204,7 @@ static void knot_clicked_handler(SPKnot *knot, guint state, gpointer data) SPKnotHolder *knot_holder = (SPKnotHolder *) data; SPItem *item = SP_ITEM (knot_holder->item); + g_object_ref(G_OBJECT(knot_holder)); for (GSList *el = knot_holder->entity; el; el = el->next) { SPKnotHolderEntity *e = (SPKnotHolderEntity *) el->data; if (e->knot == knot) { @@ -178,6 +220,7 @@ static void knot_clicked_handler(SPKnot *knot, guint state, gpointer data) } knotholder_update_knots(knot_holder, item); + g_object_unref(G_OBJECT(knot_holder)); // for drag, this is done by ungrabbed_handler, but for click we must do it here sp_document_done(SP_OBJECT_DOCUMENT(knot_holder->item)); diff --git a/src/knotholder.h b/src/knotholder.h index 6980e2fdd..971dae3b0 100644 --- a/src/knotholder.h +++ b/src/knotholder.h @@ -32,7 +32,7 @@ typedef NR::Point (* SPKnotHolderGetFunc) (SPItem *item); /* fixme: Think how to make callbacks most sensitive (Lauris) */ typedef void (* SPKnotHolderReleasedFunc) (SPItem *item); -struct SPKnotHolder { +struct SPKnotHolder : GObject { SPDesktop *desktop; SPItem *item; GSList *entity; @@ -44,6 +44,8 @@ struct SPKnotHolder { gboolean local_change; ///< if true, no need to recreate knotholder if repr was changed. }; +struct SPKnotHolderClass : GObjectClass { +}; /* fixme: As a temporary solution, if released is NULL knotholder flushes undo itself (Lauris) */ SPKnotHolder *sp_knot_holder_new(SPDesktop *desktop, SPItem *item, SPKnotHolderReleasedFunc relhandler); @@ -64,6 +66,9 @@ void sp_knot_holder_add_full(SPKnotHolder *knot_holder, SPKnotModeType mode, gchar const *tip); +GType sp_knot_holder_get_type(); + +#define SP_TYPE_KNOT_HOLDER (sp_knot_holder_get_type()) #endif /* !__SP_KNOTHOLDER_H__ */ -- 2.30.2