Bug 121068

Summary: Arrow view objects not always clipped correctly
Product: [Applications] kst Reporter: Andrew Walker <arwalker>
Component: generalAssignee: kst
Status: RESOLVED FIXED    
Severity: normal CC: nicolas.brisset
Priority: HI    
Version: 1.x   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: Propsosed patch
Andrew's 'draw outside of geometry' patch against pre-applied svn

Description Andrew Walker 2006-01-30 22:39:46 UTC
Version:           HEAD (using KDE KDE 3.5.0)
Installed from:    Compiled From Sources
OS:                Linux

PROBLEM:
The arrow view object is not clipped correctly, particularly when positioned on or near the vertical or horizontal.

STEPS TO REPRODUCE:
Start kst
In the default window draw several windows oriented mostly on the vertical or horizontal
Draw one arrow at a 45 degree angle for reference.


RESULTS:
The horizontal and vetical arrows have clipped width and arrow heads

EXPECTED RESULTS:
The arrow view object should be clipped correctly
Comment 1 Andrew Walker 2006-02-01 02:29:04 UTC
This also applies to line objects.
Comment 2 Andrew Walker 2006-02-01 02:29:35 UTC
Proposed patch:

Index: kstviewline.cpp
===================================================================
--- kstviewline.cpp     (revision 504422)
+++ kstviewline.cpp     (working copy)
@@ -22,6 +22,7 @@

 #include <klocale.h>

+#include <qbitmap.h>
 #include <qmetaobject.h>
 #include <qpainter.h>
 #include <qvariant.h>
@@ -84,16 +85,37 @@
 }


+QRegion KstViewLine::clipRegion() {
+  if (_clipMask.isNull()) {
+    int w = width();
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1);
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
+      KstPainter p;
+      p.setMakingMask(true);
+      p.begin(&bm);
+      p.setViewXForm(true);
+      paintSelf(p, QRegion());
+      p.flush();
+      p.end();
+      _clipMask = QRegion(bm);
+    } else {
+      _clipMask = QRegion(); // only invalidate our own variable
+    }
+  }
+
+  return _clipMask;
+}
+
 void KstViewLine::paintSelf(KstPainter& p, const QRegion& bounds) {
   p.save();
   if (p.type() != KstPainter::P_PRINT && p.type() != KstPainter::P_EXPORT) {
     if (p.makingMask()) {
       p.setRasterOp(Qt::SetROP);
-      KstViewObject::paintSelf(p, geometry());
     } else {
       const QRegion clip(clipRegion());
       KstViewObject::paintSelf(p, bounds - clip);
-      p.setClipRegion(bounds & clip);
+      p.setClipRegion(clip);
     }
   }

@@ -104,30 +126,14 @@
   p.setPen(pen);

   const QRect geom(geometry());
-  int u = 0, v = 0;
-
-  // Adjust for large widths.  We don't want the line clipped because it goes
-  // out of the bounding box.
-  if (_width > 1 && geom.height() > 0) {
-    double theta = atan(geom.width()/geom.height());
-    int w = _width;
-    if (theta >= 0 && theta <= M_PI/4) {
-      u = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-      v = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-    } else {
-      u = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-      v = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-    }
-  }
-
   switch (_orientation) {
     case UpLeft:
     case DownRight:
-      p.drawLine(geom.bottomRight() + QPoint(-u, -v), geom.topLeft() + QPoint(u, v));
+      p.drawLine(geom.bottomRight(), geom.topLeft());
       break;
     case UpRight:
     case DownLeft:
-      p.drawLine(geom.bottomLeft() + QPoint(u, -v), geom.topRight() + QPoint(-u, v));
+      p.drawLine(geom.bottomLeft(), geom.topRight());
       break;
   }
   p.restore();
@@ -238,21 +244,21 @@
     if (_from.y() < _to.y()) {
       _orientation = DownRight;
       move(_from);
-      resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _to.y()- _from.y() + 1)));
+      resize(QSize(_to.x() - _from.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpRight;
       move(QPoint(_from.x(), _to.y()));
-      resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      resize(QSize(_to.x() - _from.x() + 1, _from.y() - _to.y() + 1));
     }
   } else {
     if (_from.y() < _to.y()) {
       _orientation = DownLeft;
       move(QPoint(_to.x(), _from.y()));
-      resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _to.y()- _from.y() + 1)));
+      resize(QSize(_from.x() - _to.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpLeft;
       move(_to);
-      resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      resize(QSize(_from.x() - _to.x() + 1, _from.y() - _to.y() + 1));
     }
   }
 }
Index: kstviewline.h
===================================================================
--- kstviewline.h       (revision 504422)
+++ kstviewline.h       (working copy)
@@ -51,7 +51,8 @@
     // just calls KstViewObject functions - Q_PROPERTY doesn't work in KstViewObject?
     void setForegroundColor(const QColor& color);
     QColor foregroundColor() const;
-
+    virtual QRegion clipRegion();
+
     virtual void setCapStyle(Qt::PenCapStyle style);
     virtual Qt::PenCapStyle capStyle() const;
     virtual void setPenStyle(Qt::PenStyle style);
Index: kstviewarrow.cpp
===================================================================
--- kstviewarrow.cpp    (revision 504422)
+++ kstviewarrow.cpp    (working copy)
@@ -81,28 +81,26 @@

 QRegion KstViewArrow::clipRegion() {
   if (_clipMask.isNull()) {
+    int w = int(ceil(sqrt(3.0) * _toArrowScaling * 2.0 * double(width())));
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1 );
     _myClipMask = QRegion();
-    QBitmap bm1(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm1.isNull()) {
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
       KstPainter p;
       p.setMakingMask(true);
-      p.begin(&bm1);
+      p.begin(&bm);
       p.setViewXForm(true);
+
       KstViewLine::paintSelf(p, QRegion());
       p.flush();
-      p.end();
-      _clipMask = QRegion(bm1);
-    }
-    QBitmap bm2(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm2.isNull()) {
-      KstPainter p;
-      p.setMakingMask(true);
-      p.begin(&bm2);
-      p.setViewXForm(true);
-      paintSelf(p, QRegion());
+      _clipMask = QRegion(bm);
+
+      p.eraseRect(rect);
+      paintSelf(p, QRegion());
       p.flush();
+      _myClipMask = QRegion(bm);
+
       p.end();
-      _myClipMask = QRegion(bm2);
     }
   }

