Bug 149402

Summary: transparent objects with children are not drawn correctly
Product: [Applications] kst Reporter: Andrew Walker <arwalker>
Component: generalAssignee: kst
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: 1.4.0   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Andrew Walker 2007-08-31 01:33:28 UTC
Version:           1.4.0 (using KDE KDE 3.5.1)
Installed from:    Compiled From Sources
OS:                Linux

PROBLEM:
When picture, legend, label, or arrow objects are added as children to transparent objects they are not drawn correctly.

STEPS TO REPRODUCE:
Start Kst
Draw an arrow
Draw an ellipse and make it transparent
Drag the arrow into the ellipse

RESULTS:
Only the arrow head is visible

EXPECTED RESULTS:
All of the arrow should be drawn
Comment 1 Andrew Walker 2007-08-31 01:41:01 UTC
The problem is caused by incorrect calculation of the clipRegion of a transparent.
This calculation is done in KstViewObject::clipRegion().

The problem, in essence, is that some objects internally use _clipMask and _myClipMask to cache their clipping region. In these cases paintSelf only draws the object associated with _myClipMask with the makingMask flag is set on the KstPainter.

However, KstViewObject::clipRegion() is implicitly expecting paintSelf() to draw both objects (associated with _clipMask and _myClipMask) else the clipping region will not be correctly determined.

There would seem to be three alternatives:

1) have paintSelf() draw everything when the makingMask flag is set
2) have Kst::clipRegion() not rely on KstViewObject::paint() to determine the clipping region
3) remove entirely the clipping mechanism as it is not necessary
Comment 2 Andrew Walker 2007-08-31 18:41:02 UTC
SVN commit 706959 by arwalker:

BUG:149402 fix problems with transparent objects with children

 M  +8 -12     kstviewarrow.cpp  
 M  +3 -6      kstviewarrow.h  
 M  +32 -28    kstviewlabel.cpp  
 M  +1 -3      kstviewlabel.h  
 M  +39 -35    kstviewlegend.cpp  
 M  +1 -3      kstviewlegend.h  
 M  +2 -2      kstviewline.cpp  
 M  +9 -2      kstviewobject.cpp  
 M  +4 -5      kstviewpicture.cpp  
 M  +3 -4      kstviewpicture.h  


--- branches/work/kst/1.5/kst/src/libkstapp/kstviewarrow.cpp #706958:706959
@@ -54,7 +54,7 @@
     if (!el.isNull()) {
       if (metaObject()->findProperty(el.tagName().latin1(), true) > -1) {
         setProperty(el.tagName().latin1(), QVariant(el.text()));  
-      }  
+      }
     }
     n = n.nextSibling();
   }
@@ -126,7 +126,7 @@
     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 bm(rect.size(), true);
     if (!bm.isNull()) {
       KstPainter p;
@@ -134,19 +134,14 @@
       p.setMakingMask(true);
       p.begin(&bm);
       p.setViewXForm(true);
-      KstViewLine::paintSelf(p, QRegion());
-      p.flush();
-      _clipMask = QRegion(bm);
-
       p.eraseRect(rect);
       paintSelf(p, QRegion());
       p.flush();
-      _myClipMask = QRegion(bm);
-      p.end();
+      _clipMask = QRegion(bm);
     }
   }
 
-  return _myClipMask | _clipMask;
+  return _clipMask;
 }
 
 
