update to trunk

(bzr r11950.1.158)

6 files changed, 72 insertions, 36 deletions

diff --git a/src/display/drawing-item.cpp b/src/display/drawing-item.cpp
index 1af07cb44..71a7f8906 100644
--- a/src/display/drawing-item.cpp
+++ b/src/display/drawing-item.cpp
@@ -141,7 +141,14 @@ DrawingItem::appendChild(DrawingItem *item)
     assert(item->_child_type == CHILD_ORPHAN);
     item->_child_type = CHILD_NORMAL;
     _children.push_back(*item);
-    _markForUpdate(STATE_ALL, false);
+
+    // This ensures that _markForUpdate() called on the child will recurse to this item
+    item->_state = STATE_ALL;
+    // Because _markForUpdate recurses through ancestors, we can simply call it
+    // on the just-added child. This has the additional benefit that we do not
+    // rely on the appended child being in the default non-updated state.
+    // We set propagate to true, because the child might have descendants of its own.
+    item->_markForUpdate(STATE_ALL, true);
 }
 
 void
@@ -151,7 +158,9 @@ DrawingItem::prependChild(DrawingItem *item)
     assert(item->_child_type == CHILD_ORPHAN);
     item->_child_type = CHILD_NORMAL;
     _children.push_front(*item);
-    _markForUpdate(STATE_ALL, false);
+    // See appendChild for explanation
+    item->_state = STATE_ALL;
+    item->_markForUpdate(STATE_ALL, true);
 }
 
 /// Delete all regular children of this item (not mask or clip).
@@ -281,17 +290,10 @@ DrawingItem::setZOrder(unsigned z)
     _markForRendering();
 }
 
-void DrawingItem::setItemBounds(Geom::OptRect const &bounds)
-{
-    if (!bounds) return;
-    Geom::IntRect copy = bounds->roundOutwards();
-    if (_filter) _filter->area_enlarge(copy, this);
-    this->setFilterBounds(copy);
-}
-
-void DrawingItem::setFilterBounds(Geom::OptRect const &bounds)
+void
+DrawingItem::setItemBounds(Geom::OptRect const &bounds)
 {
-    if (bounds) _filter_bbox = bounds;
+    _item_bbox = bounds;
 }
 
 /**
@@ -359,10 +361,14 @@ DrawingItem::update(Geom::IntRect const &area, UpdateContext const &ctx, unsigne
 
     if (to_update & STATE_BBOX) {
         // compute drawbox
-        if (_filter && render_filters && _filter_bbox) {
-            Geom::OptRect enlarged = _filter_bbox;
-            *enlarged *= ctm();
-            _drawbox = enlarged->roundOutwards();
+        if (_filter && render_filters) {
+            Geom::OptRect enlarged = _filter->filter_effect_area(_item_bbox);
+            if (enlarged) {
+                *enlarged *= ctm();
+                _drawbox = enlarged->roundOutwards();
+            } else {
+                _drawbox = Geom::OptIntRect();
+            }
         } else {
             _drawbox = _bbox;
         }
@@ -714,8 +720,11 @@ DrawingItem::pick(Geom::Point const &p, double delta, unsigned flags)
 {
     // Sometimes there's no BBOX in state, reason unknown (bug 992817)
     // I made this not an assert to remove the warning
-    if (!(_state & STATE_BBOX) || !(_state & STATE_PICK))
+    if (!(_state & STATE_BBOX) || !(_state & STATE_PICK)) {
+        g_warning("Invalid state when picking: STATE_BBOX = %d, STATE_PICK = %d",
+                  _state & STATE_BBOX, _state & STATE_PICK);
         return NULL;
+    }
     // ignore invisible and insensitive items unless sticky
     if (!(flags & PICK_STICKY) && !(_visible && _sensitive))
         return NULL;
@@ -736,7 +745,10 @@ DrawingItem::pick(Geom::Point const &p, double delta, unsigned flags)
     }
 
     Geom::OptIntRect box = (outline || (flags & PICK_AS_CLIP)) ? _bbox : _drawbox;
-    if (!box) return NULL;
+    if (!box) {
+        g_warning("bbox unset when picking");
+        return NULL;
+    }
 
     Geom::Rect expanded = *box;
     expanded.expandBy(delta);
@@ -756,6 +768,9 @@ DrawingItem::pick(Geom::Point const &p, double delta, unsigned flags)
 void
 DrawingItem::_markForRendering()
 {
+    // TODO: this function does too much work when a large subtree
+    // is invalidated - fix
+    
     bool outline = _drawing.outline();
     Geom::OptIntRect dirty = outline ? _bbox : _drawbox;
     if (!dirty) return;
@@ -807,8 +822,8 @@ DrawingItem::_invalidateFilterBackground(Geom::IntRect const &area)
  * all children should also have the corresponding flags unset before checking
  * whether they need to be traversed. This way there is one less traversal
  * of the tree. Without this we would need to unset state bits in all children.
- * With _propagate we do this during the update call, when we have to traverse
- * the tree anyway.
+ * With _propagate we do this during the update call, when we have to recurse
+ * into children anyway.
  */
 void
 DrawingItem::_markForUpdate(unsigned flags, bool propagate)