@@ -121,7 +119,7 @@
       p.setClipRegion(bounds & clip);
     }
   } else {
-      KstViewLine::paintSelf(p, bounds);
+    KstViewLine::paintSelf(p, bounds);
   }

   if (hasArrow()) {
Comment 3 Andrew Walker 2006-02-01 02:39:10 UTC
A better patch that allows for possible differences in the scaling of the two arrow ends:

Index: kstviewline.cpp
===================================================================
--- kstviewline.cpp     (revision 504422)
+++ kstviewline.cpp     (working copy)
@@ -22,6 +22,7 @@

 #include <klocale.h>

+#include <qbitmap.h>
 #include <qmetaobject.h>
 #include <qpainter.h>
 #include <qvariant.h>
@@ -84,16 +85,37 @@
 }


+QRegion KstViewLine::clipRegion() {
+  if (_clipMask.isNull()) {
+    int w = width();
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1);
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
+      KstPainter p;
+      p.setMakingMask(true);
+      p.begin(&bm);
+      p.setViewXForm(true);
+      paintSelf(p, QRegion());
+      p.flush();
+      p.end();
+      _clipMask = QRegion(bm);
+    } else {
+      _clipMask = QRegion(); // only invalidate our own variable
+    }
+  }
+
+  return _clipMask;
+}
+
 void KstViewLine::paintSelf(KstPainter& p, const QRegion& bounds) {
   p.save();
   if (p.type() != KstPainter::P_PRINT && p.type() != KstPainter::P_EXPORT) {
     if (p.makingMask()) {
       p.setRasterOp(Qt::SetROP);
-      KstViewObject::paintSelf(p, geometry());
     } else {
       const QRegion clip(clipRegion());
       KstViewObject::paintSelf(p, bounds - clip);
-      p.setClipRegion(bounds & clip);
+      p.setClipRegion(clip);
     }
   }

@@ -104,30 +126,14 @@
   p.setPen(pen);

   const QRect geom(geometry());
-  int u = 0, v = 0;
-
-  // Adjust for large widths.  We don't want the line clipped because it goes
-  // out of the bounding box.
-  if (_width > 1 && geom.height() > 0) {
-    double theta = atan(geom.width()/geom.height());
-    int w = _width;
-    if (theta >= 0 && theta <= M_PI/4) {
-      u = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-      v = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-    } else {
-      u = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-      v = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-    }
-  }
-
   switch (_orientation) {
     case UpLeft:
     case DownRight:
-      p.drawLine(geom.bottomRight() + QPoint(-u, -v), geom.topLeft() + QPoint(u, v));
+      p.drawLine(geom.bottomRight(), geom.topLeft());
       break;
     case UpRight:
     case DownLeft:
-      p.drawLine(geom.bottomLeft() + QPoint(u, -v), geom.topRight() + QPoint(-u, v));
+      p.drawLine(geom.bottomLeft(), geom.topRight());
       break;
   }
   p.restore();
@@ -238,21 +244,21 @@
     if (_from.y() < _to.y()) {
       _orientation = DownRight;
       move(_from);
-      resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _to.y()- _from.y() + 1)));
+      resize(QSize(_to.x() - _from.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpRight;
       move(QPoint(_from.x(), _to.y()));
-      resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      resize(QSize(_to.x() - _from.x() + 1, _from.y() - _to.y() + 1));
     }
   } else {
     if (_from.y() < _to.y()) {
       _orientation = DownLeft;
       move(QPoint(_to.x(), _from.y()));
-      resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _to.y()- _from.y() + 1)));
+      resize(QSize(_from.x() - _to.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpLeft;
       move(_to);
-      resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      resize(QSize(_from.x() - _to.x() + 1, _from.y() - _to.y() + 1));
     }
   }
 }
Index: kstviewline.h
===================================================================
--- kstviewline.h       (revision 504422)
+++ kstviewline.h       (working copy)
@@ -51,7 +51,8 @@
     // just calls KstViewObject functions - Q_PROPERTY doesn't work in KstViewObject?
     void setForegroundColor(const QColor& color);
     QColor foregroundColor() const;
-
+    virtual QRegion clipRegion();
+
     virtual void setCapStyle(Qt::PenCapStyle style);
     virtual Qt::PenCapStyle capStyle() const;
     virtual void setPenStyle(Qt::PenStyle style);
Index: kstviewarrow.cpp
===================================================================
--- kstviewarrow.cpp    (revision 504422)
+++ kstviewarrow.cpp    (working copy)
@@ -58,8 +58,8 @@
 }


-void KstViewArrow::paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w) {
-  double deltax = _toArrowScaling * 2.0 * double(w);
+void KstViewArrow::paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double arrowScaling) {
+  double deltax = arrowScaling * 2.0 * double(w);
   double theta = atan2(double(from.y() - to.y()), double(from.x() - to.x())) -M_PI / 2.0;
   double sina = sin(theta);
   double cosa = cos(theta);
@@ -81,28 +81,31 @@

 QRegion KstViewArrow::clipRegion() {
   if (_clipMask.isNull()) {
+    double arrowScaling = _toArrowScaling;
+    if (_fromArrowScaling > arrowScaling) {
+      arrowScaling = _fromArrowScaling;
+    }
+
+    int w = int(ceil(sqrt(3.0) * arrowScaling * 2.0 * double(width())));
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1 );
     _myClipMask = QRegion();
-    QBitmap bm1(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm1.isNull()) {
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
       KstPainter p;
       p.setMakingMask(true);
-      p.begin(&bm1);
+      p.begin(&bm);
       p.setViewXForm(true);
+
       KstViewLine::paintSelf(p, QRegion());
       p.flush();
-      p.end();
-      _clipMask = QRegion(bm1);
-    }
-    QBitmap bm2(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm2.isNull()) {
-      KstPainter p;
-      p.setMakingMask(true);
-      p.begin(&bm2);
-      p.setViewXForm(true);
-      paintSelf(p, QRegion());
+      _clipMask = QRegion(bm);
+
+      p.eraseRect(rect);
+      paintSelf(p, QRegion());
       p.flush();
+      _myClipMask = QRegion(bm);
+
       p.end();
-      _myClipMask = QRegion(bm2);
     }
   }

@@ -121,7 +124,7 @@
       p.setClipRegion(bounds & clip);
     }
   } else {
-      KstViewLine::paintSelf(p, bounds);
+    KstViewLine::paintSelf(p, bounds);
   }

   if (hasArrow()) {
@@ -135,10 +138,10 @@
     p.setBrush(_foregroundColor);

     if (_hasToArrow) {
-      paintArrow(p, to, from, w);
+      paintArrow(p, to, from, w, _toArrowScaling);
     }
     if (_hasFromArrow) {
-      paintArrow(p, from, to, w);
+      paintArrow(p, from, to, w, _fromArrowScaling);
     }
   }
   p.restore();