@@ -154,10 +149,11 @@
   p.save();
   if (p.type() != KstPainter::P_PRINT && p.type() != KstPainter::P_EXPORT) {
     if (p.makingMask()) {
+      KstViewLine::paintSelf(p, bounds);
       p.setRasterOp(Qt::SetROP);
     } else {
       const QRegion clip(clipRegion());
-      KstViewLine::paintSelf(p, bounds - _myClipMask);
+      KstViewLine::paintSelf(p, bounds);
       p.setClipRegion(bounds & clip);
     }
   } else {
@@ -201,7 +197,7 @@
 QMap<QString, QVariant> KstViewArrow::widgetHints(const QString& propertyName) const {
   QMap<QString, QVariant> map = KstViewLine::widgetHints(propertyName);
   if (!map.empty()) {
-    return map;  
+    return map;
   }
   if (propertyName == "hasFromArrow") {
     map.insert(QString("_kst_widgetType"), QString("QCheckBox"));
@@ -323,4 +319,4 @@
 
 
 #include "kstviewarrow.moc"
-// vim: ts=2 sw=2 et
+
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewarrow.h #706958:706959
@@ -30,13 +30,13 @@
   Q_PROPERTY(bool hasToArrow READ hasToArrow WRITE setHasToArrow)
   Q_PROPERTY(double fromArrowScaling READ fromArrowScaling WRITE setFromArrowScaling)
   Q_PROPERTY(double toArrowScaling READ toArrowScaling WRITE setToArrowScaling)
-      
+
   public:
     KstViewArrow();
     KstViewArrow(const QDomElement& e);
     KstViewArrow(const KstViewArrow& arrow);
     ~KstViewArrow();
-    
+
     QMap<QString, QVariant > widgetHints(const QString& propertyName) const;
 
     virtual KstViewObject* copyObjectQuietly(KstViewObject& parent, const QString& name = QString::null) const;
@@ -44,9 +44,7 @@
 
     void paintSelf(KstPainter& p, const QRegion& bounds);
     void paintArrow(KstPainter& p, const QPoint& to, const QPoint &from, int w, double scaling);
-    // true if either end has an arrow 
     bool hasArrow() const;
-    
     bool hasFromArrow() const;
     void setHasFromArrow(bool yes);
     bool hasToArrow() const;
@@ -60,13 +58,12 @@
 
   public:
     void save(QTextStream& ts, const QString& indent = QString::null);
-    
+
   private:
     bool _hasFromArrow;
     bool _hasToArrow;
     double _fromArrowScaling;
     double _toArrowScaling;
-    QRegion _myClipMask;
 };
 
 typedef KstObjectList<KstViewArrowPtr> KstViewArrowList;
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewlabel.cpp #706958:706959
@@ -417,7 +417,7 @@
 
     QRect cr(contentsRectForPainter(p));
     cr.setSize(sizeForText(_parent->geometry()));
-    setContentsRectForPainter(p, cr);    
+    setContentsRectForPainter(p, cr);
     KstBorderedViewObject::paintSelf(p, bounds);
 
     p.translate(cr.left(), cr.top());
@@ -429,50 +429,54 @@
     _absFontSize = absFontSizeOld;
   } else {
     if (p.makingMask()) {
+      KstBorderedViewObject::paintSelf(p, bounds);
       p.setRasterOp(Qt::SetROP);
+      const QRect cr(contentsRect());
+      // slow but preserves antialiasing...
+      QBitmap bm = _backBuffer.buffer().createHeuristicMask(false);
+      bm.setMask(bm);
+      p.drawPixmap(cr.left(), cr.top(), bm, 0, 0, cr.width(), cr.height());
     } else {
       const QRegion clip(clipRegion());
-      KstBorderedViewObject::paintSelf(p, bounds - _myClipMask);
+      KstBorderedViewObject::paintSelf(p, bounds);
       p.setClipRegion(bounds & clip);
+      _backBuffer.paintInto(p, contentsRect());
     }
-
-    _backBuffer.paintInto(p, contentsRect());
   }
   p.restore();
 }
 
 
