marker refactoring work

diff --git a/ChangeLog b/ChangeLog
index 280d0ceedabc2674fae36380499512fd28face81..40aff18fc7a47d6d330e72600e746a7658b669b0 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2006-07-18  Bryce Harrington <bryce@bryceharrington.org>
+       * 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  <Tim.Dwyer@infotech.monash.edu.au>
 
        * src/graphlayout/graphlayout.cpp,

diff --git a/src/document.cpp b/src/document.cpp
index 534ad412d1eeb566bc19788ddeecfda3cf6db495..44e13b4f20df8e1ffbe8bc5ffe9da8a54ba4dd1e 100644 (file)
--- 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<SPDocument *>(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<SPDocument *>(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 f7ce73fe8d2e629ab0cd1aaa611b80e7fa084b33..cf2194a952a64ba67a6ba55ad4f8f9bf218a741d 100644 (file)
--- 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 95823ee62f4442db0412ab8e3ff6d36f0af68a82..cf9f5a28f75ff89264448a38691f510e18b5d6c3 100644 (file)
--- 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 e91349ad901af89d4f230658df3b95ae172554fd..0f7031f56c9fe4f15ad0c0e9631d2e3bfbd3d452 100644 (file)
--- 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 b78291074697f8c30af899a26d164bf3b461703a..a677d08d7f124498383386e3b998675d97fe8b2c 100644 (file)
--- 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 5e011bd046dce8ccde3ecf4f00011dc8d5a9f7a2..8a96258e8477388ce3cff8c0e18de0c930b6f144 100644 (file)
--- 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 4879e5bea0fe3e7d71c4db7e2ff40f737dd7b79e..054fb6c7af14b6e35a6b932f0a6a2f5176908d0d 100644 (file)
--- 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 6884c352f170125389ce4523262ea3eea5dd6d8e..65ecc5442ffbe0a3824b444d33c8672a0fba8086 100644 (file)
--- 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 84fc35298269a95f24233cf57eea9d9e70480ef4..421d562ab0e772c88b5d280b05111202a97e8008 100644 (file)
--- 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();