From 5c85d657aed31c86ec1d89f03f56960bdb4d6df6 Mon Sep 17 00:00:00 2001 From: bryce Date: Thu, 20 Jul 2006 04:03:08 +0000 Subject: [PATCH] marker refactoring work --- ChangeLog | 7 +++ src/document.cpp | 100 ++++++++++++++++++++++-------------------- src/document.h | 2 + src/sp-defs.cpp | 13 ++---- src/sp-item-group.cpp | 31 +++++-------- src/sp-item-group.h | 2 - src/sp-object.cpp | 63 ++++++++++++++++++-------- src/sp-object.h | 4 ++ src/sp-switch.cpp | 10 ++--- src/sp-switch.h | 2 +- 10 files changed, 130 insertions(+), 104 deletions(-) diff --git a/ChangeLog b/ChangeLog index 280d0ceed..40aff18fc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2006-07-18 Bryce Harrington + * src/document.h, src/document.cpp: Refactoring from mental & + bryce to consolidate document update functionality from + sp_document_ensure_is_up_to_date() and + sp_document_idle_handler(). This is the first step in getting + updates of defs fixed. + 2006-07-18 Tim Dwyer * src/graphlayout/graphlayout.cpp, diff --git a/src/document.cpp b/src/document.cpp index 534ad412d..44e13b4f2 100644 --- a/src/document.cpp +++ b/src/document.cpp @@ -746,69 +746,75 @@ sp_document_setup_viewport (SPDocument *doc, SPItemCtx *ctx) ctx->i2vp = NR::identity(); } +/** + * Tries to update the document state based on the modified and + * "update required" flags, and return true if the document has + * been brought fully up to date. + */ +bool +SPDocument::_updateDocument() +{ + /* Process updates */ + if (this->root->uflags || this->root->mflags) { + if (this->root->uflags) { + SPItemCtx ctx; + sp_document_setup_viewport (this, &ctx); + + bool saved = sp_document_get_undo_sensitive(this); + sp_document_set_undo_sensitive(this, FALSE); + + this->root->updateDisplay((SPCtx *)&ctx, 0); + + sp_document_set_undo_sensitive(this, saved); + } + this->_emitModified(); + } + + return !(this->root->uflags || this->root->mflags); +} + + +/** + * Repeatedly works on getting the document updated, since sometimes + * it takes more than one pass to get the document updated. But it + * usually should not take more than a few loops, and certainly never + * more than 32 iterations. So we bail out if we hit 32 iterations, + * since this typically indicates we're stuck in an update loop. + */ gint sp_document_ensure_up_to_date(SPDocument *doc) { - int lc; - lc = 32; - while (doc->root->uflags || doc->root->mflags) { - lc -= 1; - if (lc < 0) { - g_warning("More than 32 iterations while updating document '%s'", doc->uri); - if (doc->modified_id) { - /* Remove handler */ - gtk_idle_remove(doc->modified_id); - doc->modified_id = 0; - } - return FALSE; - } - /* Process updates */ - if (doc->root->uflags) { - SPItemCtx ctx; - sp_document_setup_viewport (doc, &ctx); - doc->root->updateDisplay((SPCtx *)&ctx, 0); + int counter = 32; + while (!doc->_updateDocument()) { + if (counter == 0) { + g_warning("More than 32 iteration while updating document '%s'", doc->uri); + break; } - doc->_emitModified(); + counter--; } + if (doc->modified_id) { /* Remove handler */ gtk_idle_remove(doc->modified_id); doc->modified_id = 0; } - return TRUE; + return counter>0; } +/** + * An idle handler to update the document. Returns true if + * the document needs further updates. + */ static gint sp_document_idle_handler(gpointer data) { - SPDocument *doc; - int repeat; - - doc = static_cast(data); - -#ifdef SP_DOCUMENT_DEBUG_IDLE - g_print("->\n"); -#endif - - /* Process updates */ - if (doc->root->uflags) { - SPItemCtx ctx; - sp_document_setup_viewport (doc, &ctx); - - gboolean saved = sp_document_get_undo_sensitive(doc); - sp_document_set_undo_sensitive(doc, FALSE); - - doc->root->updateDisplay((SPCtx *)&ctx, 0); - - sp_document_set_undo_sensitive(doc, saved); - /* if (doc->root->uflags & SP_OBJECT_MODIFIED_FLAG) return TRUE; */ + SPDocument *doc = static_cast(data); + if (doc->_updateDocument()) { + doc->modified_id = 0; + return false; + } else { + return true; } - - doc->_emitModified(); - - repeat = (doc->root->uflags || doc->root->mflags); - if (!repeat) doc->modified_id = 0; - return repeat; } static bool is_within(NR::Rect const &area, NR::Rect const &box) diff --git a/src/document.h b/src/document.h index f7ce73fe8..cf2194a95 100644 --- a/src/document.h +++ b/src/document.h @@ -115,6 +115,8 @@ struct SPDocument : public Inkscape::GC::Managed<>, Inkscape::EventLog& getEventLog() const; + bool _updateDocument(); + private: SPDocument(SPDocument const &); // no copy void operator=(SPDocument const &); // no assign diff --git a/src/sp-defs.cpp b/src/sp-defs.cpp index 95823ee62..cf9f5a28f 100644 --- a/src/sp-defs.cpp +++ b/src/sp-defs.cpp @@ -83,21 +83,14 @@ static void sp_defs_update(SPObject *object, SPCtx *ctx, guint flags) flags &= SP_OBJECT_MODIFIED_CASCADE; - GSList *l = NULL; - for ( SPObject *child = sp_object_first_child(object) ; child != NULL; child = SP_OBJECT_NEXT(child) ) { - g_object_ref(G_OBJECT(child)); - l = g_slist_prepend(l, child); - } - - l = g_slist_reverse(l); - + GSList *l = g_slist_reverse(object->childList(true)); while (l) { SPObject *child = SP_OBJECT(l->data); l = g_slist_remove(l, child); - if (flags || (child->uflags & SP_OBJECT_MODIFIED_FLAG)) { + if (flags || (child->uflags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_CHILD_MODIFIED_FLAG))) { child->updateDisplay(ctx, flags); } - g_object_unref(G_OBJECT(child)); + g_object_unref (G_OBJECT (child)); } } diff --git a/src/sp-item-group.cpp b/src/sp-item-group.cpp index e91349ad9..0f7031f56 100644 --- a/src/sp-item-group.cpp +++ b/src/sp-item-group.cpp @@ -592,13 +592,15 @@ void CGroup::onChildRemoved(Inkscape::XML::Node */*child*/) { } void CGroup::onUpdate(SPCtx *ctx, unsigned int flags) { - SPObject *child; SPItemCtx *ictx, cctx; ictx = (SPItemCtx *) ctx; cctx = *ictx; - if (flags & SP_OBJECT_MODIFIED_FLAG) flags |= SP_OBJECT_PARENT_MODIFIED_FLAG; + if (flags & SP_OBJECT_MODIFIED_FLAG) { + flags |= SP_OBJECT_PARENT_MODIFIED_FLAG; + } + flags &= SP_OBJECT_MODIFIED_CASCADE; if (flags & SP_OBJECT_STYLE_MODIFIED_FLAG) { @@ -608,9 +610,9 @@ void CGroup::onUpdate(SPCtx *ctx, unsigned int flags) { } } - GSList *l = g_slist_reverse(_childList(true, ActionUpdate)); + GSList *l = g_slist_reverse(_group->childList(true, SPObject::ActionUpdate)); while (l) { - child = SP_OBJECT (l->data); + SPObject *child = SP_OBJECT (l->data); l = g_slist_remove (l, child); if (flags || (child->uflags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_CHILD_MODIFIED_FLAG))) { if (SP_IS_ITEM (child)) { @@ -626,17 +628,6 @@ void CGroup::onUpdate(SPCtx *ctx, unsigned int flags) { } } -GSList *CGroup::_childList(bool add_ref, Action) { - GSList *l = NULL; - for (SPObject *child = sp_object_first_child(_group) ; child != NULL ; child = SP_OBJECT_NEXT(child) ) { - if (add_ref) - g_object_ref (G_OBJECT (child)); - - l = g_slist_prepend (l, child); - } - return l; -} - void CGroup::onModified(guint flags) { SPObject *child; @@ -650,7 +641,7 @@ void CGroup::onModified(guint flags) { } } - GSList *l = g_slist_reverse(_childList(true)); + GSList *l = g_slist_reverse(_group->childList(true)); while (l) { child = SP_OBJECT (l->data); l = g_slist_remove (l, child); @@ -662,7 +653,7 @@ void CGroup::onModified(guint flags) { } void CGroup::calculateBBox(NRRect *bbox, NR::Matrix const &transform, unsigned const flags) { - GSList *l = _childList(false, ActionBBox); + GSList *l = _group->childList(false, SPObject::ActionBBox); while (l) { SPObject *o = SP_OBJECT (l->data); if (SP_IS_ITEM(o)) { @@ -675,7 +666,7 @@ void CGroup::calculateBBox(NRRect *bbox, NR::Matrix const &transform, unsigned c } void CGroup::onPrint(SPPrintContext *ctx) { - GSList *l = g_slist_reverse(_childList(false)); + GSList *l = g_slist_reverse(_group->childList(false)); while (l) { SPObject *o = SP_OBJECT (l->data); if (SP_IS_ITEM(o)) { @@ -723,7 +714,7 @@ void CGroup::_showChildren (NRArena *arena, NRArenaItem *ai, unsigned int key, u NRArenaItem *ac = NULL; NRArenaItem *ar = NULL; SPItem * child = NULL; - GSList *l = g_slist_reverse(_childList(false, ActionShow)); + GSList *l = g_slist_reverse(_group->childList(false, SPObject::ActionShow)); while (l) { SPObject *o = SP_OBJECT (l->data); if (SP_IS_ITEM (o)) { @@ -742,7 +733,7 @@ void CGroup::_showChildren (NRArena *arena, NRArenaItem *ai, unsigned int key, u void CGroup::hide (unsigned int key) { SPItem * child; - GSList *l = g_slist_reverse(_childList(false, ActionShow)); + GSList *l = g_slist_reverse(_group->childList(false, SPObject::ActionShow)); while (l) { SPObject *o = SP_OBJECT (l->data); if (SP_IS_ITEM (o)) { diff --git a/src/sp-item-group.h b/src/sp-item-group.h index b78291074..a677d08d7 100644 --- a/src/sp-item-group.h +++ b/src/sp-item-group.h @@ -79,8 +79,6 @@ public: gint getItemCount(); protected: - enum Action { ActionGeneral, ActionBBox, ActionUpdate, ActionShow }; - virtual GSList *_childList(bool add_ref, Action action = ActionGeneral); virtual void _showChildren (NRArena *arena, NRArenaItem *ai, unsigned int key, unsigned int flags); SPGroup *_group; diff --git a/src/sp-object.cpp b/src/sp-object.cpp index 5e011bd04..8a96258e8 100644 --- a/src/sp-object.cpp +++ b/src/sp-object.cpp @@ -458,6 +458,22 @@ SPObject::appendChildRepr(Inkscape::XML::Node *repr) { } } +/** + * Retrieves the children as a GSList object, optionally ref'ing the children + * in the process, if add_ref is specified. + */ +GSList *SPObject::childList(bool add_ref, Action) { + GSList *l = NULL; + for (SPObject *child = sp_object_first_child(this) ; child != NULL; child = SP_OBJECT_NEXT(child) ) { + if (add_ref) + g_object_ref (G_OBJECT (child)); + + l = g_slist_prepend (l, child); + } + return l; + +} + /** Gets the label property for the object or a default if no label * is defined. */ @@ -1168,26 +1184,31 @@ SPObject::updateRepr(Inkscape::XML::Node *repr, unsigned int flags) { void SPObject::requestDisplayUpdate(unsigned int flags) { + g_return_if_fail( this->document != NULL ); + if (update_in_progress) { g_print("WARNING: Requested update while update in progress, counter = %d\n", update_in_progress); } + /* requestModified must be used only to set one of SP_OBJECT_MODIFIED_FLAG or + * SP_OBJECT_CHILD_MODIFIED_FLAG */ g_return_if_fail(!(flags & SP_OBJECT_PARENT_MODIFIED_FLAG)); g_return_if_fail((flags & SP_OBJECT_MODIFIED_FLAG) || (flags & SP_OBJECT_CHILD_MODIFIED_FLAG)); g_return_if_fail(!((flags & SP_OBJECT_MODIFIED_FLAG) && (flags & SP_OBJECT_CHILD_MODIFIED_FLAG))); - /* Check for propagate before we set any flags */ - /* Propagate means, that this is not passed through by modification request cascade yet */ - unsigned int propagate = (!(this->uflags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_CHILD_MODIFIED_FLAG))); + bool already_propagated = (!(this->uflags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_CHILD_MODIFIED_FLAG))); - /* Just set this flags safe even if some have been set before */ this->uflags |= flags; - if (propagate) { - if (this->parent) { - this->parent->requestDisplayUpdate(SP_OBJECT_CHILD_MODIFIED_FLAG); + /* If requestModified has already been called on this object or one of its children, then we + * don't need to set CHILD_MODIFIED on our ancestors because it's already been done. + */ + if (already_propagated) { + SPObject *parent = SP_OBJECT_PARENT(this); + if (parent) { + parent->requestDisplayUpdate(SP_OBJECT_CHILD_MODIFIED_FLAG); } else { - sp_document_request_modified(this->document); + sp_document_request_modified(SP_OBJECT_DOCUMENT(this)); } } } @@ -1228,29 +1249,30 @@ SPObject::updateDisplay(SPCtx *ctx, unsigned int flags) update_in_progress --; } +/** + * Request modified always bubbles *up* the tree, as opposed to + * request display update, which trickles down and relies on the + * flags set during this pass... + */ void SPObject::requestModified(unsigned int flags) { g_return_if_fail( this->document != NULL ); - /* PARENT_MODIFIED is computed later on and is not intended to be - * "manually" queued */ + /* requestModified must be used only to set one of SP_OBJECT_MODIFIED_FLAG or + * SP_OBJECT_CHILD_MODIFIED_FLAG */ g_return_if_fail(!(flags & SP_OBJECT_PARENT_MODIFIED_FLAG)); - - /* we should be setting either MODIFIED or CHILD_MODIFIED... */ g_return_if_fail((flags & SP_OBJECT_MODIFIED_FLAG) || (flags & SP_OBJECT_CHILD_MODIFIED_FLAG)); - - /* ...but not both */ g_return_if_fail(!((flags & SP_OBJECT_MODIFIED_FLAG) && (flags & SP_OBJECT_CHILD_MODIFIED_FLAG))); - unsigned int old_mflags=this->mflags; + bool already_propagated = (!(this->mflags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_CHILD_MODIFIED_FLAG))); + this->mflags |= flags; - /* If we already had MODIFIED or CHILD_MODIFIED queued, we will - * have already queued CHILD_MODIFIED with our ancestors and - * need not disturb them again. + /* If requestModified has already been called on this object or one of its children, then we + * don't need to set CHILD_MODIFIED on our ancestors because it's already been done. */ - if (!( old_mflags & ( SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_CHILD_MODIFIED_FLAG ) )) { + if (already_propagated) { SPObject *parent=SP_OBJECT_PARENT(this); if (parent) { parent->requestModified(SP_OBJECT_CHILD_MODIFIED_FLAG); @@ -1260,6 +1282,9 @@ SPObject::requestModified(unsigned int flags) } } +/** + * This is what actually delivers the modified signals + */ void SPObject::emitModified(unsigned int flags) { diff --git a/src/sp-object.h b/src/sp-object.h index 4879e5bea..054fb6c7a 100644 --- a/src/sp-object.h +++ b/src/sp-object.h @@ -206,6 +206,10 @@ struct SPObject : public GObject { SPObject *lastChild() { return _last_child; } SPObject const *lastChild() const { return _last_child; } + enum Action { ActionGeneral, ActionBBox, ActionUpdate, ActionShow }; + /** @brief Retrieves children as a GSList */ + GSList *childList(bool add_ref, Action action = ActionGeneral); + SPObject *appendChildRepr(Inkscape::XML::Node *repr); /** @brief Gets the author-visible label for this object. */ diff --git a/src/sp-switch.cpp b/src/sp-switch.cpp index 6884c352f..65ecc5442 100644 --- a/src/sp-switch.cpp +++ b/src/sp-switch.cpp @@ -75,9 +75,9 @@ SPObject *CSwitch::_evaluateFirst() { return NULL; } -GSList *CSwitch::_childList(bool add_ref, Action action) { - if ( ActionGeneral != action ) { - return CGroup::_childList(add_ref, action); +GSList *CSwitch::_childList(bool add_ref, SPObject::Action action) { + if ( action != SPObject::ActionGeneral ) { + return _group->childList(add_ref, action); } SPObject *child = _evaluateFirst(); @@ -120,7 +120,7 @@ void CSwitch::_reevaluate(bool add_to_arena) { _releaseLastItem(_cached_item); SPItem * child; - for ( GSList *l = _childList(false, ActionShow); + for ( GSList *l = _childList(false, SPObject::ActionShow); NULL != l ; l = g_slist_remove (l, l->data)) { SPObject *o = SP_OBJECT (l->data); @@ -162,7 +162,7 @@ void CSwitch::_showChildren (NRArena *arena, NRArenaItem *ai, unsigned int key, NRArenaItem *ac = NULL; NRArenaItem *ar = NULL; SPItem * child; - GSList *l = _childList(false, ActionShow); + GSList *l = _childList(false, SPObject::ActionShow); while (l) { SPObject *o = SP_OBJECT (l->data); if (SP_IS_ITEM (o)) { diff --git a/src/sp-switch.h b/src/sp-switch.h index 84fc35298..421d562ab 100644 --- a/src/sp-switch.h +++ b/src/sp-switch.h @@ -38,7 +38,7 @@ public: virtual gchar *getDescription(); protected: - virtual GSList *_childList(bool add_ref, Action action); + virtual GSList *_childList(bool add_ref, SPObject::Action action); virtual void _showChildren (NRArena *arena, NRArenaItem *ai, unsigned int key, unsigned int flags); SPObject *_evaluateFirst(); -- 2.30.2