From 2029349f5291ae10048202732a89f34c0a179130 Mon Sep 17 00:00:00 2001 From: "Liam P. White" Date: Wed, 6 Aug 2014 20:40:19 -0400 Subject: Add in some debugging code that will complain when deleted knots are accessed by snap handler (bzr r13341.1.120) --- src/CMakeLists.txt | 2 ++ src/Makefile_insert | 1 + src/knot-ptr.cpp | 25 +++++++++++++++++++++++++ src/knot-ptr.h | 8 ++++++++ src/knot.cpp | 3 +++ src/ui/tools/tool-base.cpp | 2 ++ 6 files changed, 41 insertions(+) create mode 100644 src/knot-ptr.cpp create mode 100644 src/knot-ptr.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4a792af6b..94b4b8c85 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -218,6 +218,7 @@ set(inkscape_SRC knot-holder-entity.cpp knot.cpp knotholder.cpp + knot-ptr.cpp layer-fns.cpp layer-manager.cpp layer-model.cpp @@ -356,6 +357,7 @@ set(inkscape_SRC knot-holder-entity.h knot.h knotholder.h + knot-ptr.h layer-fns.h layer-manager.h layer-model.h diff --git a/src/Makefile_insert b/src/Makefile_insert index f191839a1..436ab26f9 100644 --- a/src/Makefile_insert +++ b/src/Makefile_insert @@ -80,6 +80,7 @@ ink_common_sources += \ knot-enums.h \ knotholder.cpp knotholder.h \ knot-holder-entity.h knot-holder-entity.cpp \ + knot-ptr.h knot-ptr.cpp \ layer-fns.cpp layer-fns.h \ layer-manager.cpp layer-manager.h \ layer-model.cpp layer-model.h \ diff --git a/src/knot-ptr.cpp b/src/knot-ptr.cpp new file mode 100644 index 000000000..de8118ba7 --- /dev/null +++ b/src/knot-ptr.cpp @@ -0,0 +1,25 @@ +#include +#include +#include +#include "knot-ptr.h" + +static std::list deleted_knots; + +void knot_deleted_callback(void* knot) { + if (std::find(deleted_knots.begin(), deleted_knots.end(), knot) == deleted_knots.end()) { + deleted_knots.push_back(knot); + } +} + +void knot_created_callback(void* knot) { + std::list::iterator it = std::find(deleted_knots.begin(), deleted_knots.end(), knot); + if (it != deleted_knots.end()) { + deleted_knots.erase(it); + } +} + +void check_if_knot_deleted(void* knot) { + if (std::find(deleted_knots.begin(), deleted_knots.end(), knot) != deleted_knots.end()) { + g_warning("Accessed knot after it was freed at %p", knot); + } +} diff --git a/src/knot-ptr.h b/src/knot-ptr.h new file mode 100644 index 000000000..5895dfd2e --- /dev/null +++ b/src/knot-ptr.h @@ -0,0 +1,8 @@ +#ifndef KNOT_PTR_DETECTOR +#define KNOT_PTR_DETECTOR + +void knot_deleted_callback(void* knot); +void knot_created_callback(void* knot); +void check_if_knot_deleted(void* knot); + +#endif diff --git a/src/knot.cpp b/src/knot.cpp index 2f8f55a2e..6205af26a 100644 --- a/src/knot.cpp +++ b/src/knot.cpp @@ -21,6 +21,7 @@ #include "desktop.h" #include "desktop-handles.h" #include "knot.h" +#include "knot-ptr.h" #include "document.h" #include "document-undo.h" #include "preferences.h" @@ -118,6 +119,7 @@ SPKnot::SPKnot(SPDesktop *desktop, gchar const *tip) this->_event_handler_id = g_signal_connect(G_OBJECT(this->item), "event", G_CALLBACK(sp_knot_handler), this); + knot_created_callback(this); } SPKnot::~SPKnot() { @@ -165,6 +167,7 @@ SPKnot::~SPKnot() { // FIXME: cannot snap to destroyed knot (lp:1309050) //sp_event_context_discard_delayed_snap_event(this->desktop->event_context); + knot_deleted_callback(this); } void SPKnot::startDragging(Geom::Point const &p, gint x, gint y, guint32 etime) { diff --git a/src/ui/tools/tool-base.cpp b/src/ui/tools/tool-base.cpp index f1d90f6c6..c23da87c0 100644 --- a/src/ui/tools/tool-base.cpp +++ b/src/ui/tools/tool-base.cpp @@ -58,6 +58,7 @@ #include "sp-guide.h" #include "color.h" #include "knot.h" +#include "knot-ptr.h" // globals for temporary switching to selector by space static bool selector_toggled = FALSE; @@ -1359,6 +1360,7 @@ gboolean sp_event_context_snap_watchdog_callback(gpointer data) { break; case DelayedSnapEvent::KNOT_HANDLER: { gpointer knot = dse->getItem2(); + check_if_knot_deleted(knot); if (knot && SP_IS_KNOT(knot)) { sp_knot_handler_request_position(dse->getEvent(), SP_KNOT(knot)); } -- cgit v1.2.3 From 0561be82a9285311360df88d61e31505073a2de3 Mon Sep 17 00:00:00 2001 From: "Liam P. White" Date: Thu, 7 Aug 2014 16:31:25 -0400 Subject: Rendering performance. Optimize bezier cases, implement arc rendering via Cairo. (bzr r13341.1.121) --- src/display/cairo-utils.cpp | 90 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 70 insertions(+), 20 deletions(-) diff --git a/src/display/cairo-utils.cpp b/src/display/cairo-utils.cpp index 5b358ade7..15ce35146 100644 --- a/src/display/cairo-utils.cpp +++ b/src/display/cairo-utils.cpp @@ -19,6 +19,7 @@ #include #include <2geom/pathvector.h> #include <2geom/bezier-curve.h> +#include <2geom/elliptical-arc.h> #include <2geom/hvlinesegment.h> #include <2geom/affine.h> #include <2geom/point.h> @@ -502,7 +503,17 @@ void Pixbuf::ensurePixelFormat(PixelFormat fmt) static void feed_curve_to_cairo(cairo_t *cr, Geom::Curve const &c, Geom::Affine const & trans, Geom::Rect view, bool optimize_stroke) { - if( is_straight_curve(c) ) + using Geom::X; + using Geom::Y; + + unsigned order = 0; + if (Geom::BezierCurve const* b = dynamic_cast(&c)) { + order = b->order(); + } + + // handle the three typical curve cases + switch (order) { + case 1: { Geom::Point end_tr = c.finalPoint() * trans; if (!optimize_stroke) { @@ -516,57 +527,96 @@ feed_curve_to_cairo(cairo_t *cr, Geom::Curve const &c, Geom::Affine const & tran } } } - else if(Geom::QuadraticBezier const *quadratic_bezier = dynamic_cast(&c)) { + break; + case 2: + { + Geom::QuadraticBezier const *quadratic_bezier = static_cast(&c); std::vector points = quadratic_bezier->points(); points[0] *= trans; points[1] *= trans; points[2] *= trans; + // degree-elevate to cubic Bezier, since Cairo doesn't do quadratic Beziers Geom::Point b1 = points[0] + (2./3) * (points[1] - points[0]); Geom::Point b2 = b1 + (1./3) * (points[2] - points[0]); if (!optimize_stroke) { - cairo_curve_to(cr, b1[0], b1[1], b2[0], b2[1], points[2][0], points[2][1]); + cairo_curve_to(cr, b1[0], b1[1], b2[0], b2[1], points[2][X], points[2][Y]); } else { Geom::Rect swept(points[0], points[2]); swept.expandTo(points[1]); if (swept.intersects(view)) { - cairo_curve_to(cr, b1[0], b1[1], b2[0], b2[1], points[2][0], points[2][1]); + cairo_curve_to(cr, b1[0], b1[1], b2[0], b2[1], points[2][X], points[2][Y]); } else { - cairo_move_to(cr, points[2][0], points[2][1]); + cairo_move_to(cr, points[2][X], points[2][Y]); } } } - else if(Geom::CubicBezier const *cubic_bezier = dynamic_cast(&c)) { + break; + case 3: + { + Geom::CubicBezier const *cubic_bezier = static_cast(&c); std::vector points = cubic_bezier->points(); //points[0] *= trans; // don't do this one here for fun: it is only needed for optimized strokes points[1] *= trans; points[2] *= trans; points[3] *= trans; if (!optimize_stroke) { - cairo_curve_to(cr, points[1][0], points[1][1], points[2][0], points[2][1], points[3][0], points[3][1]); + cairo_curve_to(cr, points[1][X], points[1][Y], points[2][X], points[2][Y], points[3][X], points[3][Y]); } else { points[0] *= trans; // didn't transform this point yet Geom::Rect swept(points[0], points[3]); swept.expandTo(points[1]); swept.expandTo(points[2]); if (swept.intersects(view)) { - cairo_curve_to(cr, points[1][0], points[1][1], points[2][0], points[2][1], points[3][0], points[3][1]); + cairo_curve_to(cr, points[1][X], points[1][Y], points[2][X], points[2][Y], points[3][X], points[3][Y]); } else { - cairo_move_to(cr, points[3][0], points[3][1]); + cairo_move_to(cr, points[3][X], points[3][Y]); } } } -// else if(Geom::SVGEllipticalArc const *svg_elliptical_arc = dynamic_cast(c)) { -// //TODO: get at the innards and spit them out to cairo -// } - else { - //this case handles sbasis as well as all other curve types - Geom::Path sbasis_path = Geom::cubicbezierpath_from_sbasis(c.toSBasis(), 0.1); - - //recurse to convert the new path resulting from the sbasis to svgd - for(Geom::Path::iterator iter = sbasis_path.begin(); iter != sbasis_path.end(); ++iter) { - feed_curve_to_cairo(cr, *iter, trans, view, optimize_stroke); + break; + default: + { + if (Geom::EllipticalArc const *a = dynamic_cast(&c)) { + if (!optimize_stroke || a->boundsFast().intersects(view)) { + Geom::Affine xform = a->unitCircleTransform() * trans; + bool sweep = a->sweep(); + Geom::Point ang(a->initialAngle().radians(), a->finalAngle().radians()); + // Apply the transformation to the current context + cairo_matrix_t cm; + cm.xx = xform[0]; + cm.xy = xform[2]; + cm.x0 = xform[4]; + cm.yx = xform[1]; + cm.yy = xform[3]; + cm.y0 = xform[5]; + + cairo_save(cr); + cairo_transform(cr, &cm); + + // Draw the circle + if (sweep) { + cairo_arc(cr, 0, 0, 1, ang[0], ang[1]); + } else { + cairo_arc_negative(cr, 0, 0, 1, ang[0], ang[1]); + } + + cairo_restore(cr); + } else { + cairo_move_to(cr, a->finalPoint()[X], a->finalPoint()[Y]); + } + } else { + // handles sbasis as well as all other curve types + // this is very slow + Geom::Path sbasis_path = Geom::cubicbezierpath_from_sbasis(c.toSBasis(), 0.1); + + // recurse to convert the new path resulting from the sbasis to svgd + for (Geom::Path::iterator iter = sbasis_path.begin(); iter != sbasis_path.end(); ++iter) { + feed_curve_to_cairo(cr, *iter, trans, view, optimize_stroke); + } } } + break; + } } @@ -579,7 +629,7 @@ feed_path_to_cairo (cairo_t *ct, Geom::Path const &path) cairo_move_to(ct, path.initialPoint()[0], path.initialPoint()[1] ); - for(Geom::Path::const_iterator cit = path.begin(); cit != path.end_open(); ++cit) { + for (Geom::Path::const_iterator cit = path.begin(); cit != path.end_open(); ++cit) { feed_curve_to_cairo(ct, *cit, Geom::identity(), Geom::Rect(), false); // optimize_stroke is false, so the view rect is not used } -- cgit v1.2.3 From a8e25e6d4a86ffcfc6facb8efbba093f995fe8fc Mon Sep 17 00:00:00 2001 From: "Liam P. White" Date: Thu, 7 Aug 2014 20:34:34 -0400 Subject: Fix small regression (bzr r13341.1.122) --- src/display/cairo-utils.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/display/cairo-utils.cpp b/src/display/cairo-utils.cpp index 15ce35146..d6ff7b2f0 100644 --- a/src/display/cairo-utils.cpp +++ b/src/display/cairo-utils.cpp @@ -539,12 +539,12 @@ feed_curve_to_cairo(cairo_t *cr, Geom::Curve const &c, Geom::Affine const & tran Geom::Point b1 = points[0] + (2./3) * (points[1] - points[0]); Geom::Point b2 = b1 + (1./3) * (points[2] - points[0]); if (!optimize_stroke) { - cairo_curve_to(cr, b1[0], b1[1], b2[0], b2[1], points[2][X], points[2][Y]); + cairo_curve_to(cr, b1[X], b1[Y], b2[X], b2[Y], points[2][X], points[2][Y]); } else { Geom::Rect swept(points[0], points[2]); swept.expandTo(points[1]); if (swept.intersects(view)) { - cairo_curve_to(cr, b1[0], b1[1], b2[0], b2[1], points[2][X], points[2][Y]); + cairo_curve_to(cr, b1[X], b1[Y], b2[X], b2[Y], points[2][X], points[2][Y]); } else { cairo_move_to(cr, points[2][X], points[2][Y]); } @@ -577,10 +577,10 @@ feed_curve_to_cairo(cairo_t *cr, Geom::Curve const &c, Geom::Affine const & tran default: { if (Geom::EllipticalArc const *a = dynamic_cast(&c)) { - if (!optimize_stroke || a->boundsFast().intersects(view)) { + //if (!optimize_stroke || a->boundsFast().intersects(view)) { Geom::Affine xform = a->unitCircleTransform() * trans; - bool sweep = a->sweep(); Geom::Point ang(a->initialAngle().radians(), a->finalAngle().radians()); + // Apply the transformation to the current context cairo_matrix_t cm; cm.xx = xform[0]; @@ -594,16 +594,17 @@ feed_curve_to_cairo(cairo_t *cr, Geom::Curve const &c, Geom::Affine const & tran cairo_transform(cr, &cm); // Draw the circle - if (sweep) { + if (a->sweep()) { cairo_arc(cr, 0, 0, 1, ang[0], ang[1]); } else { cairo_arc_negative(cr, 0, 0, 1, ang[0], ang[1]); } - + // Revert the current context cairo_restore(cr); - } else { - cairo_move_to(cr, a->finalPoint()[X], a->finalPoint()[Y]); - } + //} else { + // Geom::Point f = a->finalPoint() * trans; + // cairo_move_to(cr, f[X], f[Y]); + //} } else { // handles sbasis as well as all other curve types // this is very slow -- cgit v1.2.3 From 2c681a38a34a12162c713d1a923256433bf57e82 Mon Sep 17 00:00:00 2001 From: "Liam P. White" Date: Thu, 7 Aug 2014 21:35:14 -0400 Subject: Small tweak to bbox calculation (bzr r13341.1.123) --- src/helper/geom.cpp | 53 +++++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/helper/geom.cpp b/src/helper/geom.cpp index c9148a634..6eba6e949 100644 --- a/src/helper/geom.cpp +++ b/src/helper/geom.cpp @@ -168,24 +168,25 @@ bounds_exact_transformed(Geom::PathVector const & pv, Geom::Affine const & t) for (Geom::Path::const_iterator cit = it->begin(); cit != it->end_open(); ++cit) { Geom::Curve const &c = *cit; - if( is_straight_curve(c) ) - { - bbox.expandTo( c.finalPoint() * t ); - } - else if(Geom::CubicBezier const *cubic_bezier = dynamic_cast(&c)) - { - Geom::Point c0 = (*cubic_bezier)[0] * t; - Geom::Point c1 = (*cubic_bezier)[1] * t; - Geom::Point c2 = (*cubic_bezier)[2] * t; - Geom::Point c3 = (*cubic_bezier)[3] * t; - cubic_bbox( c0[0], c0[1], - c1[0], c1[1], - c2[0], c2[1], - c3[0], c3[1], - bbox ); + unsigned order = 0; + if (Geom::BezierCurve const* b = dynamic_cast(&c)) { + order = b->order(); } - else - { + + if (order == 1) { // line segment + bbox.expandTo(c.finalPoint() * t); + + // TODO: we can make the case for quadratics faster by degree elevating them to + // cubic and then taking the bbox of that. + + } else if (order == 3) { // cubic bezier + Geom::CubicBezier const &cubic_bezier = static_cast(c); + Geom::Point c0 = cubic_bezier[0] * t; + Geom::Point c1 = cubic_bezier[1] * t; + Geom::Point c2 = cubic_bezier[2] * t; + Geom::Point c3 = cubic_bezier[3] * t; + cubic_bbox(c0[0], c0[1], c1[0], c1[1], c2[0], c2[1], c3[0], c3[1], bbox); + } else { // should handle all not-so-easy curves: Geom::Curve *ctemp = cit->transformed(t); bbox.unionWith( ctemp->boundsExact()); @@ -356,8 +357,11 @@ geom_curve_bbox_wind_distance(Geom::Curve const & c, Geom::Affine const &m, Geom::Coord tolerance, Geom::Rect const *viewbox, Geom::Point &p0) // pass p0 through as it represents the last endpoint added (the finalPoint of last curve) { - if( is_straight_curve(c) ) - { + unsigned order = 0; + if (Geom::BezierCurve const* b = dynamic_cast(&c)) { + order = b->order(); + } + if (order == 1) { Geom::Point pe = c.finalPoint() * m; if (bbox) { bbox->expandTo(pe); @@ -373,10 +377,11 @@ geom_curve_bbox_wind_distance(Geom::Curve const & c, Geom::Affine const &m, } p0 = pe; } - else if(Geom::CubicBezier const *cubic_bezier = dynamic_cast(&c)) { - Geom::Point p1 = (*cubic_bezier)[1] * m; - Geom::Point p2 = (*cubic_bezier)[2] * m; - Geom::Point p3 = (*cubic_bezier)[3] * m; + else if (order == 3) { + Geom::CubicBezier const& cubic_bezier = static_cast(c); + Geom::Point p1 = cubic_bezier[1] * m; + Geom::Point p2 = cubic_bezier[2] * m; + Geom::Point p3 = cubic_bezier[3] * m; // get approximate bbox from handles (convex hull property of beziers): Geom::Rect swept(p0, p3); @@ -402,7 +407,7 @@ geom_curve_bbox_wind_distance(Geom::Curve const & c, Geom::Affine const &m, Geom::Path sbasis_path = Geom::cubicbezierpath_from_sbasis(c.toSBasis(), 0.1); //recurse to convert the new path resulting from the sbasis to svgd - for(Geom::Path::iterator iter = sbasis_path.begin(); iter != sbasis_path.end(); ++iter) { + for (Geom::Path::iterator iter = sbasis_path.begin(); iter != sbasis_path.end(); ++iter) { geom_curve_bbox_wind_distance(*iter, m, pt, bbox, wind, dist, tolerance, viewbox, p0); } } -- cgit v1.2.3