Index: kstviewarrow.h
===================================================================
--- kstviewarrow.h      (revision 504422)
+++ kstviewarrow.h      (working copy)
@@ -39,7 +39,7 @@
     QMap<QString, QVariant > widgetHints(const QString& propertyName) const;

     void paintSelf(KstPainter& p, const QRegion& bounds);
-    void paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w);
+    void paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double arrowScaling);
     // true if either end has an arrow
     bool hasArrow() const;
Comment 4 George Staikos 2006-02-01 05:15:29 UTC
On Tuesday 31 January 2006 20:39, Andrew Walker wrote:

>      } else {
>        const QRegion clip(clipRegion());
>        KstViewObject::paintSelf(p, bounds - clip);
> -      p.setClipRegion(bounds & clip);
> +      p.setClipRegion(clip);
>      }


  From the looks of this, it will paint outside the bounds of the object, 
which is invalid.  I haven't tested, but if it does paint outside the bounds, 
it's wrong and needs to be corrected.  I can test it tomorrow.
Comment 5 Andrew Walker 2006-02-01 18:45:45 UTC
Does the following address your concerns George?

Index: kstviewline.cpp
===================================================================
--- kstviewline.cpp     (revision 504657)
+++ kstviewline.cpp     (working copy)
@@ -22,6 +22,7 @@

 #include <klocale.h>

+#include <qbitmap.h>
 #include <qmetaobject.h>
 #include <qpainter.h>
 #include <qvariant.h>
@@ -84,12 +85,34 @@
 }


+QRegion KstViewLine::clipRegion() {
+  if (_clipMask.isNull()) {
+    int w = width();
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1);
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
+      KstPainter p;
+      p.setMakingMask(true);
+      p.begin(&bm);
+      p.setViewXForm(true);
+      p.eraseRect(rect);
+      paintSelf(p, QRegion());
+      p.flush();
+      p.end();
+      _clipMask = QRegion(bm);
+    } else {
+      _clipMask = QRegion(); // only invalidate our own variable
+    }
+  }
+
+  return _clipMask;
+}
+
 void KstViewLine::paintSelf(KstPainter& p, const QRegion& bounds) {
   p.save();
   if (p.type() != KstPainter::P_PRINT && p.type() != KstPainter::P_EXPORT) {
     if (p.makingMask()) {
       p.setRasterOp(Qt::SetROP);
-      KstViewObject::paintSelf(p, geometry());
     } else {
       const QRegion clip(clipRegion());
       KstViewObject::paintSelf(p, bounds - clip);
@@ -104,30 +127,14 @@
   p.setPen(pen);

   const QRect geom(geometry());
-  int u = 0, v = 0;
-
-  // Adjust for large widths.  We don't want the line clipped because it goes
-  // out of the bounding box.
-  if (_width > 1 && geom.height() > 0) {
-    double theta = atan(geom.width()/geom.height());
-    int w = _width;
-    if (theta >= 0 && theta <= M_PI/4) {
-      u = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-      v = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-    } else {
-      u = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-      v = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-    }
-  }
-
   switch (_orientation) {
     case UpLeft:
     case DownRight:
-      p.drawLine(geom.bottomRight() + QPoint(-u, -v), geom.topLeft() + QPoint(u, v));
+      p.drawLine(geom.bottomRight(), geom.topLeft());
       break;
     case UpRight:
     case DownLeft:
-      p.drawLine(geom.bottomLeft() + QPoint(u, -v), geom.topRight() + QPoint(-u, v));
+      p.drawLine(geom.bottomLeft(), geom.topRight());
       break;
   }
   p.restore();
@@ -238,21 +245,21 @@
     if (_from.y() < _to.y()) {
       _orientation = DownRight;
       move(_from);
-      resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _to.y()- _from.y() + 1)));
+      resize(QSize(_to.x() - _from.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpRight;
       move(QPoint(_from.x(), _to.y()));
-      resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      resize(QSize(_to.x() - _from.x() + 1, _from.y() - _to.y() + 1));
     }
   } else {
     if (_from.y() < _to.y()) {
       _orientation = DownLeft;
       move(QPoint(_to.x(), _from.y()));
-      resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _to.y()- _from.y() + 1)));
+      resize(QSize(_from.x() - _to.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpLeft;
       move(_to);
-      resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      resize(QSize(_from.x() - _to.x() + 1, _from.y() - _to.y() + 1));
     }
   }
 }
Index: kstviewline.h
===================================================================
--- kstviewline.h       (revision 504657)
+++ kstviewline.h       (working copy)
@@ -51,7 +51,8 @@
     // just calls KstViewObject functions - Q_PROPERTY doesn't work in KstViewObject?
     void setForegroundColor(const QColor& color);
     QColor foregroundColor() const;
-
+    QRegion clipRegion();
+
     virtual void setCapStyle(Qt::PenCapStyle style);
     virtual Qt::PenCapStyle capStyle() const;
     virtual void setPenStyle(Qt::PenStyle style);
Index: kstviewarrow.cpp
===================================================================
--- kstviewarrow.cpp    (revision 504657)
+++ kstviewarrow.cpp    (working copy)
@@ -58,8 +58,8 @@
 }


