From 16e7cf17a1bc8bb0d79dfa6adc4f75b843fb6d16 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Krzysztof=20Kosi=C5=84ski?= Date: Thu, 4 Feb 2010 03:14:09 +0100 Subject: [PATCH] Reduce libsigc++ usage to partially fix performance regressions in the new node tool. --- src/ui/tool/control-point-selection.cpp | 50 +++------- src/ui/tool/control-point-selection.h | 10 +- src/ui/tool/control-point.cpp | 114 ++++++----------------- src/ui/tool/control-point.h | 39 +++++++- src/ui/tool/curve-drag-point.cpp | 69 ++++++-------- src/ui/tool/curve-drag-point.h | 12 +-- src/ui/tool/multi-path-manipulator.cpp | 4 +- src/ui/tool/node.cpp | 56 +++++------ src/ui/tool/node.h | 31 ++++-- src/ui/tool/path-manipulator.cpp | 35 ------- src/ui/tool/path-manipulator.h | 2 +- src/ui/tool/selectable-control-point.cpp | 35 +++---- src/ui/tool/selectable-control-point.h | 9 +- src/ui/tool/selector.cpp | 28 ++---- src/ui/tool/transform-handle-set.cpp | 18 +--- 15 files changed, 205 insertions(+), 307 deletions(-) diff --git a/src/ui/tool/control-point-selection.cpp b/src/ui/tool/control-point-selection.cpp index 615968fef..de19fef8d 100644 --- a/src/ui/tool/control-point-selection.cpp +++ b/src/ui/tool/control-point-selection.cpp @@ -78,29 +78,7 @@ std::pair ControlPointSelection::insert(c return std::pair(found, false); } - boost::shared_ptr clist(new connlist_type()); - - // hide event param and always return false - clist->push_back( - x->signal_grabbed.connect( - sigc::bind_return( - sigc::hide( - sigc::mem_fun(*this, &ControlPointSelection::_pointGrabbed)), - false))); - clist->push_back( - x->signal_dragged.connect( - sigc::mem_fun(*this, &ControlPointSelection::_pointDragged))); - clist->push_back( - x->signal_ungrabbed.connect( - sigc::hide( - sigc::mem_fun(*this, &ControlPointSelection::_pointUngrabbed)))); - clist->push_back( - x->signal_clicked.connect( - sigc::bind<0>( - sigc::mem_fun(*this, &ControlPointSelection::_pointClicked), - x))); - - found = _points.insert(std::make_pair(x, clist)).first; + found = _points.insert(x).first; x->updateState(); _rot_radius.reset(); @@ -112,11 +90,7 @@ std::pair ControlPointSelection::insert(c /** Remove a point from the selection. */ void ControlPointSelection::erase(iterator pos) { - SelectableControlPoint *erased = pos->first; - boost::shared_ptr clist = pos->second; - for (connlist_type::iterator i = clist->begin(); i != clist->end(); ++i) { - i->disconnect(); - } + SelectableControlPoint *erased = *pos; _points.erase(pos); erased->updateState(); _rot_radius.reset(); @@ -198,7 +172,7 @@ void ControlPointSelection::spatialGrow(SelectableControlPoint *origin, int dir) void ControlPointSelection::transform(Geom::Matrix const &m) { for (iterator i = _points.begin(); i != _points.end(); ++i) { - SelectableControlPoint *cur = i->first; + SelectableControlPoint *cur = *i; cur->transform(m); } // TODO preserving the rotation radius needs some rethinking... @@ -214,14 +188,14 @@ void ControlPointSelection::align(Geom::Dim2 axis) Geom::OptInterval bound; for (iterator i = _points.begin(); i != _points.end(); ++i) { - bound.unionWith(Geom::OptInterval(i->first->position()[d])); + bound.unionWith(Geom::OptInterval((*i)->position()[d])); } double new_coord = bound->middle(); for (iterator i = _points.begin(); i != _points.end(); ++i) { - Geom::Point pos = i->first->position(); + Geom::Point pos = (*i)->position(); pos[d] = new_coord; - i->first->move(pos); + (*i)->move(pos); } } @@ -238,8 +212,8 @@ void ControlPointSelection::distribute(Geom::Dim2 d) // first we insert all points into a multimap keyed by the aligned coord to sort them // simultaneously we compute the extent of selection for (iterator i = _points.begin(); i != _points.end(); ++i) { - Geom::Point pos = i->first->position(); - sm.insert(std::make_pair(pos[d], i->first)); + Geom::Point pos = (*i)->position(); + sm.insert(std::make_pair(pos[d], (*i))); bound.unionWith(Geom::OptInterval(pos[d])); } @@ -261,7 +235,7 @@ Geom::OptRect ControlPointSelection::pointwiseBounds() { Geom::OptRect bound; for (iterator i = _points.begin(); i != _points.end(); ++i) { - SelectableControlPoint *cur = i->first; + SelectableControlPoint *cur = (*i); Geom::Point p = cur->position(); if (!bound) { bound = Geom::Rect(p, p); @@ -276,7 +250,7 @@ Geom::OptRect ControlPointSelection::bounds() { Geom::OptRect bound; for (iterator i = _points.begin(); i != _points.end(); ++i) { - SelectableControlPoint *cur = i->first; + SelectableControlPoint *cur = (*i); Geom::OptRect r = cur->bounds(); bound.unionWith(r); } @@ -320,7 +294,7 @@ void ControlPointSelection::_pointDragged(Geom::Point const &old_pos, Geom::Poin { Geom::Point delta = new_pos - old_pos; for (iterator i = _points.begin(); i != _points.end(); ++i) { - SelectableControlPoint *cur = i->first; + SelectableControlPoint *cur = (*i); cur->move(cur->position() + delta); } _handles->rotationCenter().move(_handles->rotationCenter().position() + delta); @@ -355,7 +329,7 @@ void ControlPointSelection::_updateTransformHandles(bool preserve_center) _handles->setBounds(*b, preserve_center); _handles->setVisible(true); } else if (_one_node_handles && size() == 1) { // only one control point in selection - SelectableControlPoint *p = begin()->first; + SelectableControlPoint *p = *begin(); _handles->setBounds(p->bounds()); _handles->rotationCenter().move(p->position()); _handles->rotationCenter().setVisible(false); diff --git a/src/ui/tool/control-point-selection.h b/src/ui/tool/control-point-selection.h index 54d724f00..025f9bb61 100644 --- a/src/ui/tool/control-point-selection.h +++ b/src/ui/tool/control-point-selection.h @@ -44,14 +44,12 @@ public: ControlPointSelection(SPDesktop *d, SPCanvasGroup *th_group); ~ControlPointSelection(); typedef std::list connlist_type; - typedef std::tr1::unordered_map< SelectableControlPoint *, - boost::shared_ptr > map_type; typedef std::tr1::unordered_set< SelectableControlPoint * > set_type; typedef set_type Set; // convenience alias - typedef map_type::iterator iterator; - typedef map_type::const_iterator const_iterator; - typedef map_type::size_type size_type; + typedef set_type::iterator iterator; + typedef set_type::const_iterator const_iterator; + typedef set_type::size_type size_type; typedef SelectableControlPoint *value_type; typedef SelectableControlPoint *key_type; @@ -127,7 +125,7 @@ private: bool _keyboardFlip(Geom::Dim2); void _keyboardTransform(Geom::Matrix const &); void _commitTransform(CommitEvent ce); - map_type _points; + set_type _points; set_type _all_points; boost::optional _rot_radius; TransformHandleSet *_handles; diff --git a/src/ui/tool/control-point.cpp b/src/ui/tool/control-point.cpp index 941924e0a..2eaddc6b4 100644 --- a/src/ui/tool/control-point.cpp +++ b/src/ui/tool/control-point.cpp @@ -34,46 +34,24 @@ namespace UI { * point in the drawing. The drawing can be changed by dragging the point and the things that are * attached to it with the mouse. Example things that could be edited with draggable points * are gradient stops, the place where text is attached to a path, text kerns, nodes and handles - * in a path, and many more. Control points use signals heavily - read the libsigc++ - * tutorial on the wiki before using this class. + * in a path, and many more. * - * @par Control point signals + * @par Control point event handlers * @par - * The control point has several signals which allow you to react to things that happen to it. - * The most important singals are the grabbed, dragged, ungrabbed and moved signals. - * When a drag happens, the order of emission is as follows: - * - signal_grabbed - * - signal_dragged - * - signal_dragged - * - signal_dragged + * The control point has several virtual methods which allow you to react to things that + * happen to it. The most important ones are the grabbed, dragged, ungrabbed and moved functions. + * When a drag happens, the order of calls is as follows: + * - grabbed() + * - dragged() + * - dragged() + * - dragged() * - ... - * - signal_dragged - * - signal_ungrabbed + * - dragged() + * - ungrabbed() * * The control point can also respond to clicks and double clicks. On a double click, - * signal_clicked is emitted, followed by signal_doubleclicked. - * - * A few signal usage hints if you can't be bothered to read the tutorial: - * - If you want some other object or a global function to react to signals of a control point - * from some other object, and you want to access the control point that emitted the signal - * in the handler, use sigc::bind like this: - * @code - * void handle_clicked_signal(ControlPoint *point, int button); - * point->signal_clicked.connect( - * sigc::bind<0>( sigc::ptr_fun(handle_clicked_signal), - * point )); - * @endcode - * - You can ignore unneeded parameters using sigc::hide. - * - If you want to get rid of the handlers added by constructors in superclasses, - * use the clear() method: @code signal_clicked.clear(); @endcode - * - To connect at the front of the slot list instead of at the end, use: - * @code - * signal_clicked.slots().push_front( - * sigc::mem_fun(*this, &FunkyPoint::_clickedHandler)); - * @endcode - * - Note that calling slots() does not copy anything. You can disconnect - * and reorder slots by manipulating the elements of the slot list. The returned object is - * of type @verbatim (signal type)::slot_list @endverbatim. + * clicked() is called, followed by doubleclicked(). When deriving from SelectableControlPoint, + * you need to manually call the superclass version at the appropriate point in your handler. * * @par Which method to override? * @par @@ -87,46 +65,6 @@ namespace UI { * the setPosition() method. */ -/** - * @var ControlPoint::signal_dragged - * Emitted while dragging, but before moving the knot to new position. - * Old position will always be the same as position() - there are two parameters - * only for convenience. - * - First parameter: old position, always equal to position() - * - Second parameter: new position (after drag). This is passed as a non-const reference, - * so you can change it from the handler - that's how constrained dragging is implemented. - * - Third parameter: motion event - */ - -/** - * @var ControlPoint::signal_clicked - * Emitted when the control point is clicked, at mouse button release. The parameter contains - * the event that caused the signal to be emitted. Your signal handler should return true - * if the click had some effect. If it did nothing, return false. Improperly handling this signal - * can cause the context menu not to appear when a control point is right-clicked. - */ - -/** - * @var ControlPoint::signal_doubleclicked - * Emitted when the control point is doubleclicked, at mouse button release. The parameter - * contains the event that caused the signal to be emitted. Your signal handler should return true - * if the double click had some effect. If it did nothing, return false. - */ - -/** - * @var ControlPoint::signal_grabbed - * Emitted when the control point is grabbed and a drag starts. The parameter contains - * the causing event. Return true to prevent further processing. Because all control points - * handle drag tolerance, signal_dragged will be emitted immediately after this signal - * to move the point to its new position. - */ - -/** - * @var ControlPoint::signal_ungrabbed - * Emitted when the control point finishes a drag. The parameter contains the event which - * caused the signal, but it can be NULL if the grab was broken. - */ - /** * @enum ControlPoint::State * Enumeration representing the possible states of the control point, used to determine @@ -247,9 +185,9 @@ ControlPoint::~ControlPoint() void ControlPoint::_commonInit() { + SP_CTRL(_canvas_item)->moveto(_position); _event_handler_connection = g_signal_connect(G_OBJECT(_canvas_item), "event", G_CALLBACK(_event_handler), this); - SP_CTRL(_canvas_item)->moveto(_position); } /** Relocate the control point without side effects. @@ -408,8 +346,8 @@ bool ControlPoint::_eventHandler(GdkEvent *event) // if we are here, it means the tolerance was just exceeded. next_release_doubleclick = 0; _drag_origin = _position; - transferred = signal_grabbed.emit(&event->motion); - // _drag_initiated might change during the above signal emission + transferred = grabbed(&event->motion); + // _drag_initiated might change during the above virtual call if (!_drag_initiated) { // this guarantees smooth redraws while dragging sp_canvas_force_full_redraw_after_interruptions(_desktop->canvas, 5); @@ -421,7 +359,7 @@ bool ControlPoint::_eventHandler(GdkEvent *event) Geom::Point new_pos = _desktop->w2d(event_point(event->motion)) + pointer_offset; // the new position is passed by reference and can be changed in the handlers. - signal_dragged.emit(_position, new_pos, &event->motion); + dragged(new_pos, &event->motion); move(new_pos); _updateDragTip(&event->motion); // update dragging tip after moving to new position @@ -453,17 +391,17 @@ bool ControlPoint::_eventHandler(GdkEvent *event) if (event->button.button == next_release_doubleclick) { _drag_initiated = false; - return signal_doubleclicked.emit(&event->button); + return doubleclicked(&event->button); } if (event->button.button == 1) { if (_drag_initiated) { // it is the end of a drag - signal_ungrabbed.emit(&event->button); + ungrabbed(&event->button); _drag_initiated = false; return true; } else { // it is the end of a click - return signal_clicked.emit(&event->button); + return clicked(&event->button); } } _drag_initiated = false; @@ -479,7 +417,7 @@ bool ControlPoint::_eventHandler(GdkEvent *event) case GDK_GRAB_BROKEN: if (!event->grab_broken.keyboard && _event_grab) { { - signal_ungrabbed.emit(0); + ungrabbed(NULL); if (_drag_initiated) sp_canvas_end_forced_full_redraws(_desktop->canvas); } @@ -578,7 +516,7 @@ void ControlPoint::transferGrab(ControlPoint *prev_point, GdkEventMotion *event) { if (!_event_grab) return; - signal_grabbed.emit(event); + grabbed(event); sp_canvas_item_ungrab(prev_point->_canvas_item, event->time); sp_canvas_item_grab(_canvas_item, _grab_event_mask, NULL, event->time); @@ -615,6 +553,14 @@ void ControlPoint::_setColors(ColorEntry colors) g_object_set(_canvas_item, "fill_color", colors.fill, "stroke_color", colors.stroke, NULL); } +// dummy implementations for handlers +// they are here to avoid unused param warnings +bool ControlPoint::grabbed(GdkEventMotion *) { return false; } +void ControlPoint::dragged(Geom::Point &, GdkEventMotion *) {} +void ControlPoint::ungrabbed(GdkEventButton *) {} +bool ControlPoint::clicked(GdkEventButton *) { return false; } +bool ControlPoint::doubleclicked(GdkEventButton *) { return false; } + } // namespace UI } // namespace Inkscape diff --git a/src/ui/tool/control-point.h b/src/ui/tool/control-point.h index e59d6d5cf..48c70748b 100644 --- a/src/ui/tool/control-point.h +++ b/src/ui/tool/control-point.h @@ -73,11 +73,11 @@ public: /// @name Receive notifications about control point events /// @{ - sigc::signal signal_dragged; + /*sigc::signal signal_dragged; sigc::signal::accumulated signal_clicked; sigc::signal::accumulated signal_doubleclicked; sigc::signal::accumulated signal_grabbed; - sigc::signal signal_ungrabbed; + sigc::signal signal_ungrabbed;*/ /// @} /// @name Inspect the state of the control point @@ -90,7 +90,7 @@ public: static sigc::signal signal_mouseover_change; static Glib::ustring format_tip(char const *format, ...) G_GNUC_PRINTF(1,2); - // temporarily public, until snapping is refactored a little + // temporarily public, until snap delay is refactored a little virtual bool _eventHandler(GdkEvent *event); protected: @@ -99,6 +99,39 @@ protected: ControlPoint(SPDesktop *d, Geom::Point const &initial_pos, Gtk::AnchorType anchor, Glib::RefPtr pixbuf, ColorSet *cset = 0, SPCanvasGroup *group = 0); + /// @name Handle control point events in subclasses + /// @{ + /** + * Called when the user moves the point beyond the drag tolerance with the first button held + * down. Return true if you called transferGrab() during this method. + * @param event Motion event when drag tolerance was exceeded */ + virtual bool grabbed(GdkEventMotion *event); + /** + * Called while dragging, but before moving the knot to new position. + * @param pos Old position, always equal to position() + * @param new_pos New position (after drag). This is passed as a non-const reference, + * so you can change it from the handler - that's how constrained dragging is implemented. + * @param event Motion event */ + virtual void dragged(Geom::Point &new_pos, GdkEventMotion *event); + /** + * @var ControlPoint::signal_ungrabbed + * Emitted when the control point finishes a drag. + * @param event Button release event + */ + virtual void ungrabbed(GdkEventButton *event); + /** + * Called when the control point is clicked, at mouse button release. Your override should + * return true if the click had some effect. If it did nothing, return false. Improperly + * implementing this method can cause the default context menu not to appear when a control + * point is right-clicked. + * @param event Button release event */ + virtual bool clicked(GdkEventButton *event); + /** + * Called when the control point is doubleclicked, at mouse button release. + * @param event Button release event */ + virtual bool doubleclicked(GdkEventButton *); + /// @} + /// @name Manipulate the control point's appearance in subclasses /// @{ virtual void _setState(State state); diff --git a/src/ui/tool/curve-drag-point.cpp b/src/ui/tool/curve-drag-point.cpp index 0e0276173..88cb72ed5 100644 --- a/src/ui/tool/curve-drag-point.cpp +++ b/src/ui/tool/curve-drag-point.cpp @@ -42,23 +42,9 @@ CurveDragPoint::CurveDragPoint(PathManipulator &pm) , _pm(pm) { setVisible(false); - signal_grabbed.connect( - sigc::bind_return( - sigc::mem_fun(*this, &CurveDragPoint::_grabbedHandler), - false)); - signal_dragged.connect( - sigc::hide( - sigc::mem_fun(*this, &CurveDragPoint::_draggedHandler))); - signal_ungrabbed.connect( - sigc::hide( - sigc::mem_fun(*this, &CurveDragPoint::_ungrabbedHandler))); - signal_clicked.connect( - sigc::mem_fun(*this, &CurveDragPoint::_clickedHandler)); - signal_doubleclicked.connect( - sigc::mem_fun(*this, &CurveDragPoint::_doubleclickedHandler)); } -void CurveDragPoint::_grabbedHandler(GdkEventMotion */*event*/) +bool CurveDragPoint::grabbed(GdkEventMotion */*event*/) { _pm._selection.hideTransformHandles(); NodeList::iterator second = first.next(); @@ -71,44 +57,41 @@ void CurveDragPoint::_grabbedHandler(GdkEventMotion */*event*/) first->front()->move(first->front()->position() + delta); second->back()->move(second->back()->position() - delta); - signal_update.emit(); + _pm.update(); } + return false; } -void CurveDragPoint::_draggedHandler(Geom::Point const &old_pos, Geom::Point const &new_pos) +void CurveDragPoint::dragged(Geom::Point &new_pos, GdkEventMotion *) { - if (_drags_stroke) { - // TODO - } else { - NodeList::iterator second = first.next(); - // Magic Bezier Drag Equations follow! - // "weight" describes how the influence of the drag should be distributed - // among the handles; 0 = front handle only, 1 = back handle only. - double weight, t = _t; - if (t <= 1.0 / 6.0) weight = 0; - else if (t <= 0.5) weight = (pow((6 * t - 1) / 2.0, 3)) / 2; - else if (t <= 5.0 / 6.0) weight = (1 - pow((6 * (1-t) - 1) / 2.0, 3)) / 2 + 0.5; - else weight = 1; - - Geom::Point delta = new_pos - old_pos; - Geom::Point offset0 = ((1-weight)/(3*t*(1-t)*(1-t))) * delta; - Geom::Point offset1 = (weight/(3*t*t*(1-t))) * delta; - - first->front()->move(first->front()->position() + offset0); - second->back()->move(second->back()->position() + offset1); - } - - signal_update.emit(); + NodeList::iterator second = first.next(); + // Magic Bezier Drag Equations follow! + // "weight" describes how the influence of the drag should be distributed + // among the handles; 0 = front handle only, 1 = back handle only. + double weight, t = _t; + if (t <= 1.0 / 6.0) weight = 0; + else if (t <= 0.5) weight = (pow((6 * t - 1) / 2.0, 3)) / 2; + else if (t <= 5.0 / 6.0) weight = (1 - pow((6 * (1-t) - 1) / 2.0, 3)) / 2 + 0.5; + else weight = 1; + + Geom::Point delta = new_pos - position(); + Geom::Point offset0 = ((1-weight)/(3*t*(1-t)*(1-t))) * delta; + Geom::Point offset1 = (weight/(3*t*t*(1-t))) * delta; + + first->front()->move(first->front()->position() + offset0); + second->back()->move(second->back()->position() + offset1); + + _pm.update(); } -void CurveDragPoint::_ungrabbedHandler() +void CurveDragPoint::ungrabbed(GdkEventButton *) { _pm._updateDragPoint(_desktop->d2w(position())); _pm._commit(_("Drag curve")); _pm._selection.restoreTransformHandles(); } -bool CurveDragPoint::_clickedHandler(GdkEventButton *event) +bool CurveDragPoint::clicked(GdkEventButton *event) { // This check is probably redundant if (!first || event->button != 1) return false; @@ -141,7 +124,7 @@ bool CurveDragPoint::_clickedHandler(GdkEventButton *event) return true; } -bool CurveDragPoint::_doubleclickedHandler(GdkEventButton *event) +bool CurveDragPoint::doubleclicked(GdkEventButton *event) { if (event->button != 1 || !first || !first.next()) return false; _insertNode(true); @@ -160,7 +143,7 @@ void CurveDragPoint::_insertNode(bool take_selection) } _pm._selection.insert(inserted.ptr()); - signal_update.emit(); + _pm.update(); _pm._commit(_("Add node")); } diff --git a/src/ui/tool/curve-drag-point.h b/src/ui/tool/curve-drag-point.h index 51382615e..147a91837 100644 --- a/src/ui/tool/curve-drag-point.h +++ b/src/ui/tool/curve-drag-point.h @@ -27,15 +27,15 @@ public: void setSize(double sz) { _setSize(sz); } void setTimeValue(double t) { _t = t; } void setIterator(NodeList::iterator i) { first = i; } - sigc::signal signal_update; protected: virtual Glib::ustring _getTip(unsigned state); private: - void _grabbedHandler(GdkEventMotion *); - void _draggedHandler(Geom::Point const &, Geom::Point const &); - bool _clickedHandler(GdkEventButton *); - bool _doubleclickedHandler(GdkEventButton *); - void _ungrabbedHandler(); + virtual void dragged(Geom::Point &, GdkEventMotion *); + virtual bool grabbed(GdkEventMotion *); + virtual void ungrabbed(GdkEventButton *); + virtual bool clicked(GdkEventButton *); + virtual bool doubleclicked(GdkEventButton *); + void _insertNode(bool take_selection); double _t; PathManipulator &_pm; diff --git a/src/ui/tool/multi-path-manipulator.cpp b/src/ui/tool/multi-path-manipulator.cpp index d133fcf25..c34ef066e 100644 --- a/src/ui/tool/multi-path-manipulator.cpp +++ b/src/ui/tool/multi-path-manipulator.cpp @@ -45,7 +45,7 @@ void find_join_iterators(ControlPointSelection &sel, IterPairList &pairs) // find all endnodes in selection for (ControlPointSelection::iterator i = sel.begin(); i != sel.end(); ++i) { - Node *node = dynamic_cast(i->first); + Node *node = dynamic_cast(*i); if (!node) continue; NodeList::iterator iter = NodeList::get_iterator(node); if (!iter.next() || !iter.prev()) join_iters.insert(iter); @@ -203,7 +203,7 @@ void MultiPathManipulator::setNodeType(NodeType type) { if (_selection.empty()) return; for (ControlPointSelection::iterator i = _selection.begin(); i != _selection.end(); ++i) { - Node *node = dynamic_cast(i->first); + Node *node = dynamic_cast(*i); if (node) node->setType(type); } _done(_("Change node type")); diff --git a/src/ui/tool/node.cpp b/src/ui/tool/node.cpp index 2675b4045..6419acf6d 100644 --- a/src/ui/tool/node.cpp +++ b/src/ui/tool/node.cpp @@ -89,20 +89,10 @@ Handle::Handle(NodeSharedData const &data, Geom::Point const &initial_pos, Node _cset = &handle_colors; _handle_line = sp_canvas_item_new(data.handle_line_group, SP_TYPE_CTRLLINE, NULL); setVisible(false); - signal_grabbed.connect( - sigc::bind_return( - sigc::hide( - sigc::mem_fun(*this, &Handle::_grabbedHandler)), - false)); - signal_dragged.connect( - sigc::hide<0>( - sigc::mem_fun(*this, &Handle::_draggedHandler))); - signal_ungrabbed.connect( - sigc::hide(sigc::mem_fun(*this, &Handle::_ungrabbedHandler))); } Handle::~Handle() { - sp_canvas_item_hide(_handle_line); + //sp_canvas_item_hide(_handle_line); gtk_object_destroy(GTK_OBJECT(_handle_line)); } @@ -232,12 +222,14 @@ char const *Handle::handle_type_to_localized_string(NodeType type) } } -void Handle::_grabbedHandler() +bool Handle::grabbed(GdkEventMotion *) { _saved_length = _drag_out ? 0 : length(); + _pm()._handleGrabbed(); + return false; } -void Handle::_draggedHandler(Geom::Point &new_pos, GdkEventMotion *event) +void Handle::dragged(Geom::Point &new_pos, GdkEventMotion *event) { Geom::Point parent_pos = _parent->position(); // with Alt, preserve length @@ -253,12 +245,13 @@ void Handle::_draggedHandler(Geom::Point &new_pos, GdkEventMotion *event) new_pos = parent_pos + Geom::constrain_angle(Geom::Point(0,0), new_pos - parent_pos, snaps, _drag_out ? Geom::Point(1,0) : Geom::unit_vector(rel_origin)); } - signal_update.emit(); + _pm().update(); } -void Handle::_ungrabbedHandler() +void Handle::ungrabbed(GdkEventButton *) { // hide the handle if it's less than dragtolerance away from the node + // TODO is this actually desired? Inkscape::Preferences *prefs = Inkscape::Preferences::get(); int drag_tolerance = prefs->getIntLimited("/options/dragtolerance/value", 0, 0, 100); @@ -267,6 +260,14 @@ void Handle::_ungrabbedHandler() move(_parent->position()); } _drag_out = false; + + _pm()._handleUngrabbed(); +} + +bool Handle::clicked(GdkEventButton *event) +{ + _pm()._handleClicked(this, event); + return true; } static double snap_increment_degrees() { @@ -337,11 +338,6 @@ Node::Node(NodeSharedData const &data, Geom::Point const &initial_pos) , _handles_shown(false) { // NOTE we do not set type here, because the handles are still degenerate - // connect to own grabbed signal - dragging out handles - signal_grabbed.connect( - sigc::mem_fun(*this, &Node::_grabbedHandler)); - signal_dragged.connect( sigc::hide<0>( - sigc::mem_fun(*this, &Node::_draggedHandler))); } // NOTE: not using iterators won't make this much quicker because iterators can be 100% inlined. @@ -807,8 +803,11 @@ void Node::_setState(State state) SelectableControlPoint::_setState(state); } -bool Node::_grabbedHandler(GdkEventMotion *event) +bool Node::grabbed(GdkEventMotion *event) { + if (SelectableControlPoint::grabbed(event)) + return true; + // Dragging out handles with Shift + drag on a node. if (!held_shift(*event)) return false; @@ -843,7 +842,7 @@ bool Node::_grabbedHandler(GdkEventMotion *event) return true; } -void Node::_draggedHandler(Geom::Point &new_pos, GdkEventMotion *event) +void Node::dragged(Geom::Point &new_pos, GdkEventMotion *event) { // For a note on how snapping is implemented in Inkscape, see snap.h. SnapManager &sm = _desktop->namedview->snap_manager; @@ -923,6 +922,15 @@ void Node::_draggedHandler(Geom::Point &new_pos, GdkEventMotion *event) } else if (snap) { sm.freeSnapReturnByRef(new_pos, _snapSourceType()); } + + SelectableControlPoint::dragged(new_pos, event); +} + +bool Node::clicked(GdkEventButton *event) +{ + if(_pm()._nodeClicked(this, event)) + return true; + return SelectableControlPoint::clicked(event); } Inkscape::SnapSourceType Node::_snapSourceType() @@ -1080,7 +1088,6 @@ NodeList::iterator NodeList::insert(iterator i, Node *x) ins->prev->next = x; ins->prev = x; x->ListNode::list = this; - _list.signal_insert_node.emit(x); return iterator(x); } @@ -1100,9 +1107,7 @@ void NodeList::splice(iterator pos, NodeList &list, iterator first, iterator las { ListNode *ins_beg = first._node, *ins_end = last._node, *at = pos._node; for (ListNode *ln = ins_beg; ln != ins_end; ln = ln->next) { - list._list.signal_remove_node.emit(static_cast(ln)); ln->list = this; - _list.signal_insert_node.emit(static_cast(ln)); } ins_beg->prev->next = ins_end; ins_end->prev->next = at; @@ -1157,7 +1162,6 @@ NodeList::iterator NodeList::erase(iterator i) Node *rm = static_cast(i._node); ListNode *rmnext = rm->next, *rmprev = rm->prev; ++i; - _list.signal_remove_node.emit(rm); delete rm; rmprev->next = rmnext; rmnext->prev = rmprev; diff --git a/src/ui/tool/node.h b/src/ui/tool/node.h index d822d854f..581cc9b6f 100644 --- a/src/ui/tool/node.h +++ b/src/ui/tool/node.h @@ -11,6 +11,7 @@ #ifndef SEEN_UI_TOOL_NODE_H #define SEEN_UI_TOOL_NODE_H +#include #include #include #include @@ -97,16 +98,19 @@ public: Node *parent() { return _parent; } static char const *handle_type_to_localized_string(NodeType type); - sigc::signal signal_update; protected: Handle(NodeSharedData const &data, Geom::Point const &initial_pos, Node *parent); + + virtual void dragged(Geom::Point &, GdkEventMotion *); + virtual bool grabbed(GdkEventMotion *); + virtual void ungrabbed(GdkEventButton *); + virtual bool clicked(GdkEventButton *); + virtual Glib::ustring _getTip(unsigned state); virtual Glib::ustring _getDragTip(GdkEventMotion *event); virtual bool _hasDragTips() { return true; } private: - void _grabbedHandler(); - void _draggedHandler(Geom::Point &, GdkEventMotion *); - void _ungrabbedHandler(); + inline PathManipulator &_pm(); Node *_parent; // the handle's lifetime does not extend beyond that of the parent node, // so a naked pointer is OK and allows setting it during Node's construction SPCanvasItem *_handle_line; @@ -140,14 +144,16 @@ public: // temporarily public virtual bool _eventHandler(GdkEvent *event); protected: + virtual void dragged(Geom::Point &, GdkEventMotion *); + virtual bool grabbed(GdkEventMotion *); + virtual bool clicked(GdkEventButton *); + virtual void _setState(State state); virtual Glib::ustring _getTip(unsigned state); virtual Glib::ustring _getDragTip(GdkEventMotion *event); virtual bool _hasDragTips() { return true; } private: Node(Node const &); - bool _grabbedHandler(GdkEventMotion *); - void _draggedHandler(Geom::Point &, GdkEventMotion *); void _fixNeighbors(Geom::Point const &old_pos, Geom::Point const &new_pos); void _updateAutoHandles(); void _linearGrow(int dir); @@ -155,6 +161,7 @@ private: Node *_prev(); Inkscape::SnapSourceType _snapSourceType(); Inkscape::SnapTargetType _snapTargetType(); + inline PathManipulator &_pm(); static SPCtrlShapeType _node_type_to_shape(NodeType type); static bool _is_line_segment(Node *first, Node *second); @@ -327,8 +334,6 @@ public: SubpathList(PathManipulator &pm) : _path_manipulator(pm) {} - sigc::signal signal_insert_node; - sigc::signal signal_remove_node; private: list_type _nodelists; PathManipulator &_path_manipulator; @@ -349,6 +354,12 @@ inline void Handle::setRelativePos(Geom::Point const &p) { inline double Handle::length() { return relativePos().length(); } +inline PathManipulator &Handle::_pm() { + return _parent->_pm(); +} +inline PathManipulator &Node::_pm() { + return list()->_list._path_manipulator; +} // definitions for node iterator template @@ -359,14 +370,14 @@ template NodeIterator NodeIterator::next() const { NodeIterator ret(*this); ++ret; - if (!ret && _node->list->closed()) ++ret; + if (G_UNLIKELY(!ret) && _node->list->closed()) ++ret; return ret; } template NodeIterator NodeIterator::prev() const { NodeIterator ret(*this); --ret; - if (!ret && _node->list->closed()) --ret; + if (G_UNLIKELY(!ret) && _node->list->closed()) --ret; return ret; } diff --git a/src/ui/tool/path-manipulator.cpp b/src/ui/tool/path-manipulator.cpp index 0ce02aa95..43955edbf 100644 --- a/src/ui/tool/path-manipulator.cpp +++ b/src/ui/tool/path-manipulator.cpp @@ -119,19 +119,10 @@ PathManipulator::PathManipulator(MultiPathManipulator &mpm, SPPath *path, SP_STROKE_LINEJOIN_MITER, SP_STROKE_LINECAP_BUTT); sp_canvas_bpath_set_fill(SP_CANVAS_BPATH(_outline), 0, SP_WIND_RULE_NONZERO); - _subpaths.signal_insert_node.connect( - sigc::mem_fun(*this, &PathManipulator::_attachNodeHandlers)); - // NOTE: signal_remove_node is called just before destruction. Nodes are trackable, - // so removing the signals manually is not necessary. - /*_subpaths.signal_remove_node.connect( - sigc::mem_fun(*this, &PathManipulator::_removeNodeHandlers));*/ - _selection.signal_update.connect( sigc::mem_fun(*this, &PathManipulator::update)); _selection.signal_point_changed.connect( sigc::mem_fun(*this, &PathManipulator::_selectionChanged)); - _dragpoint->signal_update.connect( - sigc::mem_fun(*this, &PathManipulator::update)); _desktop->signal_zoom_changed.connect( sigc::hide( sigc::mem_fun(*this, &PathManipulator::_updateOutlineOnZoomChange))); @@ -1174,34 +1165,8 @@ Inkscape::XML::Node *PathManipulator::_getXMLNode() return LIVEPATHEFFECT(_path)->repr; } -void PathManipulator::_attachNodeHandlers(Node *node) -{ - Handle *handles[2] = { node->front(), node->back() }; - for (int i = 0; i < 2; ++i) { - handles[i]->signal_update.connect( - sigc::mem_fun(*this, &PathManipulator::update)); - handles[i]->signal_ungrabbed.connect( - sigc::hide( - sigc::mem_fun(*this, &PathManipulator::_handleUngrabbed))); - handles[i]->signal_grabbed.connect( - sigc::bind_return( - sigc::hide( - sigc::mem_fun(*this, &PathManipulator::_handleGrabbed)), - false)); - handles[i]->signal_clicked.connect( - sigc::bind<0>( - sigc::mem_fun(*this, &PathManipulator::_handleClicked), - handles[i])); - } - node->signal_clicked.connect( - sigc::bind<0>( - sigc::mem_fun(*this, &PathManipulator::_nodeClicked), - node)); -} - bool PathManipulator::_nodeClicked(Node *n, GdkEventButton *event) { - // cycle between node types on ctrl+click if (event->button != 1) return false; if (held_alt(*event) && held_control(*event)) { // Ctrl+Alt+click: delete nodes diff --git a/src/ui/tool/path-manipulator.h b/src/ui/tool/path-manipulator.h index 6b69e226a..1e9e50575 100644 --- a/src/ui/tool/path-manipulator.h +++ b/src/ui/tool/path-manipulator.h @@ -104,7 +104,6 @@ private: Glib::ustring _nodetypesKey(); Inkscape::XML::Node *_getXMLNode(); - void _attachNodeHandlers(Node *n); void _selectionChanged(SelectableControlPoint *p, bool selected); bool _nodeClicked(Node *, GdkEventButton *); void _handleGrabbed(); @@ -139,6 +138,7 @@ private: friend class PathManipulatorObserver; friend class CurveDragPoint; friend class Node; + friend class Handle; }; } // namespace UI diff --git a/src/ui/tool/selectable-control-point.cpp b/src/ui/tool/selectable-control-point.cpp index 9039bd533..1835f0008 100644 --- a/src/ui/tool/selectable-control-point.cpp +++ b/src/ui/tool/selectable-control-point.cpp @@ -34,7 +34,7 @@ SelectableControlPoint::SelectableControlPoint(SPDesktop *d, Geom::Point const & : reinterpret_cast(&default_scp_color_set), group) , _selection (sel) { - _connectHandlers(); + _selection.allPoints().insert(this); } SelectableControlPoint::SelectableControlPoint(SPDesktop *d, Geom::Point const &initial_pos, Gtk::AnchorType anchor, Glib::RefPtr pixbuf, @@ -44,7 +44,7 @@ SelectableControlPoint::SelectableControlPoint(SPDesktop *d, Geom::Point const & : reinterpret_cast(&default_scp_color_set), group) , _selection (sel) { - _connectHandlers(); + _selection.allPoints().insert(this); } SelectableControlPoint::~SelectableControlPoint() @@ -53,28 +53,31 @@ SelectableControlPoint::~SelectableControlPoint() _selection.allPoints().erase(this); } -void SelectableControlPoint::_connectHandlers() -{ - _selection.allPoints().insert(this); - signal_grabbed.connect( - sigc::bind_return( - sigc::hide( - sigc::mem_fun(*this, &SelectableControlPoint::_grabbedHandler)), - false)); - signal_clicked.connect( - sigc::mem_fun(*this, &SelectableControlPoint::_clickedHandler)); -} - -void SelectableControlPoint::_grabbedHandler() +bool SelectableControlPoint::grabbed(GdkEventMotion *) { // if a point is dragged while not selected, it should select itself if (!selected()) { _takeSelection(); } + _selection._pointGrabbed(); + return false; } -bool SelectableControlPoint::_clickedHandler(GdkEventButton *event) +void SelectableControlPoint::dragged(Geom::Point &new_pos, GdkEventMotion *event) { + _selection._pointDragged(position(), new_pos, event); +} + +void SelectableControlPoint::ungrabbed(GdkEventButton *) +{ + _selection._pointUngrabbed(); +} + +bool SelectableControlPoint::clicked(GdkEventButton *event) +{ + if (_selection._pointClicked(this, event)) + return true; + if (event->button != 1) return false; if (held_shift(*event)) { if (selected()) { diff --git a/src/ui/tool/selectable-control-point.h b/src/ui/tool/selectable-control-point.h index f5e9541c3..2fde16ea9 100644 --- a/src/ui/tool/selectable-control-point.h +++ b/src/ui/tool/selectable-control-point.h @@ -45,13 +45,14 @@ protected: virtual void _setState(State state); + virtual void dragged(Geom::Point &, GdkEventMotion *); + virtual bool grabbed(GdkEventMotion *); + virtual void ungrabbed(GdkEventButton *); + virtual bool clicked(GdkEventButton *); + ControlPointSelection &_selection; private: - void _connectHandlers(); void _takeSelection(); - - void _grabbedHandler(); - bool _clickedHandler(GdkEventButton *); }; } // namespace UI diff --git a/src/ui/tool/selector.cpp b/src/ui/tool/selector.cpp index bf3ea6714..8d3cf5650 100644 --- a/src/ui/tool/selector.cpp +++ b/src/ui/tool/selector.cpp @@ -36,17 +36,6 @@ public: _rubber = static_cast(sp_canvas_item_new(sp_desktop_controls(_desktop), SP_TYPE_CTRLRECT, NULL)); sp_canvas_item_hide(_rubber); - - signal_clicked.connect(sigc::mem_fun(*this, &SelectorPoint::_clicked)); - signal_grabbed.connect( - sigc::bind_return( - sigc::hide( - sigc::mem_fun(*this, &SelectorPoint::_grabbed)), - false)); - signal_dragged.connect( - sigc::hide<0>( sigc::hide( - sigc::mem_fun(*this, &SelectorPoint::_dragged)))); - signal_ungrabbed.connect(sigc::mem_fun(*this, &SelectorPoint::_ungrabbed)); } ~SelectorPoint() { gtk_object_destroy(_rubber); @@ -69,27 +58,28 @@ protected: } private: - bool _clicked(GdkEventButton *event) { - if (event->button != 1) return false; - _selector->signal_point.emit(position(), event); - return true; - } - void _grabbed() { + virtual bool grabbed(GdkEventMotion *) { _cancel = false; _start = position(); sp_canvas_item_show(_rubber); + return false; } - void _dragged(Geom::Point &new_pos) { + virtual void dragged(Geom::Point &new_pos, GdkEventMotion *) { if (_cancel) return; Geom::Rect sel(_start, new_pos); _rubber->setRectangle(sel); } - void _ungrabbed(GdkEventButton *event) { + virtual void ungrabbed(GdkEventButton *event) { if (_cancel) return; sp_canvas_item_hide(_rubber); Geom::Rect sel(_start, position()); _selector->signal_area.emit(sel, event); } + virtual bool clicked(GdkEventButton *event) { + if (event->button != 1) return false; + _selector->signal_point.emit(position(), event); + return true; + } CtrlRect *_rubber; Selector *_selector; Geom::Point _start; diff --git a/src/ui/tool/transform-handle-set.cpp b/src/ui/tool/transform-handle-set.cpp index f9086950d..1af848b96 100644 --- a/src/ui/tool/transform-handle-set.cpp +++ b/src/ui/tool/transform-handle-set.cpp @@ -84,17 +84,6 @@ public: , _th(th) { setVisible(false); - signal_grabbed.connect( - sigc::bind_return( - sigc::hide( - sigc::mem_fun(*this, &TransformHandle::_grabbedHandler)), - false)); - signal_dragged.connect( - sigc::hide<0>( - sigc::mem_fun(*this, &TransformHandle::_draggedHandler))); - signal_ungrabbed.connect( - sigc::hide( - sigc::mem_fun(*this, &TransformHandle::_ungrabbedHandler))); } protected: virtual void startTransform() {} @@ -106,7 +95,7 @@ protected: Geom::Point _origin; TransformHandleSet &_th; private: - void _grabbedHandler() { + virtual bool grabbed(GdkEventMotion *) { _origin = position(); _last_transform.setIdentity(); startTransform(); @@ -114,8 +103,9 @@ private: _th._setActiveHandle(this); _cset = &invisible_cset; _setState(_state); + return false; } - void _draggedHandler(Geom::Point &new_pos, GdkEventMotion *event) + virtual void dragged(Geom::Point &new_pos, GdkEventMotion *event) { Geom::Matrix t = computeTransform(new_pos, event); // protect against degeneracies @@ -125,7 +115,7 @@ private: _th.signal_transform.emit(incr); _last_transform = t; } - void _ungrabbedHandler() { + virtual void ungrabbed(GdkEventButton *) { _th._clearActiveHandle(); _cset = &thandle_cset; _setState(_state); -- 2.30.2