-void KstViewLabel::invalidateClipRegion() {
-  KstBorderedViewObject::invalidateClipRegion();
-  _myClipMask = QRegion();
-}
+QRegion KstViewLabel::clipRegion() {
+  if (_clipMask.isNull()) {
+    if (_transparent) {
+      const QRect cr(contentsRect());
+      // slow but preserves antialiasing...
+      QBitmap bm = _backBuffer.buffer().createHeuristicMask(false);
 
+      _clipMask = QRegion(bm);
+      _clipMask.translate(cr.topLeft().x(), cr.topLeft().y());
 
-QRegion KstViewLabel::clipRegion() {
-  if (!_transparent) {
-    return KstBorderedViewObject::clipRegion();
-  }
+      QBitmap bm1(_geom.bottomRight().x() + 1, _geom.bottomRight().y() + 1, true);
+      if (!bm1.isNull()) {
+        KstPainter p;
 
-  if (_clipMask.isNull() && _myClipMask.isNull()) {
-    const QRect cr(contentsRect());
-    QBitmap bm = _backBuffer.buffer().createHeuristicMask(false); // slow but preserves antialiasing...
-    _myClipMask = QRegion(bm);
-    _myClipMask.translate(cr.topLeft().x(), cr.topLeft().y());
+        p.setMakingMask(true);
+        p.begin(&bm1);
+        p.setViewXForm(true);
+        KstBorderedViewObject::paintSelf(p, QRegion());
+        paint(p, QRegion());
+        p.flush();
+        p.end();
 
-    QBitmap bm1(_geom.bottomRight().x() + 1, _geom.bottomRight().y() + 1, true);
-    if (!bm1.isNull()) {
-      KstPainter p;
-      p.setMakingMask(true);
-      p.begin(&bm1);
-      p.setViewXForm(true);
-      KstBorderedViewObject::paintSelf(p, QRegion());
-      p.flush();
-      p.end();
-      _clipMask = QRegion(bm1);
+        _clipMask |= QRegion(bm1);
+      }
+    } else {
+      _clipMask = KstBorderedViewObject::clipRegion();
     }
   }
 
-  return _clipMask | _myClipMask;
+  return _clipMask;
 }
 
 
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewlabel.h #706958:706959
@@ -88,7 +88,6 @@
     void paintSelf(KstPainter& p, const QRegion& bounds);
     void resize(const QSize&);
     QRegion clipRegion();
-    void invalidateClipRegion();
 
     void setLabelMargin(int margin);
     int labelMargin() const;
@@ -133,7 +132,6 @@
     KstLJustifyType _justify;
     KstBackBuffer _backBuffer;
     Label::Parsed *_parsed;
-    QRegion _myClipMask;
     int _labelMargin;
 
     struct DataCache {
@@ -154,4 +152,4 @@
 typedef KstObjectList<KstSharedPtr<KstViewLabel> > KstViewLabelList;
 
 #endif
-// vim: ts=2 sw=2 et
+
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewlegend.cpp #706958:706959
@@ -182,7 +182,7 @@
 
 KstViewObject* KstViewLegend::copyObjectQuietly(KstViewObject &parent, const QString& name) const { 
   Q_UNUSED(name)
-  
+
   KstViewLegend* viewLegend = new KstViewLegend(*this);
   parent.appendChild(viewLegend, true);
 
@@ -239,13 +239,15 @@
 
 
 void KstViewLegend::drawToBuffer() {
+  KstPainter p;
+  QPen pen;
+
   setDirty(false);
 
   _backBuffer.buffer().resize(contentsRect().size());
   _backBuffer.buffer().fill(backgroundColor());
-  KstPainter p;
+
   p.begin(&_backBuffer.buffer());
-  QPen pen;
   pen.setColor(foregroundColor());
   p.setPen(pen);
   drawToPainter(p);
@@ -394,11 +396,11 @@
 
 
 void KstViewLegend::paintSelf(KstPainter& p, const QRegion& bounds) {
+  p.save();
   if (p.type() == KstPainter::P_PRINT || p.type() == KstPainter::P_EXPORT) {
-    p.save();
     QRect cr(contentsRectForPainter(p));
     cr.setSize(sizeForText(_parent->geometry()));
-    setContentsRectForPainter(p, cr);    
+    setContentsRectForPainter(p, cr);
     KstBorderedViewObject::paintSelf(p, bounds);
 
     p.translate(cr.left(), cr.top());
@@ -406,54 +408,56 @@
       p.fillRect(0, 0, cr.width(), cr.height(), _backgroundColor);
     }
     drawToPainter(p);
-
-    p.restore();
   } else {
-    const QRect cr(contentsRect());
     if (p.makingMask()) {
+      KstBorderedViewObject::paintSelf(p, bounds);
       p.setRasterOp(Qt::SetROP);
+      const QRect cr(contentsRect());
+      // slow but preserves antialiasing...
+      QBitmap bm = _backBuffer.buffer().createHeuristicMask(false);
+      bm.setMask(bm);
+      p.drawPixmap(cr.left(), cr.top(), bm, 0, 0, cr.width(), cr.height());
     } else {
       const QRegion clip(clipRegion());
-      KstBorderedViewObject::paintSelf(p, bounds - _myClipMask);
+      KstBorderedViewObject::paintSelf(p, bounds);
       p.setClipRegion(bounds & clip);
+      _backBuffer.paintInto(p, contentsRect());
     }
-
-    _backBuffer.paintInto(p, cr);
   }
+  p.restore();
 }
 
 
-void KstViewLegend::invalidateClipRegion() {
-  KstBorderedViewObject::invalidateClipRegion();
-  _myClipMask = QRegion();
-}
+QRegion KstViewLegend::clipRegion() {
+  if (_clipMask.isNull()) {
+    if (_transparent) {
+      const QRect cr(contentsRect());
+      // slow but preserves antialiasing...
+      QBitmap bm = _backBuffer.buffer().createHeuristicMask(false);
 
+      _clipMask = QRegion(bm);
+      _clipMask.translate(cr.topLeft().x(), cr.topLeft().y());
 
-QRegion KstViewLegend::clipRegion() {
-  if (!_transparent) {
-    return KstBorderedViewObject::clipRegion();
-  }
+      QBitmap bm1(_geom.bottomRight().x() + 1, _geom.bottomRight().y() + 1, true);
+      if (!bm1.isNull()) {
+        KstPainter p;
 
-  if (_clipMask.isNull() && _myClipMask.isNull()) {
-    const QRect cr(contentsRect());
-    QBitmap bm = _backBuffer.buffer().createHeuristicMask(false); // slow but preserves antialiasing...
-    _myClipMask = QRegion(bm);
-    _myClipMask.translate(cr.topLeft().x(), cr.topLeft().y());
+        p.setMakingMask(true);
+        p.begin(&bm1);
+        p.setViewXForm(true);
+        KstBorderedViewObject::paintSelf(p, QRegion());
+        paint(p, QRegion());
+        p.flush();
+        p.end();
 
-    QBitmap bm1(_geom.bottomRight().x() + 1, _geom.bottomRight().y() + 1, true);
-    if (!bm1.isNull()) {
-      KstPainter p;
-      p.setMakingMask(true);
-      p.begin(&bm1);
-      p.setViewXForm(true);
-      KstBorderedViewObject::paintSelf(p, QRegion());
-      p.flush();
-      p.end();
-      _clipMask = QRegion(bm1);
+        _clipMask |= QRegion(bm1);
+      }
+    } else {
+      _clipMask = KstBorderedViewObject::clipRegion();
     }
   }
 
-  return _clipMask | _myClipMask;
+  return _clipMask;
 }
 
 
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewlegend.h #706958:706959
@@ -98,7 +98,6 @@
 
     KstBaseCurveList& curves();
 
-    void invalidateClipRegion();
     bool trackContents() const;
     void setTrackContents(bool track);
     void adjustSizeForText(const QRect& w);
@@ -133,7 +132,6 @@
     int _legendMargin;
     KstBackBuffer _backBuffer;
     KstBaseCurveList _curves;
-    QRegion _myClipMask;
     bool _trackContents;
 
     QString _title;
@@ -143,4 +141,4 @@
 typedef KstSharedPtr<KstViewLegend> KstViewLegendPtr;
 
 #endif
-// vim: ts=2 sw=2 et
+
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewline.cpp #706958:706959
@@ -87,6 +87,7 @@
 
 KstViewLine::KstViewLine(const KstViewLine& line)
 : KstViewObject(line) {
+  setTransparent(true);
   _capStyle = line._capStyle;
   _penStyle = line._penStyle;
   _orientation = line._orientation;
@@ -216,7 +217,6 @@
 void KstViewLine::setWidth(int width) {
   if (_width != width) {
     _width = width;
-    //updateOrientation();
     setDirty();
   }
 }
@@ -274,7 +274,7 @@
     }
   }
 
-  return _clipMask; 
+  return _clipMask;
 }
 
 
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewobject.cpp #706958:706959
@@ -380,12 +380,13 @@
           kstdDebug() << "   -> object " << (*i)->tagName() << " took " << x << "ms" << endl;
 #endif
         }
+
         if (i == begin) {
           break;
         }
       }
     }
-    // Paint ourself
+
     paintSelf(p, clipRegion - p.uiMask());
   }
 
@@ -1891,8 +1892,9 @@
       QBitmap bm(_geom.bottomRight().x(), _geom.bottomRight().y(), true);
       if (!bm.isNull()) {
         KstPainter p;
+
+        p.begin(&bm);
         p.setMakingMask(true);
-        p.begin(&bm);
         p.setViewXForm(true);
         paint(p, QRegion());
         p.flush();
@@ -2091,6 +2093,11 @@
 
 void KstViewObject::invalidateClipRegion() {
   _clipMask = QRegion();
+  if (_parent) {
+    if (_parent->transparent()) {
+      _parent->invalidateClipRegion();
+    }
+  }
 }
 
 
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewpicture.cpp #706958:706959
@@ -107,8 +107,6 @@
 
 QRegion KstViewPicture::clipRegion() {
   if (_clipMask.isNull()) {
-    _myClipMask = QRegion();
-
     QBitmap bm(_geom.bottomRight().x() + 1, _geom.bottomRight().y() + 1, true);
     if (!bm.isNull()) {
       KstPainter p;
@@ -124,12 +122,12 @@
       p.eraseRect(0, 0, _geom.bottomRight().x() + 1, _geom.bottomRight().y() + 1);
       paintSelf(p, QRegion());
       p.flush();
-      _myClipMask = QRegion(bm);
+      _clipMask |= QRegion(bm);
       p.end();
     }
   }
 
-  return _myClipMask | _clipMask;
+  return _clipMask;
 }
 
 
@@ -137,10 +135,11 @@
   p.save();
   if (p.type() != KstPainter::P_PRINT && p.type() != KstPainter::P_EXPORT) {
     if (p.makingMask()) {
+      KstBorderedViewObject::paintSelf(p, bounds);
       p.setRasterOp(Qt::OrROP);
     } else {
       const QRegion clip(clipRegion());
-      KstBorderedViewObject::paintSelf(p, bounds - _myClipMask);
+      KstBorderedViewObject::paintSelf(p, bounds);
       p.setClipRegion(bounds & clip);
     }
   } else {
--- branches/work/kst/1.5/kst/src/libkstapp/kstviewpicture.h #706958:706959
@@ -45,7 +45,7 @@
     void setImage(const QImage& image);
     const QString& url() const;
     const QImage& image() const;
-    
+
     // resize picture to the image size
     void restoreSize();
     void restoreAspect();
@@ -56,7 +56,7 @@
     // 0 == no refresh (default)
     void setRefreshTimer(int seconds);
     int refreshTimer() const;
-    
+
     virtual QMap<QString, QVariant> widgetHints(const QString& propertyName) const;
     QRegion clipRegion();
 
@@ -74,11 +74,10 @@
     QString _url;
     int _refresh;
     QTimer *_timer;
-    QRegion _myClipMask;
 };
 
 typedef KstObjectList<KstViewPicturePtr> KstViewPictureList;
 
 
 #endif
-// vim: ts=2 sw=2 et
+