[Panel-devel] KDE/kdebase/workspace/libs/plasma/widgets

Aaron J. Seigo aseigo at kde.org
Fri Jul 27 22:00:08 CEST 2007


SVN commit 693361 by aseigo:

someone got confused as to the difference between "the layout that manages my children" and "the layout that manages me". this fixes that and stops crashes in applets that use vbox/hbox

* don't crash when parent=0 is passed in
* don't divid by 0 when we have no children
* have the (fugly) setManagingLayout and unsetManagingLayout and managingLayout methods
* do some memory management so layouts that are thrown around don't get lost in the heap

some unit tests would be nice. hell, a design document on the layout stuff would be nice. i know we're only keeping this around for 4.0, but  ... yeah. ugh.
CCMAIL:panel-devel at kde.org


 M  +12 -7     boxlayout.cpp  
 M  +6 -6      hboxlayout.cpp  
 M  +18 -10    layout.cpp  
 M  +34 -11    layoutitem.cpp  
 M  +23 -14    layoutitem.h  
 M  +9 -4      vboxlayout.cpp  
 M  +1 -19     widget.cpp  
 M  +0 -10     widget.h  


--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/boxlayout.cpp #693360:693361
@@ -37,15 +37,16 @@
     : Layout(parent),
       d(new Private)
 {
-    parent->setLayout(this);
+    if (parent) {
+        parent->setLayout(this);
+    }
 }
 
 BoxLayout::~BoxLayout()
 {
-    foreach (LayoutItem *l, d->childList) {
-        l->resetLayout();
+    foreach (LayoutItem* item, children()) {
+        item->unsetManagingLayout(this);
     }
-
     delete d;
 }
 
@@ -90,7 +91,7 @@
         return;
     }
 
-    l->setLayout(this);
+    //l->setLayout(this);
     d->childList.insert(index, l);
     setGeometry(geometry());
 }
@@ -101,14 +102,18 @@
         return;
     }
 
-    l->setLayout(this);
+    l->setManagingLayout(this);
     d->childList.append(l);
-    qDebug("Added Child LayoutItem : %p", l);
     setGeometry(geometry());
 }
 
 void BoxLayout::removeItem(LayoutItem *l)
 {
+    if (!l) {
+        return;
+    }
+
+    l->unsetManagingLayout(this);
     d->childList.removeAll(l);
     setGeometry(geometry());
 }
--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/hboxlayout.cpp #693360:693361
@@ -66,12 +66,10 @@
     QSizeF available = geometry.size() - QSizeF(2 * margin(), 2 * margin());
 
     foreach (LayoutItem *l, children()) {
-        kDebug() << "testing layout item " << l << endl;
         if (l->expandingDirections() & Qt::Horizontal) {
-            expandingChildren += l;
+            expandingChildren.append(l);
         } else {
-
-            fixedChildren += l;
+            fixedChildren.append(l);
         }
     }
 
@@ -81,10 +79,12 @@
         available -= QSizeF(hint.width() + spacing(), 0.0f);
     }
 
