Review Request: Convert libplasma to new qt4.4 technologies.

Dan Meltzer parallelgrapefruit at gmail.com
Wed Feb 27 20:33:48 CET 2008 


> On 2008-02-24 21:24:04, Aaron Seigo wrote:
> > wow.. big patch.
> > 
> > the crashing is going to be a show stopper of sorts. we'll need the fixes in Qt to make those things go away.
> > 
> > is this in a branch in svn somewhere? if not, one should be made and this patch applied. i'm not sure i'm comfortable with merging it into trunk with known (and easy to trigger) crashes.
> > 
> > generally the patch looks ok, though i'm sure we'll come across a number of issues as we pound on it in trunk once it is in.
> > 
> > i'll try and ping bibr tomorrow (monday) to get some status reports on our issues.
> >

Update: Using the qt4.4 beta the crash no longer occurs.  

The resizing issue still exists, but icons on the taskbar do not get smaller than their minSize, which is good.

I'll see about getting this into a branch over the next day or two, currently have it in git here.


> On 2008-02-24 21:24:04, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/libs/plasma/containment.cpp, lines 540-541
> > <http://mattr.info/r/187/diff/1/#file486line540>
> >
> >     this won't work, since you'll be removing the applet that is now at index i after inserting the applet that was at i. so the order of the applets is likely to change and it will remove the wrong applet.
> >     
> >     in fact, this looks like a very poor idea in the original code. the applet should be put in the right place to begin with. imo: just remove this whole hack and whatever it was fixing (hooray for no comments!) we can fix properly this time around.

Okay, removed this.

Theres a crash after removing an applet and then trying to add one however.  This is the backtrace.

0xb6e95dc7 in QGraphicsLayoutItemPrivate::effectiveSizeHints (this=0xb76f1f81, 
    constraint=@0xbfb75230) at graphicsview/qgraphicslayoutitem.cpp:134
134             cachedSizeHints[i] = constraint;
(gdb) bt
#0  0xb6e95dc7 in QGraphicsLayoutItemPrivate::effectiveSizeHints (
    this=0xb76f1f81, constraint=@0xbfb75230)
    at graphicsview/qgraphicslayoutitem.cpp:134
#1  0xb6e96148 in QGraphicsLayoutItem::effectiveSizeHint (this=0x8292fa0, 
    which=Qt::PreferredSize, constraint=@0xbfb75230)
    at graphicsview/qgraphicslayoutitem.cpp:545
#2  0xb6e9ef7a in QGridLayoutItem::sizeHint (this=0x8281c30, 
    which=Qt::PreferredSize, constraint=@0xbfb75230)
    at graphicsview/qgridlayoutengine.cpp:541
#3  0xb6e9f19a in QGridLayoutItem::box (this=0x8281c30, 
    orientation=Qt::Horizontal, constraint=-1)
    at graphicsview/qgridlayoutengine.cpp:552
#4  0xb6ea1ce4 in QGridLayoutEngine::fillRowData (this=0x8276c3c, 
    rowData=0x8276ca0, styleInfo=@0xbfb7576c, orientation=Qt::Horizontal)
    at graphicsview/qgridlayoutengine.cpp:1238
#5  0xb6ea29c2 in QGridLayoutEngine::ensureColumnAndRowData (this=0x8276c3c, 
    styleInfo=@0xbfb7576c) at graphicsview/qgridlayoutengine.cpp:1370
#6  0xb6ea2b46 in QGridLayoutEngine::ensureGeometries (this=0x8276c3c, 
    styleInfo=@0xbfb7576c, size=@0xbfb75708)
    at graphicsview/qgridlayoutengine.cpp:1385
#7  0xb6ea3078 in QGridLayoutEngine::setGeometries (this=0x8276c3c, 
    styleInfo=@0xbfb7576c, contentsGeometry=@0xbfb75748)
    at graphicsview/qgridlayoutengine.cpp:913
#8  0xb6e96b31 in QGraphicsLinearLayout::setGeometry (this=0x8277ed0, 
    rect=@0xbfb757c8) at graphicsview/qgraphicslinearlayout.cpp:485
#9  0xb6e94f81 in QGraphicsLayout::activate (this=0x8277ed0)
    at graphicsview/qgraphicslayout.cpp:234
#10 0xb6e95026 in QGraphicsLayout::widgetEvent (this=0x8277ed0, e=0x8532190)
    at graphicsview/qgraphicslayout.cpp:295
#11 0xb6e993d9 in QGraphicsWidget::event (this=0x828c2b0, event=0x8532190)
    at graphicsview/qgraphicswidget.cpp:1065
#12 0xb68f19bf in QApplicationPrivate::notify_helper (this=0x80784e0, 
    receiver=0x828c2b0, e=0x8532190) at kernel/qapplication.cpp:3735
#13 0xb68f6639 in QApplication::notify (this=0x8072c08, receiver=0x828c2b0, 
    e=0x8532190) at kernel/qapplication.cpp:3329
