Version: HEAD (using KDE KDE 3.5.0) Installed from: Compiled From Sources OS: Linux PROBLEM: Moving one end of a line or arrow object outside of its window causes a crash. STEPS TO REPRODUCE: Start Kst In the default window create a line contained within the default Switch to layout mode Click on the line to select it Move the cursor over one of the end-points of the line Drag the end-point so that it moves outside of the window Release the mouse button RESULTS: Crash EXPECTED RESULTS: Cannot drag end-point outside of the window
Stack dump: Using host libthread_db library "/lib/tls/libthread_db.so.1". [Thread debugging using libthread_db enabled] [New Thread -187439392 (LWP 30535)] [New Thread -192955472 (LWP 30536)] [Thread debugging using libthread_db enabled] [New Thread -187439392 (LWP 30535)] [New Thread -192955472 (LWP 30536)] [Thread debugging using libthread_db enabled] [New Thread -187439392 (LWP 30535)] [New Thread -192955472 (LWP 30536)] [KCrash handler] #4 0xf6fe97a2 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2 #5 0xf54cc955 in raise () from /lib/tls/libc.so.6 #6 0xf54ce319 in abort () from /lib/tls/libc.so.6 #7 0xf54c5f41 in __assert_fail () from /lib/tls/libc.so.6 #8 0xf6e7617c in KstViewObject::updateFromAspect (this=0x8ffec40) at qrect.h:188 #9 0xf6e7bd9d in KstViewObject::resize (this=0x8ffec40, size=@0xfef9f478) at kstviewobject.cpp:566 #10 0xf6e67b35 in KstViewLine::updateOrientation (this=0x8ffec40) at qsize.h:120 #11 0xf6e67ca6 in KstViewLine::setFrom (this=0x8ffec40, from=@0x6) at kstviewline.cpp:156 #12 0xf6e89a96 in KstTopLevelView::releasePressLayoutModeEndPoint ( this=0x8ffeac0, pos=@0xfef9facc, shift=false) at kstsharedptr.h:136 #13 0xf6e8d068 in KstTopLevelView::releasePressLayoutMode (this=0x8ffeac0, pos=@0xfef9facc, shift=false) at ksttoplevelview.cpp:875 #14 0xf6e8d274 in KstTopLevelView::releasePress (this=0x8ffeac0, pos=@0xfef9facc, shift=false) at ksttoplevelview.cpp:1036 #15 0xf6e71371 in KstViewWidget::mouseReleaseEvent (this=0x8e1f618, e=0xfef9fac0) at kstsharedptr.h:136 #16 0xf5bf95c4 in QWidget::event () from /usr/qt/lib/libqt-mt.so.3 #17 0xf5b59a91 in QApplication::internalNotify () from /usr/qt/lib/libqt-mt.so.3 #18 0xf5b59d5f in QApplication::notify () from /usr/qt/lib/libqt-mt.so.3 #19 0xf62de4c8 in KApplication::notify () from /usr/lib/libkdecore.so.4 #20 0xf5af3195 in QETWidget::translateMouseEvent () from /usr/qt/lib/libqt-mt.so.3 #21 0xf5af1402 in QApplication::x11ProcessEvent () from /usr/qt/lib/libqt-mt.so.3 #22 0xf5b04456 in QEventLoop::processEvents () from /usr/qt/lib/libqt-mt.so.3 #23 0xf5b6f3a3 in QEventLoop::enterLoop () from /usr/qt/lib/libqt-mt.so.3 #24 0xf5b6f300 in QEventLoop::exec () from /usr/qt/lib/libqt-mt.so.3 #25 0xf5b58c90 in QApplication::exec () from /usr/qt/lib/libqt-mt.so.3 #26 0x08054c9e in main (argc=1, argv=0xfefa0be4) at main.cpp:814
Propoased patch which keeps the line/arrow within the parent, thus avoiding the assert: Index: ksttoplevelview.cpp =================================================================== --- ksttoplevelview.cpp (revision 504684) +++ ksttoplevelview.cpp (working copy) @@ -37,6 +37,7 @@ #include "kstaccessibility.h" #include "kstdoc.h" #include "kstgfxmousehandler.h" +#include "kstmath.h" #include "kstplotgroup.h" #include "kstsettings.h" #include "ksttimers.h" @@ -780,12 +781,80 @@ } +QPoint KstTopLevelView::handlePoint(bool shift, const QPoint& ptFixed, const QPoint& pos, double aspect) { + QPoint ptMoved = pos; + + if (shift) { + double absAspect = fabs(aspect); + if (absAspect < 500.0 && (double(abs((pos.y() - ptFixed.y())/(pos.x() - ptFixed.x()))) < aspect || + absAspect < 0.1)) { + ptMoved = QPoint(pos.x(), ptFixed.y() + int(aspect*(pos.x() - ptFixed.x()))); + } else { + ptMoved = QPoint(ptFixed.x() + int((pos.y() - ptFixed.y())/aspect), pos.y()); + } + } + + // + // ensure that the point we just moved is contained by its parent... + // + QRect parentRect = _pressTarget->_parent->_geom; + if (parentRect.contains(ptFixed) && !parentRect.contains(ptMoved)) { + double gradient; + bool found = false; + int x; + int y; + + if (!found && ptMoved.x() < parentRect.left()) { + gradient = double(ptMoved.y() - ptFixed.y())/double(ptMoved.x() - ptFixed.x()); + x = parentRect.left(); + y = ptFixed.y() + d2i(double(x - ptFixed.x()) * gradient); + if (y >= parentRect.top() && y <= parentRect.bottom()) { + found = true; + } + } + if (!found && ptMoved.x() > parentRect.right()) { + gradient = double(ptMoved.y() - ptFixed.y())/double(ptMoved.x() - ptFixed.x()); + x = parentRect.right(); + y = ptFixed.y() + d2i(double(x - ptFixed.x()) * gradient); + if (y >= parentRect.top() && y <= parentRect.bottom()) { + found = true; + } + } + if (!found && ptMoved.y() < parentRect.top()) { + gradient = double(ptMoved.x() - ptFixed.x())/double(ptMoved.y() - ptFixed.y()); + y = parentRect.top(); + x = ptFixed.x() + d2i(double(y - ptFixed.y()) * gradient); + if (x >= parentRect.left() && x <= parentRect.right()) { + found = true; + } + } + if (!found && ptMoved.y() > parentRect.bottom()) { + gradient = double(ptMoved.x() - ptFixed.x())/double(ptMoved.y() - ptFixed.y()); + y = parentRect.bottom(); + x = ptFixed.x() + d2i(double(y - ptFixed.y()) * gradient); + if (x >= parentRect.left() && x <= parentRect.right()) { + found = true; + } + } + + if (found) { + ptMoved.setX(x); + ptMoved.setY(y); + } + } + + return ptMoved; +} + + void KstTopLevelView::pressMoveLayoutModeEndPoint(const QPoint& pos, bool shift) { // FIXME: remove this!! Should not know about any specific type // for now we only know how to deal with lines if (KstViewLinePtr line = kst_cast<KstViewLine>(_pressTarget)) { const QRect old(_prevBand); + QPoint ptFixed; double aspect; + if (line->to().x() != line->from().x()) { aspect = double(line->to().y() - line->from().y()) / double(line->to().x() - line->from().x()); } else { @@ -795,40 +864,22 @@ aspect = 1.0E300; } } - QPoint fromPoint, toPoint; + if (_pressDirection & UP) { - // UP means we are on the start endpoint - toPoint = line->to(); - if (shift) { - double absAspect = fabs(aspect); - if (absAspect < 500 && (double(abs((pos.y() - toPoint.y())/(pos.x() - toPoint.x()))) < aspect || absAspect < 0.1)) { - fromPoint = QPoint(pos.x(), toPoint.y() + int(aspect*(pos.x() - toPoint.x()))); - } else { - fromPoint = QPoint(toPoint.x() + int((pos.y() - toPoint.y())/aspect), pos.y()); - } - } else { - fromPoint = pos; - } + // UP means we are on the "from" point + ptFixed = line->to(); + _prevBand.setTopLeft(handlePoint(shift, ptFixed, pos, aspect)); + _prevBand.setBottomRight(ptFixed); } else if (_pressDirection & DOWN) { - // DOWN means we are on the end endpoint - fromPoint = line->from(); - if (shift) { - double absAspect = fabs(aspect); - if (absAspect < 500 && (double(abs((pos.y() - toPoint.y())/(pos.x() - toPoint.x()))) < aspect || absAspect < 0.1)) { - toPoint = QPoint(pos.x(), fromPoint.y() + int(aspect*(pos.x() - fromPoint.x()))); - } else { - toPoint = QPoint(fromPoint.x() + int((pos.y() - fromPoint.y())/aspect), pos.y()); - } - } else { - toPoint = pos; - } + // DOWN means we are on the "to" point + ptFixed = line->from(); + _prevBand.setTopLeft(ptFixed); + _prevBand.setBottomRight(handlePoint(shift, ptFixed, pos, aspect)); } else { - abort(); + assert(false); + return; } - _prevBand.setTopLeft(fromPoint); - _prevBand.setBottomRight(toPoint); - if (old != _prevBand) { KstPainter p; p.begin(_w); Index: ksttoplevelview.h =================================================================== --- ksttoplevelview.h (revision 504664) +++ ksttoplevelview.h (working copy) @@ -98,6 +98,7 @@ void pressMove(const QPoint& pos, bool shift = false); void pressMoveLayoutMode(const QPoint& pos, bool shift = false); // helpers for pressMoveLayoutMode + QPoint handlePoint(bool shift, const QPoint& ptFixed, const QPoint& pos, double aspect); void pressMoveLayoutModeMove(const QPoint& pos, bool shift = false); void pressMoveLayoutModeResize(const QPoint& pos, bool shift = false); void pressMoveLayoutModeSelect(const QPoint& pos, bool shift = false);
> + QPoint handlePoint(bool shift, const QPoint& ptFixed, const QPoint& > pos, double aspect); void pressMoveLayoutModeMove(const QPoint& pos, bool The meaning of this function is entirely unclear. > void KstTopLevelView::pressMoveLayoutModeEndPoint(const QPoint& pos, bool And I think all of the fix should go in here. As far as I can tell, all we need to do is chop the line into the geometry of the view. This function can already do all the important calculations internally if it just makes use of the QPoint argument. Unfortunately Qt3's QRect doesn't ::intersect() well when it isn't normalized, so this patch is more complicated than it should be. One other bug I noticed is that the viewline object doesn't track moves properly, so it sometimes gets desynchronized. This patch also fixes that problem. Created an attachment (id=14494) linemoving.patch
Created attachment 14499 [details] Proposed patch
> One other bug I noticed is that the viewline object doesn't track moves > properly, so it sometimes gets desynchronized. This patch also fixes that > problem. George, could you provide more details on this. How do I reproduce it?
On Thursday 02 February 2006 13:22, Andrew Walker wrote: > George, could you provide more details on this. How do I reproduce it? Move a line and check the from/to values from KstScript, comparing to the bounding box. They don't stay synchronized.
A minor point but it looks as if George's patch simply "rescues" a line that extends beyond its parent by considering the x and y values independently. This has the effect of not drawing a line that extends partway between the fixed point and the mouse position, but instead draws a line between the fixed point and some point on the edge of the parent. The effect is less intuitive but simpler to calculate.
> Move a line and check the from/to values from KstScript, comparing to the > bounding box. They don't stay synchronized. This sounds like the same thing you describe in 121068. Is that the case?
I think that Andrew's patch behavior is the most consistent. -Like all other view objects, it doesn't reparent. -Like all other view objects, it doesn't let the line leave the parent. -The UI feedback does not imply that it can leave the parent. -The line lands exactly where the feedback told you it would be. It is OK to commit in my mind, but we can wait 'till tomorrow for George to comment (or he can tweak and commit it).
Restrict lines and arrows to their parent - consistent with other view objects.
Created attachment 14533 [details] Tweak to George's patch
We still have dueling patches. This is a tweak to George's patch which: -Keeps the band line consistent with where the line will actually end up when the pointer is outside the tlv -re-parents after the move. Upon further consideration, I really like allowing the line to move outside the current parent - which is what this does.
If we're going to allow the line outside of the parent on a resize then we should also do the same for all the other view objects.
On Friday 03 February 2006 16:34, Barth Netterfield wrote: > I agree, but not for 1.2.0. I tried to deal with this for a bit, but it > will touch too much code at this late time. The reparenting was the > problem. But the line stuff looks pretty solid now. I disagree. See upcoming email.
This bug has been fixed, but I BUG:ed the wrong bug number on commit.