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
This also applies to line objects.
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()) {
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;
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.
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;
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.
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?
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.
Created attachment 14498 [details] Propsosed patch
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.
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.
> 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?
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.
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);
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.
Time for option 3. Whoever is going to do this needs to first post a plan.
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.
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.
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
*** Bug 130283 has been marked as a duplicate of this bug. ***
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.
See also related bug reports #123561 and #143806
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);