-void KstViewArrow::paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w) {
-  double deltax = _toArrowScaling * 2.0 * double(w);
+void KstViewArrow::paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double arrowScaling) {
+  double deltax = arrowScaling * 2.0 * double(w);
   double theta = atan2(double(from.y() - to.y()), double(from.x() - to.x())) -M_PI / 2.0;
   double sina = sin(theta);
   double cosa = cos(theta);
@@ -81,28 +81,31 @@

 QRegion KstViewArrow::clipRegion() {
   if (_clipMask.isNull()) {
+    double arrowScaling = _toArrowScaling;
+    if (_fromArrowScaling > arrowScaling) {
+      arrowScaling = _fromArrowScaling;
+    }
+
+    int w = int(ceil(sqrt(3.0) * arrowScaling * 2.0 * double(width())));
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1 );
     _myClipMask = QRegion();
-    QBitmap bm1(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm1.isNull()) {
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
       KstPainter p;
       p.setMakingMask(true);
-      p.begin(&bm1);
+      p.begin(&bm);
       p.setViewXForm(true);
+
       KstViewLine::paintSelf(p, QRegion());
       p.flush();
-      p.end();
-      _clipMask = QRegion(bm1);
-    }
-    QBitmap bm2(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm2.isNull()) {
-      KstPainter p;
-      p.setMakingMask(true);
-      p.begin(&bm2);
-      p.setViewXForm(true);
-      paintSelf(p, QRegion());
+      _clipMask = QRegion(bm);
+
+      p.eraseRect(rect);
+      paintSelf(p, QRegion());
       p.flush();
+      _myClipMask = QRegion(bm);
+
       p.end();
-      _myClipMask = QRegion(bm2);
     }
   }

@@ -121,7 +124,7 @@
       p.setClipRegion(bounds & clip);
     }
   } else {
-      KstViewLine::paintSelf(p, bounds);
+    KstViewLine::paintSelf(p, bounds);
   }

   if (hasArrow()) {
@@ -135,10 +138,10 @@
     p.setBrush(_foregroundColor);

     if (_hasToArrow) {
-      paintArrow(p, to, from, w);
+      paintArrow(p, to, from, w, _toArrowScaling);
     }
     if (_hasFromArrow) {
-      paintArrow(p, from, to, w);
+      paintArrow(p, from, to, w, _fromArrowScaling);
     }
   }
   p.restore();
Index: kstviewarrow.h
===================================================================
--- kstviewarrow.h      (revision 504657)
+++ kstviewarrow.h      (working copy)
@@ -39,7 +39,7 @@
     QMap<QString, QVariant > widgetHints(const QString& propertyName) const;

     void paintSelf(KstPainter& p, const QRegion& bounds);
-    void paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w);
+    void paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double arrowScaling);
     // true if either end has an arrow
     bool hasArrow() const;
Comment 6 George Staikos 2006-02-02 00:45:48 UTC
No, the objects are still painting outside their bounds.

Load the testgraphics.js file, set the green line to width=31 and make it 
exactly vertical.  Notice the following situation:

kst> Kst.windows[1].view.children[3].size
(1, 484)
kst> Kst.windows[1].view.children[3].width
31

  According to the first, the line must be 1 pixel wide.  According to the 
second, the line is 31 pixels wide.  Visually it is 31, which means it's 
painting outside its boundaries.  That's why that resize adjustment was in 
there in the first place.
Comment 7 Andrew Walker 2006-02-02 02:28:21 UTC
You seem to be saying the fix is not right because of what js is reporting to you. From the user's point of view what is wrong with the fix?
Comment 8 George Staikos 2006-02-02 12:15:35 UTC
On Wednesday 01 February 2006 20:28, Andrew Walker wrote:
> ------- You seem to be saying the fix is not right because of what js is
> reporting to you. From the user's point of view what is wrong with the fix?


  It is violating preconditions and circumventing the painting work that I 
just finished doing.  The patch is wrong.
Comment 9 Andrew Walker 2006-02-02 19:07:01 UTC
Created attachment 14498 [details]
Propsosed patch
Comment 10 Andrew Walker 2006-02-02 19:14:09 UTC
Sorry but "violating preconditions" and "circumventing the painting work" are rather vague. Also it is unclear to me what size and width are reporting with respect to a line. Is width reporting the line width?

The concept of size of a line is also vague. If this is the rectangle bounding the line then I would expect it to return the same thing irrespective of the line width.
Comment 11 George Staikos 2006-02-02 22:15:20 UTC
On Thursday 02 February 2006 13:14, Andrew Walker wrote:
> ------- Sorry but "violating preconditions" and "circumventing the painting
> work" are rather vague.


  The rules are, a view object paints inside its bounding box and obeys 
clipping.  Anything that does not is incorrect.  Incorrect view objects are 
not permitted in Kst.

> Also it is unclear to me what size and width are 
> reporting with respect to a line. Is width reporting the line width?


  http://kst.kde.org/kstscript/docs/Line.html

> The concept of size of a line is also vague. If this is the rectangle
> bounding the line then I would expect it to return the same thing
> irrespective of the line width.


  Then the tip becomes a [possibly asymmetric] wedge.  The size is the size of 
the object in a Kst view.
Comment 12 Andrew Walker 2006-02-03 02:46:01 UTC
> The size is the size of the object in a Kst view. 

This seems a little circular. What does this physically represent for a line?
Comment 13 Netterfield 2006-02-03 23:19:58 UTC
Best I can tell, arrows and lines are broken at a fundamental level!

-The desired drag points for finite width lines are the centers of the ends of the lines.
-kstViewObjects drag from the corners of the bounding box.

We are left with a few options:
1: when drawing the line, move the end points so they stay inside the bounding box.  When changing orientation, move the corners so they ~make sense.  This is what the current implementation tries to do - and almost succeeds.  For wide lines, the UI is very odd this way.  The current implementation could easily be fixed to make this work (oddly though).

2: re-define the clip region so that it can extend outside the bounding box.  The drag points are where you think they should be.  BUT: the bounding box is smaller than the drawn region, so re-draw might not happen on a partial cover-expose, and the mouse doesn't always know it is over the line or arrow.  This is Andrew's patch.

3: some sort of fundamental re-design to get it right.

I think that #3 is not an option.

#1 & #2 have their own uglinesses.  In triage mode, I guess I would weakly vote for #2.  Others should give their vote (but not spend any more time on it) and I will render an edict tomorrow AM.

Comment 14 Netterfield 2006-02-04 20:29:55 UTC
SVN commit 505756 by netterfield:

CCBUG: 121068

