Bug 127524

Created attachment 16149 [details]
Proposed patch.

rewrote KstTopLevelView::pressMoveLayoutModeEndPoint to be more straightforward
 and removed some constants (needed to change the algorithm for determining the
best place to end the line when the maintain aspect modifier is enabled to do
this, but the new method is still intuitive). it works well in practice-
although there may be some problems that I've missed.

because the QRect defining a line can't have width or height 0 in the current
implementation, lines will still draw a bit funnily if you try to make them
vertical or horizontal. does anyone have an idea for how to solve this?

removed some code in KstTopLevelView::releasePressLayoutModeEndPoint- it should
not be necessary as long as _prevBand.normalize() isn't called between the last
mouse movement and the mouse release. tell me if there's a chance of this
happening and I'll put it back in.

Duncan.

Patch submitted.

SVN commit 543826 by dhanson:

CCBUG:127524 mostly fixed.

 M  +44 -67    ksttoplevelview.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/ksttoplevelview.cpp #543825:543826
@@ -799,50 +799,54 @@
   pos.setY(QMAX(pos.y(), geometry().top()));
 
   if (KstViewLinePtr line = kst_cast<KstViewLine>(_pressTarget)) {
-    const QRect old(_prevBand);
-    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 {
-      if (line->to().y() < line->from().y()) {
-        aspect = -1.0E300;
-      } else {
-        aspect = 1.0E300;  
-      }
-    }
-    QPoint fromPoint, toPoint;
+    QPoint movePoint, anchorPoint;
+    QPoint *fromPoint, *toPoint;
+
     if (_pressDirection & UP) {
       // UP means we are on the start endpoint
-      toPoint = line->to();
-      if (maintainAspect) {
-        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;
-      }
-    } else if (_pressDirection & DOWN) {
+      movePoint = line->from();
+      anchorPoint = line->to();
+      fromPoint = &movePoint;
+      toPoint = &anchorPoint;
+    } else { // (_pressDirection & DOWN)
       // DOWN means we are on the end endpoint
-      fromPoint = line->from();
-      if (maintainAspect) {
-        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());
-        }
+      movePoint = line->to();
+      anchorPoint = line->from();
+      fromPoint = &anchorPoint;
+      toPoint = &movePoint;
+    }
+
+    if (maintainAspect) {
+      if (fromPoint->x() == toPoint->x()) {
+        // FIXME: implement. should not be necessary right now (because of the way lines are constructed).
+      } else if (fromPoint->y() == toPoint->y()) {
+        // FIXME: implement. should not be necessary right now (because of the way lines are constructed).
       } else {
-        toPoint = pos;
+        double slope = double(toPoint->y() - fromPoint->y()) / double(toPoint->x() - fromPoint->x());
+
+        double newxpos, newypos;
+
+        newxpos = (((double)pos.y()) + slope*((double)anchorPoint.x()) + ((double)pos.x())/slope -((double)anchorPoint.y())) / (slope + 1.0/slope); //we want the tip of our new line to be as close as possible to the original line (while still maintaining aspect). 
+
+        newxpos = QMIN(newxpos, geometry().right()); //ensure that our x is inside the tlv.
+        newxpos = QMAX(newxpos, geometry().left()); // ""
+        newypos = slope*(newxpos - ((double)anchorPoint.x())) + ((double)anchorPoint.y()); //consistency w/ x.
+
+        newypos = QMIN(newypos, geometry().bottom()); //ensure that our y is inside the tlv.
+        newypos = QMAX(newypos, geometry().top()); // ""
+        newxpos = ((double)anchorPoint.x()) + (newypos - ((double)anchorPoint.y()))/slope; // x will still be inside the tlv because we have just moved newypos closer to anchorPoint.y(), which will send newxpos closer to anchorPoint.x(), ie. in the direction further 'into' the tlv.
+
+        movePoint.setX((int)newxpos);
+        movePoint.setY((int)newypos);
       }
     } else {
-      abort();
+      movePoint = pos; // already enforced pos inside tlv.
     }
 