@@ -818,15 +833,31 @@ DrawingItem::_markForUpdate(unsigned flags, bool propagate)
     }
 
     if (_state & flags) {
+        unsigned oldstate = _state;
         _state &= ~flags;
-        if (_parent) {
+        if (oldstate != _state && _parent) {
+            // If we actually reset anything in state, recurse on the parent.
             _parent->_markForUpdate(flags, false);
         } else {
+            // If nothing changed, it means our ancestors are already invalidated
+            // up to the root. Do not bother recursing, because it won't change anything.
+            // Also do this if we are the root item, because we have no more ancestors
+            // to invalidate.
             _drawing.signal_request_update.emit(this);
         }
     }
 }
 
+/**
+ * Process information related to the new style.
+ *
+ * This function is something of a hack to avoid creating an extra class in the hierarchy
+ * which would differ from DrawingItem only by having a _style member.
+ * This is mainly to the benefit of DrawingGlyphs, which use the style of their parent.
+ * This should probably be refactored some day, possibly by creating the relevant class
+ * or creating a more complex data model in DrawingText and removing DrawingGlyphs,
+ * which would cause every item to have a style.
+ */
 void
 DrawingItem::_setStyleCommon(SPStyle *&_style, SPStyle *style)
 {
@@ -834,7 +865,6 @@ DrawingItem::_setStyleCommon(SPStyle *&_style, SPStyle *style)
     if (_style) sp_style_unref(_style);
     _style = style;
 
-    // if group has a filter
     if (style->filter.set && style->getFilter()) {
         if (!_filter) {
             int primitives = sp_filter_primitive_count(SP_FILTER(style->getFilter()));
diff --git a/src/display/drawing-item.h b/src/display/drawing-item.h
index c69b996b4..e03bbd0f7 100644
--- a/src/display/drawing-item.h
+++ b/src/display/drawing-item.h
@@ -89,7 +89,7 @@ public:
 
     Geom::OptIntRect geometricBounds() const { return _bbox; }
     Geom::OptIntRect visualBounds() const { return _drawbox; }
-    Geom::OptRect filterBounds() const { return _filter_bbox; }
+    Geom::OptRect itemBounds() const { return _item_bbox; }
     Geom::Affine ctm() const { return _ctm; }
     Geom::Affine transform() const { return _transform ? *_transform : Geom::identity(); }
     Drawing &drawing() const { return _drawing; }
@@ -176,7 +176,9 @@ protected:
     Geom::Affine _ctm; ///< Total transform from item coords to display coords
     Geom::OptIntRect _bbox; ///< Bounding box in display (pixel) coords including stroke
     Geom::OptIntRect _drawbox; ///< Full visual bounding box - enlarged by filters, shrunk by clips and masks
-    Geom::OptRect _filter_bbox; ///< Used by filters when settings bounds
+    Geom::OptRect _item_bbox; ///< Geometric bounding box in item's user space.
+                              ///  This is used to compute the filter effect region and render in
+                              ///  objectBoundingBox units.
 
     DrawingItem *_clip;
     DrawingItem *_mask;
diff --git a/src/display/drawing-shape.cpp b/src/display/drawing-shape.cpp
index e689d0755..e80f12486 100644
--- a/src/display/drawing-shape.cpp
+++ b/src/display/drawing-shape.cpp
@@ -179,8 +179,8 @@ DrawingShape::_renderItem(DrawingContext &ct, Geom::IntRect const &area, unsigne
         // update fill and stroke paints.
         // this cannot be done during nr_arena_shape_update, because we need a Cairo context
         // to render svg:pattern
-        has_fill   = _nrstyle.prepareFill(ct, _bbox);
-        has_stroke = _nrstyle.prepareStroke(ct, _bbox);
+        has_fill   = _nrstyle.prepareFill(ct, _item_bbox);
+        has_stroke = _nrstyle.prepareStroke(ct, _item_bbox);
         has_stroke &= (_nrstyle.stroke_width != 0);
 
         if (has_fill || has_stroke) {
diff --git a/src/display/drawing-text.cpp b/src/display/drawing-text.cpp
index fa9ce4ff8..55d54b770 100644
--- a/src/display/drawing-text.cpp
+++ b/src/display/drawing-text.cpp
@@ -398,8 +398,8 @@ unsigned DrawingText::_renderItem(DrawingContext &ct, Geom::IntRect const &/*are
     using Geom::X;
     using Geom::Y;
 
-    has_fill   = _nrstyle.prepareFill(  ct, _bbox);
-    has_stroke = _nrstyle.prepareStroke(ct, _bbox);
+    has_fill   = _nrstyle.prepareFill(  ct, _item_bbox);
+    has_stroke = _nrstyle.prepareStroke(ct, _item_bbox);
 
     if (has_fill || has_stroke) {
         Geom::Affine rotinv;
diff --git a/src/display/guideline.cpp b/src/display/guideline.cpp
index f71bc82ef..55c1ee495 100644
--- a/src/display/guideline.cpp
+++ b/src/display/guideline.cpp
@@ -94,14 +94,20 @@ static void sp_guideline_destroy(SPCanvasItem *object)
     g_return_if_fail (SP_IS_GUIDELINE (object));
     //g_return_if_fail (SP_GUIDELINE(object)->origin != NULL);
     //g_return_if_fail (SP_IS_CTRLPOINT(SP_GUIDELINE(object)->origin));
-    
-    if (SP_GUIDELINE(object)->origin != NULL && SP_IS_CTRLPOINT(SP_GUIDELINE(object)->origin)) {
-        sp_canvas_item_destroy(SP_GUIDELINE(object)->origin);
+
+    SPGuideLine *gl = SP_GUIDELINE(object);
+
+    if (gl->origin != NULL && SP_IS_CTRLPOINT(gl->origin)) {
+        sp_canvas_item_destroy(gl->origin);
     } else {
         // FIXME: This branch shouldn't be reached (although it seems to be harmless).
         //g_error("Why can it be that gl->origin is not a valid SPCtrlPoint?\n");
     }
 
+    if (gl->label) {
+        g_free(gl->label);
+    }
+
     SP_CANVAS_ITEM_CLASS(parent_class)->destroy(object);
 }
 
diff --git a/src/display/nr-filter.cpp b/src/display/nr-filter.cpp
index c0044c5d8..af9c15cd8 100644
--- a/src/display/nr-filter.cpp
+++ b/src/display/nr-filter.cpp
@@ -114,14 +114,12 @@ int Filter::render(Inkscape::DrawingItem const *item, DrawingContext &graphic, D
 
     Geom::Affine trans = item->ctm();
 
-    // Get filter are, the filter_effect_area is already done in visualBounds
-    Geom::OptRect filter_area = item->filterBounds();
-    // Use the geometricBounds as a backup solution
+    Geom::OptRect filter_area = filter_effect_area(item->itemBounds());
     if (!filter_area) return 1;
 
     FilterUnits units(_filter_units, _primitive_units);
     units.set_ctm(trans);
-    units.set_item_bbox(filter_area);
+    units.set_item_bbox(item->itemBounds());
     units.set_filter_area(*filter_area);
 
     std::pair<double,double> resolution
@@ -201,7 +199,7 @@ void Filter::area_enlarge(Geom::IntRect &bbox, Inkscape::DrawingItem const *item
     }
 
     Geom::Rect item_bbox;
-    Geom::OptRect maybe_bbox = item->geometricBounds();
+    Geom::OptRect maybe_bbox = item->itemBounds();
     if (maybe_bbox.isEmpty()) {
         // Code below needs a bounding box
         return;