Apply Andrew's patch as a temporary fix for 1.2.0.  Long term, line to/from/drag 
points need to be separated from geometry, so this bug will remain open until this is 
done.



 M  +22 -19    kstviewarrow.cpp  
 M  +1 -1      kstviewarrow.h  
 M  +30 -23    kstviewline.cpp  
 M  +1 -0      kstviewline.h  


--- trunk/extragear/graphics/kst/kst/kstviewarrow.cpp #505755:505756
@@ -64,8 +64,8 @@
 }
 
 
-void KstViewArrow::paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w) {
-  double deltax = _toArrowScaling * 2.0 * double(w);
+void KstViewArrow::paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double arrowScaling) {
+  double deltax = arrowScaling * 2.0 * double(w);
   double theta = atan2(double(from.y() - to.y()), double(from.x() - to.x())) - M_PI / 2.0;
   double sina = sin(theta);
   double cosa = cos(theta);
@@ -87,28 +87,31 @@
 
 QRegion KstViewArrow::clipRegion() {
   if (_clipMask.isNull()) {
+    double arrowScaling = _toArrowScaling;
+    if (_fromArrowScaling > arrowScaling) {
+      arrowScaling = _fromArrowScaling;
+    }
+    
+    int w = int(ceil(sqrt(3.0) * arrowScaling * 2.0 * double(width())));    
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1 );
     _myClipMask = QRegion();
-    QBitmap bm1(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm1.isNull()) {
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
       KstPainter p;
       p.setMakingMask(true);
-      p.begin(&bm1);
+      p.begin(&bm);
       p.setViewXForm(true);
+      
       KstViewLine::paintSelf(p, QRegion());
       p.flush();
-      p.end();
-      _clipMask = QRegion(bm1);
-    }
-    QBitmap bm2(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm2.isNull()) {
-      KstPainter p;
-      p.setMakingMask(true);
-      p.begin(&bm2);
-      p.setViewXForm(true);
-      paintSelf(p, QRegion());
+      _clipMask = QRegion(bm);
+      
+      p.eraseRect(rect);
+      paintSelf(p, QRegion());      
       p.flush();
+      _myClipMask = QRegion(bm);
+      
       p.end();
-      _myClipMask = QRegion(bm2);
     }
   }
 
@@ -127,7 +130,7 @@
       p.setClipRegion(bounds & clip);
     }
   } else {
-      KstViewLine::paintSelf(p, bounds);
+    KstViewLine::paintSelf(p, bounds);
   }
   
   if (hasArrow()) {
@@ -141,10 +144,10 @@
     p.setBrush(_foregroundColor);
     
     if (_hasToArrow) {      
-      paintArrow(p, to, from, w);
+      paintArrow(p, to, from, w, _toArrowScaling);
     }
     if (_hasFromArrow) {      
-      paintArrow(p, from, to, w);
+      paintArrow(p, from, to, w, _fromArrowScaling);
     }
   }
   p.restore();
--- trunk/extragear/graphics/kst/kst/kstviewarrow.h #505755:505756
@@ -39,7 +39,7 @@
     QMap<QString, QVariant > widgetHints(const QString& propertyName) const;
 
     void paintSelf(KstPainter& p, const QRegion& bounds);
-    void paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w);
+    void paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double arrowScaling);
     // true if either end has an arrow 
     bool hasArrow() const;
     
--- trunk/extragear/graphics/kst/kst/kstviewline.cpp #505755:505756
@@ -22,6 +22,7 @@
 
 #include <klocale.h>
 
+#include <qbitmap.h>
 #include <qmetaobject.h>
 #include <qpainter.h>
 #include <qvariant.h>
@@ -84,12 +85,34 @@
 }
 
 
+QRegion KstViewLine::clipRegion() {
+  if (_clipMask.isNull()) {
+    int w = width();
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1);
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
+      KstPainter p;
+      p.setMakingMask(true);
+      p.begin(&bm);
+      p.setViewXForm(true);
+      p.eraseRect(rect);
+      paintSelf(p, QRegion());
+      p.flush();
+      p.end();
+      _clipMask = QRegion(bm);
+    } else {
+      _clipMask = QRegion(); // only invalidate our own variable
+    }
+  }
+
+  return _clipMask;
+}
+
 void KstViewLine::paintSelf(KstPainter& p, const QRegion& bounds) {
   p.save();
   if (p.type() != KstPainter::P_PRINT && p.type() != KstPainter::P_EXPORT) {
     if (p.makingMask()) {
       p.setRasterOp(Qt::SetROP);
-      KstViewObject::paintSelf(p, geometry());
     } else {
       const QRegion clip(clipRegion());
       KstViewObject::paintSelf(p, bounds - clip);
@@ -104,30 +127,14 @@
   p.setPen(pen);
 
   const QRect geom(geometry());
-  int u = 0, v = 0;
-  
-  // Adjust for large widths.  We don't want the line clipped because it goes
-  // out of the bounding box.
-  if (_width > 1 && geom.height() > 0) {
-    double theta = atan(geom.width()/geom.height());
-    int w = _width;
-    if (theta >= 0 && theta <= M_PI/4) {
-      u = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-      v = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-    } else {
-      u = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-      v = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-    }
-  }
-
   switch (_orientation) {
     case UpLeft:
     case DownRight:
-      p.drawLine(geom.bottomRight() + QPoint(-u, -v), geom.topLeft() + QPoint(u, v));
+      p.drawLine(geom.bottomRight(), geom.topLeft());
       break;
     case UpRight:
     case DownLeft:
-      p.drawLine(geom.bottomLeft() + QPoint(u, -v), geom.topRight() + QPoint(-u, v));
+      p.drawLine(geom.bottomLeft(), geom.topRight());
       break;
   }
   p.restore();
@@ -260,21 +267,21 @@
     if (_from.y() < _to.y()) {
       _orientation = DownRight;  
       KstViewObject::move(_from);
-      KstViewObject::resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _to.y() - _from.y() + 1)));
+      KstViewObject::resize(QSize(_to.x() - _from.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpRight;  
       KstViewObject::move(QPoint(_from.x(), _to.y()));
-      KstViewObject::resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      KstViewObject::resize(QSize(_to.x() - _from.x() + 1, _from.y() - _to.y() + 1));
     }
   } else {
     if (_from.y() < _to.y()) {
       _orientation = DownLeft;  
       KstViewObject::move(QPoint(_to.x(), _from.y()));
-      KstViewObject::resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _to.y() - _from.y() + 1)));
+      KstViewObject::resize(QSize(_from.x() - _to.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpLeft;  
       KstViewObject::move(_to);
-      KstViewObject::resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      KstViewObject::resize(QSize(_from.x() - _to.x() + 1, _from.y() - _to.y() + 1));
     }
   }
 }