-    qreal expandWidth = (available.width() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    qreal expandWidth = 0;
+    if (expandingChildren.count() > 0) {
+        expandWidth = (available.height() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    }
 
     foreach (LayoutItem *l, expandingChildren) {
-
         sizes.insert(indexOf(l), QSizeF(expandWidth, available.height()));
     }
 
--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/layout.cpp #693360:693361
@@ -25,27 +25,35 @@
 
 class Layout::Private
 {
-	public:
-		Private() : margin(10.0), spacing(10.0), parent(0) {}
-		~Private() {}
+    public:
+        Private(LayoutItem* parent)
+            : margin(12.0),
+              spacing(6.0),
+              parent(parent)
+        {
+        }
 
-		qreal margin;
-		qreal spacing;
+        ~Private() {}
 
-		LayoutItem *parent;
+        qreal margin;
+        qreal spacing;
+
+        LayoutItem *parent;
 };
 
 
 Layout::Layout(LayoutItem *parent)
-	: LayoutItem(),
-	  d(new Private)
+    : LayoutItem(),
+      d(new Private(parent))
 {
-	d->parent = parent;
 }
 
 Layout::~Layout()
 {
-        delete d;
+    if (parent()) {
+        parent()->setLayout(0);
+    }
+    delete d;
 }
 
 qreal Layout::margin() const
--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/layoutitem.cpp #693360:693361
@@ -17,6 +17,9 @@
  */
 
 #include "layoutitem.h"
+
+#include <KDebug>
+
 #include "layout.h"
 
 namespace Plasma
@@ -26,27 +29,30 @@
 {
     public:
         Private()
-            : layout(0)
+            : layout(0),
+              managingLayout(0)
         {
         }
 
         ~Private() {}
 
         Layout* layout;
+        Layout* managingLayout;
 };
 
 
 LayoutItem::LayoutItem()
-    : d(new Private)
+    : d(new Private())
 {
 }
 
 LayoutItem::~LayoutItem()
 {
-    if (d->layout) {
-        d->layout->removeItem(this);
+    if (d->managingLayout) {
+        d->managingLayout->removeItem(this);
     }
 
+    delete d->layout;
     delete d;
 }
 
@@ -70,15 +76,11 @@
 	return 0.0;
 }
 
-void LayoutItem::resetLayout()
-{
-    d->layout = 0;
-}
-
 void LayoutItem::setLayout(Layout* layout)
 {
-    if (d->layout) {
-        d->layout->removeItem(this);
+    if (d->layout && layout) {
+        kDebug() << k_funcinfo << " already have a layout." << endl;
+        return;
     }
 
     d->layout = layout;
@@ -89,4 +91,25 @@
     return d->layout;
 }
 
+void LayoutItem::setManagingLayout(Layout* layout)
+{
+    if (d->managingLayout) {
+        d->managingLayout->removeItem(this);
+    }
+
+    d->managingLayout = layout;
 }
+
+void LayoutItem::unsetManagingLayout(Layout* layout)
+{
+    if (d->managingLayout == layout) {
+        d->managingLayout = 0;
+    }
+}
+
+Layout* LayoutItem::managingLayout()
+{
+    return d->managingLayout;
+}
+
+}
--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/layoutitem.h #693360:693361
@@ -44,7 +44,7 @@
         /**
          * Constructor.
          */
-        LayoutItem();
+        explicit LayoutItem();
 
         /**
          * Virtual Destructor.
@@ -106,28 +106,37 @@
         virtual QSizeF sizeHint() const = 0;
 
         /**
-         * Resets the layout to 0 and doesn't notify the previous layout.
-         * Should only be used by the current layout when relinquishing the item,
-         * e.g. during layout destruction.
-         */
-        void resetLayout();
-
-        /**
-         * Sets the layout so that the LayoutItem may inform the layout of its
-         * deletion. Should only be used by the layout it is added to.
+         * Sets the layout that will manage children items
          *
-         * If the layout item is currently associated with another layout, it will
-         * first remove itself from that layout.
-         *
          * @param layout The Layout that this LayoutItem will be managed by.
          */
         void setLayout(Layout* layout);
 
         /**
-         * Returns the layout this item is currently associated with.
+         * @return the layout this item is currently associated with.
          */
         Layout* layout();
 
+        /**
+         * Sets the layout that manages this item's geometry
+         *
+         * @param layout the layout that manage this item's geometry
+         **/
+        void setManagingLayout(Layout* layout);
+
+        /**
+         * Resets the layout that manges this item's geometry if it is the
+         * currently associated layout
+         *
+         * @param layout to unset
+         **/
+        void unsetManagingLayout(Layout* layout);
+
+        /**
+         * @return the layout that manages this item's geometry, or 0 if none
+         **/
+        Layout* managingLayout();
+
     private:
         class Private;
         Private *const d;
--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/vboxlayout.cpp #693360:693361
@@ -55,10 +55,11 @@
 {
     if (!geometry.isValid() || geometry.isEmpty()) {
         kDebug() << "Invalid Geometry " << geometry << endl;
+        BoxLayout::setGeometry(geometry);
         return;
     }
 
-    kDebug() << this << " Geometry process " << geometry << " for " << children().count() << " childrens"<< endl;
+    kDebug() << this << " Geometry process " << geometry << " for " << children().count() << " children"<< endl;
 
     QList<LayoutItem *> fixedChildren;
     QList<LayoutItem *> expandingChildren;
@@ -68,9 +69,9 @@
     foreach (LayoutItem *l, children()) {
         kDebug() << "testing layout item " << l << endl;
         if (l->expandingDirections() & Qt::Vertical) {
-            expandingChildren += l;
+            expandingChildren.append(l);
         } else {
-            fixedChildren += l;
+            fixedChildren.append(l);
         }
     }
 
@@ -80,8 +81,12 @@
         available -= QSizeF(0.0, hint.height() + spacing());
     }
 
-    qreal expandHeight = (available.height() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    qreal expandHeight = 0;
 
+    if (expandingChildren.count() > 0) {
+        expandHeight = (available.height() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    }
+
     foreach (LayoutItem *l, expandingChildren) {
         sizes.insert(indexOf(l),QSizeF(available.width(), expandHeight));
     }
--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/widget.cpp #693360:693361
@@ -33,8 +33,7 @@
 {
     public:
         Private()
-            : parent(0),
-              layout(0)
+            : parent(0)
         { }
         ~Private() { }
 
@@ -43,7 +42,6 @@
         QSizeF maximumSize;
 
         Widget *parent;
-        Layout *layout;
         QList<Widget *> childList;
 };
 
@@ -139,9 +137,7 @@
 void Widget::updateGeometry()
 {
     if (layout()) {
-
         kDebug() << (void *) this << " updating geometry to " << size() << endl;
-
         layout()->setGeometry(geometry());
     }
 }
@@ -180,20 +176,6 @@
     resize(QSizeF(w, h));
 }
 
-void Widget::setLayout(Layout *l)
-{
-    if(!d->layout) {
-        d->layout = l;
-    } else {
-        kDebug() << "Widget " << this << "already has a layout!" << endl;
-    }
-}
-
-Layout *Widget::layout() const
-{
-    return d->layout;
-}
-
 Widget *Widget::parent() const
 {
     return d->parent;
--- trunk/KDE/kdebase/workspace/libs/plasma/widgets/widget.h #693360:693361
@@ -162,16 +162,6 @@
         void resize(qreal w, qreal h);
 
         /**
-         * Sets the Layout that will manage this Widget's childrens.
-         */
-        void setLayout(Layout *l);
-
-        /**
-         * Returns the Layout associated with this Widget.
-         */
-        Layout *layout() const;
-
-        /**
          * Returns the parent of this Widget.
          */
         Widget *parent() const;


More information about the Panel-devel mailing list