#14 0xb794afbf in KApplication::notify (this=0x8072c08, receiver=0x828c2b0, 
    event=0x8532190)
    at /home/hydrogen/kde/src/KDE/kdelibs/kdeui/kernel/kapplication.cpp:311


This doesn't make much sense to me, gdb is showing the following... 

(gdb) print constraint
$2 = (const QSizeF &) @0xbfb75230: {wd = -1, ht = -1}
(gdb) print cachedSizeHints[0]
$3 = {wd = 4.4391375576256985e+251, ht = 1.7971271811430896e-90}
(gdb) print cachedSizeHints[1]
$4 = {wd = 2.0852224014847319e+64, ht = 2.5770051646570503e+161}
(gdb) print cachedSizeHints[2]
$5 = {wd = 1.4599639834608835e-319, ht = 0}
(gdb) print cachedSizeHints[3]
$6 = {wd = 3.7392654333957796e+69, ht = 8.8442217437345597e+247}
(gdb) print cachedSizeHints[4]
$7 = {wd = 5.7311614917584599e-322, ht = 0}
(gdb) print cachedSizeHints[5]
$8 = {wd = 154368.126953125, ht = 1.8796052680814099e-309}
(gdb) print cachedSizeHints[6]
$9 = {wd = 5.369298307540021e+173, ht = 3.2506712399343566e+284}
(gdb) print cachedSizeHints[7]
$10 = {wd = 10122961056.000002, ht = -1.1264355535657189e+34}
(gdb) print cachedSizeHints[8]
$11 = {wd = 3.5196148081908233e+178, ht = 2.1308410713987442e+289}


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://mattr.info/r/187/#review188
-----------------------------------------------------------


On 2008-02-22 22:57:49, Dan Meltzer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://mattr.info/r/187/
> -----------------------------------------------------------
> 
> (Updated 2008-02-22 22:57:49)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch converts libplasma to use the new technologies found in Qt 4.4.  It does the following things:
> 
> 1) Makes Plasma::Widget descend from QGraphicsWidget:  This change allows for a lot of code simplification, widget.cpp shrinks by about 200 lines after removing redundant code.  We cannot replace Plasma::Widget with QGraphicsWidget everywhere directly, as there still are some things in widget that we need.  This patch also removes Widget::expandingDirections in favor of using sizePolicies.
> 
> 2) Removes Plasma::Layout and Plasma::LayoutItem:  These have been replaced by QGraphicsLayout and QGraphicsLayoutItem respectivly.  All layouts with the exception of boxlayout have been converted to use QGraphicsLayout.
> 
> 3) Removes Boxlayout.  QGraphicsLinearLayout replaces BoxLayout at this time.  There are a few things that boxlayout had that QGraphicsLayout does not at this point, but they are not entirly important on a wide scale I do not believe.
> 
> 4) Disables LayoutAnimator.  This is going to need to be ported to work with the new layouting concepts and classes.  When I last spoke with aseigo it sounded like LayoutAnimator needed some rethinking anyways, so I have just disabled it for now.
> 
> A few notes:  This patch is only the section of the patch applying to libplasma.  I have another equally big patch that I need to clean up that converts workspace/plasma.  I hope to post that in the next few days.  Also, there are some bugs either in my changes or in qt4.4 (I have not been able to determine which, and reguardless they appear in the applets part of the patch, not the libs.
> 
> The request here is for review of the idea and changes made, with the understanding that there may be more review necessary once the second part of the changes get posted.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/libs/plasma/CMakeLists.txt
>   trunk/KDE/kdebase/workspace/libs/plasma/applet.h
>   trunk/KDE/kdebase/workspace/libs/plasma/applet.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/containment.h
>   trunk/KDE/kdebase/workspace/libs/plasma/containment.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/corona.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/borderlayout.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/borderlayout.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/boxlayout.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/boxlayout.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/fliplayout.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/flowlayout.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/flowlayout.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/freelayout.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/freelayout.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/layout.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/layout.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/layoutitem.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/layoutitem.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/nodelayout.h
>   trunk/KDE/kdebase/workspace/libs/plasma/layouts/nodelayout.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/uiloader.h
>   trunk/KDE/kdebase/workspace/libs/plasma/uiloader.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/icon.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/icon.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/label.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/label.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/lineedit.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/lineedit.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/meter.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/meter.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/progressbar.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/progressbar.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/pushbutton.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/pushbutton.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/rectangle.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/rectangle.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/signalplotter.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/signalplotter.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/tests/testLayouts.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/tests/testProgressBar.cpp
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/widget.h
>   trunk/KDE/kdebase/workspace/libs/plasma/widgets/widget.cpp
> 
> Diff: http://mattr.info/r/187/diff
> 
> 
> Testing
> -------
> 
> Been using this patch for the past few days. There are still a number of crashes in what appears to be qt4.4 code, but these changes seem sound.
> 
> 
> Thanks,
> 
> Dan
> 
>

More information about the Panel-devel mailing list