--- trunk/extragear/graphics/kst/kst/kstviewline.h #505755:505756
@@ -52,6 +52,7 @@
     void setForegroundColor(const QColor& color);
     QColor foregroundColor() const;
 
+    QRegion clipRegion();
     void move(const QPoint& pos);
 
     virtual void setCapStyle(Qt::PenCapStyle style);
Comment 15 Netterfield 2006-02-04 20:36:35 UTC
Created attachment 14546 [details]
Andrew's 'draw outside of geometry' patch against pre-applied svn

This patch has been attached to aid in reverting pre-line/arrow rework.
Comment 16 Netterfield 2006-02-15 01:12:45 UTC
Time for option 3.  Whoever is going to do this needs to first post a plan.
Comment 17 Andrew Walker 2006-03-30 01:21:22 UTC
The full solution for this would seem to require that we move from QRect to QRegion. This would allow us to properly define the region that a line, ellipse and other non-rectangular objects occupy.
Comment 18 George Staikos 2006-03-30 02:29:07 UTC
On Wednesday 29 March 2006 18:21, Andrew Walker wrote:
> ------- The full solution for this would seem to require that we move from
> QRect to QRegion. This would allow us to properly define the region that a
> line, ellipse and other non-rectangular objects occupy.


  Which QRect?  QRegion is quite expensive btw.  I tried to avoid it whenever 
possible.
Comment 19 Andrew Walker 2006-03-30 03:00:05 UTC
I was talking about the _geom QRect. I would hope that a QRegion
that is really just a QRect would be very close in performance
as the native QRect (assuming Qt wrote the code in a clever way).

The only problem I'm aware of the line/arrow problem in Kst with
my previous fix (now reverted) was that you can hover over parts
of the arrow and not set the focus. This is because the findChild
and findDeepestChild use the _geom to detect the focus. Is there
any reason that they don't use the clip region, in which case this
problem goes away?

Andrew

-----Original Message-----
From: George Staikos [mailto:staikos@kde.org]
Sent: Wednesday, March 29, 2006 4:29 PM
To: kst@kde.org
Subject: [Kst] [Bug 121068] Arrow view objects not always clipped
correctly


------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugs.kde.org/show_bug.cgi?id=121068




------- Additional Comments From staikos kde org  2006-03-30 02:29 -------
On Wednesday 29 March 2006 18:21, Andrew Walker wrote:
> ------- The full solution for this would seem to require that we move from
> QRect to QRegion. This would allow us to properly define the region that a
> line, ellipse and other non-rectangular objects occupy.



  Which QRect?  QRegion is quite expensive btw.  I tried to avoid it
whenever
possible.
_______________________________________________
Kst mailing list
Kst@kde.org
https://mail.kde.org/mailman/listinfo/kst
Comment 20 Andrew Walker 2006-07-06 20:51:58 UTC
*** Bug 130283 has been marked as a duplicate of this bug. ***
Comment 21 Andrew Walker 2006-07-06 23:50:09 UTC
A relatively simple test of whether somethinging is a clipping problem is to export the window contents. Printing and exporting does away with the whole clipping scheme, so a problem that does not manifest during export may be caused by clipping.
Comment 22 Andrew Walker 2007-05-07 23:10:38 UTC
See also related bug reports #123561 and #143806
Comment 23 Andrew Walker 2007-05-20 23:43:38 UTC
SVN commit 666781 by arwalker:

BUG:121068 Correctly handle clipping and selection of arrow view objects.

 M  +62 -38    kstviewarrow.cpp  
 M  +1 -0      kstviewarrow.h  
 M  +5 -5      kstviewellipse.cpp  
 M  +42 -32    kstviewline.cpp  
 M  +1 -0      kstviewline.h  


--- branches/work/kst/1.5/kst/src/libkstapp/kstviewarrow.cpp #666780:666781
@@ -29,6 +29,8 @@
 #include <qpainter.h>
 #include <qvariant.h>
 
+#define SIZE_ARROW (2.0 * sqrt(3.0))
+
 KstViewArrow::KstViewArrow()
 : KstViewLine("Arrow") {
   _editTitle = i18n("Edit Arrow");
@@ -54,9 +56,9 @@
         setProperty(el.tagName().latin1(), QVariant(el.text()));  
       }  
     }
-    n = n.nextSibling();      
+    n = n.nextSibling();
   }
-  
+
   // always has this value
   _type = "Arrow";
   _editTitle = i18n("Edit Arrow");
@@ -70,7 +72,7 @@
   _hasToArrow = arrow._hasToArrow;
   _fromArrowScaling = arrow._fromArrowScaling;
   _toArrowScaling = arrow._toArrowScaling;
-  
+
   // these always have these values
   _type = "Arrow";
   _standardActions |= Delete | Edit;
@@ -92,50 +94,48 @@
 
 
 void KstViewArrow::paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double scaling) {
-  double deltax = scaling * 2.0 * double(w);
+  double deltax = scaling * double(w);
   double theta = atan2(double(from.y() - to.y()), double(from.x() - to.x())) - M_PI / 2.0;
   double sina = sin(theta);
   double cosa = cos(theta);
-  double yin = sqrt(3.0) * deltax;
+  double yin = SIZE_ARROW * deltax;
   double x1, y1, x2, y2;
   QWMatrix m(cosa, sina, -sina, cosa, 0.0, 0.0);
-  
+
   m.map( deltax, yin, &x1, &y1);
   m.map(-deltax, yin, &x2, &y2);
-  
+
   QPointArray pts(3);
   pts[0] = to;
   pts[1] = to + QPoint(d2i(x1), d2i(y1));
   pts[2] = to + QPoint(d2i(x2), d2i(y2));
-  
+
   p.drawPolygon(pts);
 }
 
 
 QRegion KstViewArrow::clipRegion() {
   if (_clipMask.isNull()) {
+    double scaling = kMax(_fromArrowScaling, _toArrowScaling);
+    int w = int(ceil(SIZE_ARROW * scaling * double(width())));
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1);
     _myClipMask = QRegion();
-    QBitmap bm1(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm1.isNull()) {
+    QBitmap bm(rect.size(), true);
+    if (!bm.isNull()) {
       KstPainter p;
+
       p.setMakingMask(true);
-      p.begin(&bm1);
+      p.begin(&bm);
       p.setViewXForm(true);
       KstViewLine::paintSelf(p, QRegion());
       p.flush();
-      p.end();
-      _clipMask = QRegion(bm1);
-    }
-    QBitmap bm2(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
-    if (!bm2.isNull()) {
-      KstPainter p;
-      p.setMakingMask(true);
-      p.begin(&bm2);
-      p.setViewXForm(true);
+      _clipMask = QRegion(bm);
+
+      p.eraseRect(rect);
       paintSelf(p, QRegion());
       p.flush();
+      _myClipMask = QRegion(bm);
       p.end();
-      _myClipMask = QRegion(bm2);
     }
   }
 
@@ -154,23 +154,23 @@
       p.setClipRegion(bounds & clip);
     }
   } else {
-      KstViewLine::paintSelf(p, bounds);
+    KstViewLine::paintSelf(p, bounds);
   }
