From 799f473597d4eb61d843c37b2e294824c919db73 Mon Sep 17 00:00:00 2001 From: "Johan B. C. Engelen" Date: Mon, 30 Sep 2013 21:59:36 +0200 Subject: fix memleak (bzr r12633) --- src/display/guideline.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'src/display') 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); } -- cgit v1.2.3 From 5cf6f3cac4b8dce6a3059c2547cf376fd382e741 Mon Sep 17 00:00:00 2001 From: Krzysztof Kosi??ski Date: Tue, 1 Oct 2013 02:55:12 +0200 Subject: Minor improvements to DrawingItem code and documentation (bzr r12645) --- src/display/drawing-item.cpp | 45 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) (limited to 'src/display') diff --git a/src/display/drawing-item.cpp b/src/display/drawing-item.cpp index 1af07cb44..526fc3000 100644 --- a/src/display/drawing-item.cpp +++ b/src/display/drawing-item.cpp @@ -141,7 +141,11 @@ DrawingItem::appendChild(DrawingItem *item) assert(item->_child_type == CHILD_ORPHAN); item->_child_type = CHILD_NORMAL; _children.push_back(*item); - _markForUpdate(STATE_ALL, false); + // 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 +155,8 @@ 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->_markForUpdate(STATE_ALL, true); } /// Delete all regular children of this item (not mask or clip). @@ -714,8 +719,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 +744,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 +767,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 +821,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 +832,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 +864,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())); -- cgit v1.2.3 From 1595a83d610f4e328d84eb1fc2764041ea9335db Mon Sep 17 00:00:00 2001 From: Martin Owens Date: Tue, 1 Oct 2013 07:55:55 -0400 Subject: Revert render svg:patern segment for fill and stroke (bzr r12646) --- src/display/drawing-shape.cpp | 4 ++-- src/display/nr-filter.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src/display') diff --git a/src/display/drawing-shape.cpp b/src/display/drawing-shape.cpp index e689d0755..9ad18243e 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, _filter_bbox); + has_stroke = _nrstyle.prepareStroke(ct, _filter_bbox); has_stroke &= (_nrstyle.stroke_width != 0); if (has_fill || has_stroke) { diff --git a/src/display/nr-filter.cpp b/src/display/nr-filter.cpp index c0044c5d8..1289362aa 100644 --- a/src/display/nr-filter.cpp +++ b/src/display/nr-filter.cpp @@ -114,7 +114,7 @@ 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 + // Get filter area, the filter_effect_area is already done in visualBounds Geom::OptRect filter_area = item->filterBounds(); // Use the geometricBounds as a backup solution if (!filter_area) return 1; -- cgit v1.2.3 From 56791e9c202001bf29b089f1f52ad128e548f198 Mon Sep 17 00:00:00 2001 From: Krzysztof Kosi??ski Date: Tue, 1 Oct 2013 14:31:34 +0200 Subject: Fix possible bug in DrawingItem code (bzr r12647) --- src/display/drawing-item.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/display') diff --git a/src/display/drawing-item.cpp b/src/display/drawing-item.cpp index 526fc3000..b2b3e68a2 100644 --- a/src/display/drawing-item.cpp +++ b/src/display/drawing-item.cpp @@ -141,6 +141,9 @@ DrawingItem::appendChild(DrawingItem *item) assert(item->_child_type == CHILD_ORPHAN); item->_child_type = CHILD_NORMAL; _children.push_back(*item); + + // 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. @@ -156,6 +159,7 @@ DrawingItem::prependChild(DrawingItem *item) item->_child_type = CHILD_NORMAL; _children.push_front(*item); // See appendChild for explanation + item->_state = STATE_ALL; item->_markForUpdate(STATE_ALL, true); } -- cgit v1.2.3 From 87d93e27330577c2fd632dbaccbd3103884aa590 Mon Sep 17 00:00:00 2001 From: Krzysztof Kosi??ski Date: Tue, 1 Oct 2013 15:25:44 +0200 Subject: Comprehensive fix for the issues with disappearing filtered objects. Fixes #304407 and possibly a few other bugs. Revert incorrect _item_bbox changes from r12528. Fixed bugs: - https://launchpad.net/bugs/304407 (bzr r12648) --- src/display/drawing-item.cpp | 25 +++++++++++-------------- src/display/drawing-item.h | 6 ++++-- src/display/drawing-shape.cpp | 4 ++-- src/display/drawing-text.cpp | 4 ++-- src/display/nr-filter.cpp | 8 +++----- 5 files changed, 22 insertions(+), 25 deletions(-) (limited to 'src/display') diff --git a/src/display/drawing-item.cpp b/src/display/drawing-item.cpp index b2b3e68a2..71a7f8906 100644 --- a/src/display/drawing-item.cpp +++ b/src/display/drawing-item.cpp @@ -290,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; } /** @@ -368,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; } 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 9ad18243e..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, _filter_bbox); - has_stroke = _nrstyle.prepareStroke(ct, _filter_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/nr-filter.cpp b/src/display/nr-filter.cpp index 1289362aa..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 area, 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 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; -- cgit v1.2.3