-    _prevBand.setTopLeft(fromPoint);
-    _prevBand.setBottomRight(toPoint);
+    const QRect old(_prevBand);
+    
+    _prevBand.setTopLeft(*fromPoint);
+    _prevBand.setBottomRight(*toPoint);
 
     if (old != _prevBand) {
       KstPainter p;
@@ -999,40 +1003,13 @@
 
 void KstTopLevelView::releasePressLayoutModeEndPoint(const QPoint& pos, bool shift) {
   Q_UNUSED(shift)
+  Q_UNUSED(pos)
 
   if (KstViewLinePtr line = kst_cast<KstViewLine>(_pressTarget)) {
     if (_prevBand.left() != -1 && _prevBand.top() != -1) {
-      if (_pressDirection & UP) {
-        // UP means we are on the start endpoint
-        const QPoint toPoint(line->to());
-        QRect band(pos, toPoint);
-        band = band.normalize().intersect(geometry());
-        if (toPoint == band.topLeft()) {
-          line->setFrom(band.bottomRight());
-        } else if (toPoint == band.bottomLeft()) {
-          line->setFrom(band.topRight());
-        } else if (toPoint == band.topRight()) {
-          line->setFrom(band.bottomLeft());
-        } else {
-          line->setFrom(band.topLeft());
-        }
-      } else if (_pressDirection & DOWN) {
-        // DOWN means we are on the end endpoint
-        const QPoint fromPoint(line->from());
-        QRect band(fromPoint, pos);
-        band = band.normalize().intersect(geometry());
-        if (fromPoint == band.topLeft()) {
-          line->setTo(band.bottomRight());
-        } else if (fromPoint == band.bottomLeft()) {
-          line->setTo(band.topRight());
-        } else if (fromPoint == band.topRight()) {
-          line->setTo(band.bottomLeft());
-        } else {
-          line->setTo(band.topLeft());
-        }
-      } else {
-        abort();
-      }
+      line->setFrom(_prevBand.topLeft());
+      line->setTo(_prevBand.bottomRight());
+
       _onGrid = false;
 
       // reparent

*** Bug has been marked as fixed ***.

Sorry but the work on this feature had to be moved to a branch.  It's too 
unstable right now and needs some review I think.  You can find the branch in 
branches/work/kst/viewaspect.

On Monday 22 May 2006 17:20, Duncan Hanson wrote:
> SVN commit 543826 by dhanson:
>
> CCBUG:127524 mostly fixed.
>
>  M  +44 -67    ksttoplevelview.cpp
>
>
> --- trunk/extragear/graphics/kst/src/libkstapp/ksttoplevelview.cpp
> #543825:543826 @@ -799,50 +799,54 @@
>    pos.setY(QMAX(pos.y(), geometry().top()));
>
>    if (KstViewLinePtr line = kst_cast<KstViewLine>(_pressTarget)) {
> -    const QRect old(_prevBand);
> -    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 {
> -      if (line->to().y() < line->from().y()) {
> -        aspect = -1.0E300;
> -      } else {
> -        aspect = 1.0E300;
> -      }
> -    }
> -    QPoint fromPoint, toPoint;
> +    QPoint movePoint, anchorPoint;
> +    QPoint *fromPoint, *toPoint;
> +
>      if (_pressDirection & UP) {
>        // UP means we are on the start endpoint
> -      toPoint = line->to();
> -      if (maintainAspect) {
> -        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;
> -      }
> -    } else if (_pressDirection & DOWN) {
> +      movePoint = line->from();
> +      anchorPoint = line->to();
> +      fromPoint = &movePoint;
> +      toPoint = &anchorPoint;
> +    } else { // (_pressDirection & DOWN)
>        // DOWN means we are on the end endpoint
> -      fromPoint = line->from();
> -      if (maintainAspect) {
> -        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()); -        }
> +      movePoint = line->to();
> +      anchorPoint = line->from();
> +      fromPoint = &anchorPoint;
> +      toPoint = &movePoint;
> +    }
> +
> +    if (maintainAspect) {
> +      if (fromPoint->x() == toPoint->x()) {
> +        // FIXME: implement. should not be necessary right now (because of
> the way lines are constructed). +      } else if (fromPoint->y() ==
> toPoint->y()) {
> +        // FIXME: implement. should not be necessary right now (because of
> the way lines are constructed). } else {
> -        toPoint = pos;
> +        double slope = double(toPoint->y() - fromPoint->y()) /
> double(toPoint->x() - fromPoint->x()); +
> +        double newxpos, newypos;
> +
> +        newxpos = (((double)pos.y()) + slope*((double)anchorPoint.x()) +
> ((double)pos.x())/slope -((double)anchorPoint.y())) / (slope + 1.0/slope);
> //we want the tip of our new line to be as close as possible to the
> original line (while still maintaining aspect). +
> +        newxpos = QMIN(newxpos, geometry().right()); //ensure that our x
> is inside the tlv. +        newxpos = QMAX(newxpos, geometry().left()); //
> ""
> +        newypos = slope*(newxpos - ((double)anchorPoint.x())) +
> ((double)anchorPoint.y()); //consistency w/ x. +
> +        newypos = QMIN(newypos, geometry().bottom()); //ensure that our y
> is inside the tlv. +        newypos = QMAX(newypos, geometry().top()); //
> ""
> +        newxpos = ((double)anchorPoint.x()) + (newypos -
> ((double)anchorPoint.y()))/slope; // x will still be inside the tlv because
> we have just moved newypos closer to anchorPoint.y(), which will send
> newxpos closer to anchorPoint.x(), ie. in the direction further 'into' the
> tlv. +
> +        movePoint.setX((int)newxpos);
> +        movePoint.setY((int)newypos);
>        }
>      } else {
> -      abort();
> +      movePoint = pos; // already enforced pos inside tlv.
>      }
>
> -    _prevBand.setTopLeft(fromPoint);
> -    _prevBand.setBottomRight(toPoint);
> +    const QRect old(_prevBand);
> +
> +    _prevBand.setTopLeft(*fromPoint);
> +    _prevBand.setBottomRight(*toPoint);
>
>      if (old != _prevBand) {
>        KstPainter p;
> @@ -999,40 +1003,13 @@
>
>  void KstTopLevelView::releasePressLayoutModeEndPoint(const QPoint& pos,
> bool shift) { Q_UNUSED(shift)
> +  Q_UNUSED(pos)
>
>    if (KstViewLinePtr line = kst_cast<KstViewLine>(_pressTarget)) {
>      if (_prevBand.left() != -1 && _prevBand.top() != -1) {
> -      if (_pressDirection & UP) {
> -        // UP means we are on the start endpoint
> -        const QPoint toPoint(line->to());
> -        QRect band(pos, toPoint);
> -        band = band.normalize().intersect(geometry());
> -        if (toPoint == band.topLeft()) {
> -          line->setFrom(band.bottomRight());
> -        } else if (toPoint == band.bottomLeft()) {
> -          line->setFrom(band.topRight());
> -        } else if (toPoint == band.topRight()) {
> -          line->setFrom(band.bottomLeft());
> -        } else {
> -          line->setFrom(band.topLeft());
> -        }
> -      } else if (_pressDirection & DOWN) {
> -        // DOWN means we are on the end endpoint
> -        const QPoint fromPoint(line->from());
> -        QRect band(fromPoint, pos);
> -        band = band.normalize().intersect(geometry());
> -        if (fromPoint == band.topLeft()) {
> -          line->setTo(band.bottomRight());
> -        } else if (fromPoint == band.bottomLeft()) {
> -          line->setTo(band.topRight());
> -        } else if (fromPoint == band.topRight()) {
> -          line->setTo(band.bottomLeft());
> -        } else {
> -          line->setTo(band.topLeft());
> -        }
> -      } else {
> -        abort();
> -      }
> +      line->setFrom(_prevBand.topLeft());
> +      line->setTo(_prevBand.bottomRight());
> +
>        _onGrid = false;
>
>        // reparent
> _______________________________________________
> Kst mailing list
> Kst@kde.org
> https://mail.kde.org/mailman/listinfo/kst