-  
+
   if (hasArrow()) {
     QPoint to = KstViewLine::to();
-    QPoint from = KstViewLine::from();    
+    QPoint from = KstViewLine::from();
     const int w = width() * p.lineWidthAdjustmentFactor();
     QPen pen(_foregroundColor, w);
-    
+
     pen.setCapStyle(capStyle());
     p.setPen(pen);
     p.setBrush(_foregroundColor);
-    
-    if (_hasToArrow) {      
+
+    if (_hasToArrow) {
       paintArrow(p, to, from, w, _toArrowScaling);
     }
-    if (_hasFromArrow) {      
+    if (_hasFromArrow) {
       paintArrow(p, from, to, w, _fromArrowScaling);
     }
   }
@@ -220,40 +220,40 @@
 
 
 bool KstViewArrow::hasFromArrow() const {
-  return _hasFromArrow;  
+  return _hasFromArrow;
 }
 
 
 void KstViewArrow::setHasFromArrow(bool yes) {
   if (_hasFromArrow != yes) {
-    _hasFromArrow = yes;  
+    _hasFromArrow = yes;
     setDirty();
   }
 }
 
 
 bool KstViewArrow::hasToArrow() const {
-  return _hasToArrow;  
+  return _hasToArrow;
 }
 
 
 void KstViewArrow::setHasToArrow(bool yes) {
   if (_hasToArrow != yes) {
-    _hasToArrow = yes;  
+    _hasToArrow = yes;
     setDirty();
   }
 }
 
 
 double KstViewArrow::fromArrowScaling() const {
-  return _fromArrowScaling;  
+  return _fromArrowScaling;
 }
 
 
 void KstViewArrow::setFromArrowScaling(double scaling) {
   if (scaling < 1.0) {
-    scaling = 1.0;  
-  }  
+    scaling = 1.0;
+  }
   if (_fromArrowScaling != scaling) {
     _fromArrowScaling = scaling;
     setDirty();
@@ -262,14 +262,14 @@
 
 
 double KstViewArrow::toArrowScaling() const {
-  return _toArrowScaling;  
+  return _toArrowScaling;
 }
 
 
 void KstViewArrow::setToArrowScaling(double scaling) {
   if (scaling < 1.0) {
-    scaling = 1.0;  
-  }  
+    scaling = 1.0;
+  }
   if (_toArrowScaling != scaling) {
     _toArrowScaling = scaling;
     setDirty();
@@ -277,6 +277,30 @@
 }
 
 
+QRect KstViewArrow::surroundingGeometry() const {
+  QRect geom(geometry());
+
+  if (_hasFromArrow || _hasToArrow) {
+    double scaling;
+    if (_hasFromArrow && _hasToArrow) {
+      scaling = kMax(_fromArrowScaling, _toArrowScaling);
+    } else if (_hasFromArrow) {
+      scaling = _fromArrowScaling;
+    } else {
+      scaling = _toArrowScaling;
+    }
+    geom.setLeft(geom.left() - int( SIZE_ARROW * scaling * double(width()/2.0) ) - 1);
+    geom.setRight(geom.right() + int( SIZE_ARROW * scaling * double(width()/2.0) ) + 1);
+    geom.setTop(geom.top() - int( SIZE_ARROW * scaling * double(width()/2.0) ) - 1);
+    geom.setBottom(geom.bottom() + int( SIZE_ARROW * scaling * double(width()/2.0) ) + 1);
+  } else {
+    geom = KstViewLine::surroundingGeometry();
+  }
+
+  return geom;
+}
+
+
 namespace {
 KstViewObject *create_KstViewArrow() {
   return new KstViewArrow;
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewarrow.h #666780:666781
@@ -55,6 +55,7 @@
     double toArrowScaling() const;
     void setToArrowScaling(double scaling);
     QRegion clipRegion();
+    virtual QRect surroundingGeometry() const;
 
   public:
     void save(QTextStream& ts, const QString& indent = QString::null);
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewellipse.cpp #666780:666781
@@ -46,9 +46,9 @@
         setProperty(el.tagName().latin1(), QVariant(el.text()));  
       }  
     }
-    n = n.nextSibling();      
+    n = n.nextSibling();
   }
-  
+
   // always have these values
   _type = "Ellipse";
   _editTitle = i18n("Edit Ellipse");
@@ -63,7 +63,7 @@
   _transparentFill = ellipse._transparentFill;
   _borderWidth = ellipse._borderWidth;
   _borderColor = ellipse._borderColor;
-  
+
   // these always have these values
   _type = "Ellipse";
   _standardActions |= Delete | Edit;
@@ -79,7 +79,7 @@
 
   KstViewEllipse* viewEllipse = new KstViewEllipse(*this);
   parent.appendChild(viewEllipse, true);
-  
+
   return viewEllipse;
 }
 
@@ -201,7 +201,7 @@
   p.drawEllipse(rect);  
 }
 
-    
+
 void KstViewEllipse::setTransparentFill(bool yes) {
   if (_transparentFill != yes) {
     _transparentFill = yes;
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewline.cpp #666780:666781
@@ -22,6 +22,7 @@
 
 #include <klocale.h>
 
+#include <qbitmap.h>
 #include <qmetaobject.h>
 #include <qpainter.h>
 #include <qvariant.h>
@@ -59,7 +60,7 @@
     }
     n = n.nextSibling();
   }
