Version: HEAD (using KDE KDE 3.5.0) Installed from: Compiled From Sources OS: Linux PROBLEM: When moving an object within a window it should snap to nearby borders of other objects. However, it does so only for objects that are at the top level of a window, and none of their descendants. STEPS TO REPRODUCE: Start Kst In the default window create a large rectangle Create a smaller retangle Move the smaller rectangle into the larger rectangle (implcitly parenting it) Create a third rectangle Move the third rectangle around RESULTS: The third rectangle onlys snaps to the edges of the first rectangle EXPECTED RESULTS: The thrid rectangle should snap to the edges of both the first and second rectangles
SVN commit 541977 by arwalker: BUG:127536 Ensure moving object snaps to edges of all objects M +2 -35 ksttoplevelview.cpp M +38 -0 kstviewobject.cpp M +2 -0 kstviewobject.h --- trunk/extragear/graphics/kst/src/libkstapp/ksttoplevelview.cpp #541976:541977 @@ -707,41 +707,8 @@ int iXMin = STICKY_THRESHOLD; int iYMin = STICKY_THRESHOLD; - // check for "sticky" borders - for (KstViewObjectList::Iterator i = _children.begin(); i != _children.end(); ++i) { - if (_selectionList.find(*i) == _selectionList.end() && _pressTarget != *i) { - const QRect rect((*i)->geometry()); + snapToBorders(&iXMin, &iYMin, _selectionList, _pressTarget, r); - int overlapLo = r.top() > rect.top() ? r.top() : rect.top(); - int overlapHi = r.bottom() < rect.bottom() ? r.bottom() : rect.bottom(); - if (overlapHi - overlapLo > 0) { - if (labs(r.left() - rect.left()) < labs(iXMin)) { - iXMin = r.left() - rect.left(); - } else if (labs(r.left() - rect.right()) < labs(iXMin)) { - iXMin = r.left() - rect.right(); - } else if (labs(r.right() - rect.left()) < labs(iXMin)) { - iXMin = r.right() - rect.left(); - } else if (labs(r.right() - rect.right()) < labs(iXMin)) { - iXMin = r.right() - rect.right(); - } - } - - overlapLo = r.left() > rect.left() ? r.left() : rect.left(); - overlapHi = r.right() < rect.right() ? r.right() : rect.right(); - if (overlapHi - overlapLo > 0) { - if (labs(r.top() - rect.top()) < labs(iYMin)) { - iYMin = r.top() - rect.top(); - } else if (labs(r.top() - rect.bottom()) < labs(iYMin)) { - iYMin = r.top() - rect.bottom(); - } else if (labs(r.bottom() - rect.top()) < labs(iYMin)) { - iYMin = r.bottom() - rect.top(); - } else if (labs(r.bottom() - rect.bottom()) < labs(iYMin)) { - iYMin = r.bottom() - rect.bottom(); - } - } - } - } - if (labs(iXMin) < STICKY_THRESHOLD) { _moveOffsetSticky.setX(iXMin); topLeft.setX(topLeft.x() - iXMin); @@ -751,7 +718,7 @@ topLeft.setY(topLeft.y() - iYMin); } - r.moveTopLeft(topLeft); + r.moveTopLeft(topLeft); if (!_geom.contains(r, true)) { slideInto(_geom, r); --- trunk/extragear/graphics/kst/src/libkstapp/kstviewobject.cpp #541976:541977 @@ -463,6 +463,44 @@ } +void KstViewObject::snapToBorders(int *iXMin, int *iYMin, KstViewObjectList &selectionList, KstViewObjectPtr &pressTarget, const QRect &r) { + for (KstViewObjectList::Iterator i = _children.begin(); i != _children.end(); ++i) { + (*i)->snapToBorders(iXMin, iYMin, selectionList, pressTarget, r); + if (selectionList.find(*i) == selectionList.end() && pressTarget != *i) { + const QRect rect((*i)->geometry()); + + int overlapLo = r.top() > rect.top() ? r.top() : rect.top(); + int overlapHi = r.bottom() < rect.bottom() ? r.bottom() : rect.bottom(); + if (overlapHi - overlapLo > 0) { + if (labs(r.left() - rect.left()) < labs(*iXMin)) { + *iXMin = r.left() - rect.left(); + } else if (labs(r.left() - rect.right()) < labs(*iXMin)) { + *iXMin = r.left() - rect.right(); + } else if (labs(r.right() - rect.left()) < labs(*iXMin)) { + *iXMin = r.right() - rect.left(); + } else if (labs(r.right() - rect.right()) < labs(*iXMin)) { + *iXMin = r.right() - rect.right(); + } + } + + overlapLo = r.left() > rect.left() ? r.left() : rect.left(); + overlapHi = r.right() < rect.right() ? r.right() : rect.right(); + if (overlapHi - overlapLo > 0) { + if (labs(r.top() - rect.top()) < labs(*iYMin)) { + *iYMin = r.top() - rect.top(); + } else if (labs(r.top() - rect.bottom()) < labs(*iYMin)) { + *iYMin = r.top() - rect.bottom(); + } else if (labs(r.bottom() - rect.top()) < labs(*iYMin)) { + *iYMin = r.bottom() - rect.top(); + } else if (labs(r.bottom() - rect.bottom()) < labs(*iYMin)) { + *iYMin = r.bottom() - rect.bottom(); + } + } + } + } +} + + void KstViewObject::appendChild(KstViewObjectPtr obj, bool keepAspect) { obj->_parent = this; _children.append(obj); --- trunk/extragear/graphics/kst/src/libkstapp/kstviewobject.h #541976:541977 @@ -118,6 +118,8 @@ virtual bool maintainAspect() const; virtual void setMaintainAspect(bool maintain); + virtual void snapToBorders(int *iXMin, int *iYMin, KstViewObjectList &selectionList, KstViewObjectPtr &pressTarget, const QRect &r); + virtual void appendChild(KstViewObjectPtr obj, bool keepAspect = false); virtual void prependChild(KstViewObjectPtr obj, bool keepAspect = false); virtual bool removeChild(KstViewObjectPtr obj, bool recursive = false);
Backed out after review
In the interests of a coherent bug report following are George's reasons for reverting the prior fix: > This code does not belong in KstViewObject, should not be a virtual function, > should take const references as parameters, should use a const iterator, > should not use iVariables, and should be a const function. If anything in > there prevents fixing constness, it's probably pointing to another issue > elsewhere. > Anyway, all this belongs in the view, not in each object.
snapToBorders() should be a virtual member of KstViewObject as its behaviour may differ depending on the object that is being snapped to. For example, it might be desirable to have objects snap to both a plot and its axes. For rectangles we might want objects to snap to both its outer edge and inner edge. For some objects we may not want to snap to any of their children. I think it makes sense to enable this behaviour this now.
SVN commit 544672 by arwalker: BUG:127536 Ensure that when mocing sticky borders apply to all objects and not just top-level objects. M +10 -43 ksttoplevelview.cpp M +39 -0 kstviewobject.cpp M +2 -0 kstviewobject.h --- trunk/extragear/graphics/kst/src/libkstapp/ksttoplevelview.cpp #544671:544672 @@ -704,54 +704,21 @@ r.moveTopLeft(topLeft); _moveOffsetSticky = QPoint(0, 0); - int iXMin = STICKY_THRESHOLD; - int iYMin = STICKY_THRESHOLD; + int xMin = STICKY_THRESHOLD; + int yMin = STICKY_THRESHOLD; - // check for "sticky" borders - for (KstViewObjectList::Iterator i = _children.begin(); i != _children.end(); ++i) { - if (_selectionList.find(*i) == _selectionList.end() && _pressTarget != *i) { - const QRect rect((*i)->geometry()); + snapToBorders(&xMin, &yMin, _selectionList, _pressTarget, r); - int overlapLo = r.top() > rect.top() ? r.top() : rect.top(); - int overlapHi = r.bottom() < rect.bottom() ? r.bottom() : rect.bottom(); - if (overlapHi - overlapLo > 0) { - if (labs(r.left() - rect.left()) < labs(iXMin)) { - iXMin = r.left() - rect.left(); - } else if (labs(r.left() - rect.right()) < labs(iXMin)) { - iXMin = r.left() - rect.right(); - } else if (labs(r.right() - rect.left()) < labs(iXMin)) { - iXMin = r.right() - rect.left(); - } else if (labs(r.right() - rect.right()) < labs(iXMin)) { - iXMin = r.right() - rect.right(); - } - } - - overlapLo = r.left() > rect.left() ? r.left() : rect.left(); - overlapHi = r.right() < rect.right() ? r.right() : rect.right(); - if (overlapHi - overlapLo > 0) { - if (labs(r.top() - rect.top()) < labs(iYMin)) { - iYMin = r.top() - rect.top(); - } else if (labs(r.top() - rect.bottom()) < labs(iYMin)) { - iYMin = r.top() - rect.bottom(); - } else if (labs(r.bottom() - rect.top()) < labs(iYMin)) { - iYMin = r.bottom() - rect.top(); - } else if (labs(r.bottom() - rect.bottom()) < labs(iYMin)) { - iYMin = r.bottom() - rect.bottom(); - } - } - } + if (labs(xMin) < STICKY_THRESHOLD) { + _moveOffsetSticky.setX(xMin); + topLeft.setX(topLeft.x() - xMin); } - - if (labs(iXMin) < STICKY_THRESHOLD) { - _moveOffsetSticky.setX(iXMin); - topLeft.setX(topLeft.x() - iXMin); + if (labs(yMin) < STICKY_THRESHOLD) { + _moveOffsetSticky.setY(yMin); + topLeft.setY(topLeft.y() - yMin); } - if (labs(iYMin) < STICKY_THRESHOLD) { - _moveOffsetSticky.setY(iYMin); - topLeft.setY(topLeft.y() - iYMin); - } - r.moveTopLeft(topLeft); + r.moveTopLeft(topLeft); if (!_geom.contains(r, true)) { slideInto(_geom, r); --- trunk/extragear/graphics/kst/src/libkstapp/kstviewobject.cpp #544671:544672 @@ -1470,6 +1470,45 @@ } +void KstViewObject::snapToBorders(int *xMin, int *yMin, const KstViewObjectList &selectionList, const KstViewObjectPtr &pressTarget, const QRect &r) const { + for (KstViewObjectList::ConstIterator i = _children.begin(); i != _children.end(); ++i) { + (*i)->snapToBorders(xMin, yMin, selectionList, pressTarget, r); + + if (selectionList.find(*i) == selectionList.end() && pressTarget != *i) { + const QRect rect((*i)->geometry()); + + int overlapLo = r.top() > rect.top() ? r.top() : rect.top(); + int overlapHi = r.bottom() < rect.bottom() ? r.bottom() : rect.bottom(); + if (overlapHi - overlapLo > 0) { + if (labs(r.left() - rect.left()) < labs(*xMin)) { + *xMin = r.left() - rect.left(); + } else if (labs(r.left() - rect.right()) < labs(*xMin)) { + *xMin = r.left() - rect.right(); + } else if (labs(r.right() - rect.left()) < labs(*xMin)) { + *xMin = r.right() - rect.left(); + } else if (labs(r.right() - rect.right()) < labs(*xMin)) { + *xMin = r.right() - rect.right(); + } + } + + overlapLo = r.left() > rect.left() ? r.left() : rect.left(); + overlapHi = r.right() < rect.right() ? r.right() : rect.right(); + if (overlapHi - overlapLo > 0) { + if (labs(r.top() - rect.top()) < labs(*yMin)) { + *yMin = r.top() - rect.top(); + } else if (labs(r.top() - rect.bottom()) < labs(*yMin)) { + *yMin = r.top() - rect.bottom(); + } else if (labs(r.bottom() - rect.top()) < labs(*yMin)) { + *yMin = r.bottom() - rect.top(); + } else if (labs(r.bottom() - rect.bottom()) < labs(*yMin)) { + *yMin = r.bottom() - rect.bottom(); + } + } + } + } +} + + bool KstViewObject::isSelected() const { return _selected; } --- trunk/extragear/graphics/kst/src/libkstapp/kstviewobject.h #544671:544672 @@ -163,6 +163,8 @@ virtual void updateSelection(const QRect& region); bool isContainer() const; + virtual void snapToBorders(int *xMin, int *yMin, const KstViewObjectList &selectionList, const KstViewObjectPtr &pressTarget, const QRect &r) const; + KstViewObjectPtr parent() const; void recursively(void (KstViewObject::*)(), bool self = false);
Still not accepted.
SVN commit 549222 by arwalker: BUG:127536,128033 Correctly handle snapping to child objects when moving and resizing. M +92 -79 ksttoplevelview.cpp M +2 -0 ksttoplevelview.h