-  
+
   switch (orientationInt) {
     case 1:
       _orientation = UpRight;
@@ -90,7 +91,7 @@
   _penStyle = line._penStyle;
   _orientation = line._orientation;
   _width = line._width;
-  
+
   // these always have these values
   _type = "Line";
   _standardActions |= Delete | Edit;
@@ -106,7 +107,7 @@
 
   KstViewLine* viewLine = new KstViewLine(*this);
   parent.appendChild(viewLine, true);
-  
+
   return viewLine;
 }
 
@@ -116,7 +117,6 @@
   if (p.type() != KstPainter::P_PRINT && p.type() != KstPainter::P_EXPORT) {
     if (p.makingMask()) {
       p.setRasterOp(Qt::SetROP);
-      KstViewObject::paintSelf(p, geometry());
     } else {
       const QRegion clip(clipRegion());
       KstViewObject::paintSelf(p, bounds - clip);
@@ -132,29 +132,15 @@
   p.setPen(pen);
 
   const QRect geom(geometry());
-  int u = 0, v = 0;
-  
-  // Adjust for large widths.  We don't want the line clipped because it goes
-  // out of the bounding box.
-  if (w > 1 && geom.height() > 0) {
-    double theta = atan(geom.width()/geom.height());
-    if (theta >= 0 && theta <= M_PI/4) {
-      u = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-      v = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-    } else {
-      u = int(fabs((w / 2.0) * (1.5*sin(theta) + 0.5*cos(theta))));
-      v = int(fabs((w / 2.0) * (sin(theta) + cos(theta))));
-    }
-  }
 
   switch (_orientation) {
     case UpLeft:
     case DownRight:
-      p.drawLine(geom.bottomRight() + QPoint(-u, -v), geom.topLeft() + QPoint(u, v));
+      p.drawLine(geom.bottomRight(), geom.topLeft());
       break;
     case UpRight:
     case DownLeft:
-      p.drawLine(geom.bottomLeft() + QPoint(u, -v), geom.topRight() + QPoint(-u, v));
+      p.drawLine(geom.bottomLeft(), geom.topRight());
       break;
   }
   p.restore();
@@ -260,6 +246,31 @@
 }
 
 
+QRegion KstViewLine::clipRegion() {
+  if (_clipMask.isNull()) {
+    int w = width();
+    QRect rect(0, 0, _geom.bottomRight().x() + w + 1, _geom.bottomRight().y() + w + 1);
+    QBitmap bm(rect.size(), true);
+
+    if (!bm.isNull()) {
+      KstPainter p;
+      p.setMakingMask(true);
+      p.begin(&bm);
+      p.setViewXForm(true);
+      p.eraseRect(rect);
+      paintSelf(p, QRegion());
+      p.flush();
+      p.end();
+      _clipMask = QRegion(bm);
+    } else {
+      _clipMask = QRegion(); // only invalidate our own variable
+    }
+  }
+
+  return _clipMask; 
+}
+
+
 void KstViewLine::move(const QPoint& pos) {
   KstViewObject::move(pos);
   if (_from.x() < _to.x()) {
@@ -287,21 +298,21 @@
     if (_from.y() < _to.y()) {
       _orientation = DownRight;  
       KstViewObject::move(_from);
-      KstViewObject::resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _to.y() - _from.y() + 1)));
+      KstViewObject::resize(QSize(_to.x() - _from.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpRight;  
       KstViewObject::move(QPoint(_from.x(), _to.y()));
-      KstViewObject::resize(QSize(kMax(_width, _to.x() - _from.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      KstViewObject::resize(QSize(_to.x() - _from.x() + 1, _from.y() - _to.y() + 1));
     }
   } else {
     if (_from.y() < _to.y()) {
       _orientation = DownLeft;  
       KstViewObject::move(QPoint(_to.x(), _from.y()));
-      KstViewObject::resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _to.y() - _from.y() + 1)));
+      KstViewObject::resize(QSize(_from.x() - _to.x() + 1, _to.y() - _from.y() + 1));
     } else {
       _orientation = UpLeft;  
       KstViewObject::move(_to);
-      KstViewObject::resize(QSize(kMax(_width, _from.x() - _to.x() + 1), kMax(_width, _from.y() - _to.y() + 1)));
+      KstViewObject::resize(QSize(_from.x() - _to.x() + 1, _from.y() - _to.y() + 1));
     }
   }
 }
@@ -310,7 +321,7 @@
 void KstViewLine::drawFocusRect(KstPainter& p) {
   // draw the hotpoints
   QPoint point1, point2;
-  
+
   const int dx = KST_RESIZE_BORDER_W/2;
 
   const QRect geom(geometry());
@@ -348,9 +359,9 @@
 
 signed int KstViewLine::directionFor(const QPoint& pos) {
   if (!isSelected()) {
-    return NONE;  
+    return NONE;
   }
-  
+
   const QRect geom(geometry());
   switch (_orientation) {
     case UpLeft:
@@ -393,7 +404,7 @@
   if (!map.empty()) {
     return map;  
   }
-  
+
   if (propertyName == "width") {
     map.insert(QString("_kst_widgetType"), QString("QSpinBox"));
     map.insert(QString("_kst_label"), i18n("Line width"));  
@@ -475,15 +486,14 @@
 
 QRect KstViewLine::surroundingGeometry() const {
   QRect geom(geometry());
-  if (from().x() == to().x()) {
-    //vertical line
+
+  if (width() > 1) {
     geom.setLeft(geom.left() - width()/2 - 1);
     geom.setRight(geom.right() + width()/2 + 1);
-  } else if (from().y() == to().y()) {
-    //horizontal line
     geom.setTop(geom.top() - width()/2 - 1);
     geom.setBottom(geom.bottom() + width()/2 + 1);
   }
+
   return geom;
 }
 
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewline.h #666780:666781
@@ -54,6 +54,7 @@
     void setForegroundColor(const QColor& color);
     QColor foregroundColor() const;
 
+    QRegion clipRegion();
     void move(const QPoint& pos);
 
     virtual void setCapStyle(Qt::PenCapStyle style);