[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: Convert libplasma to new qt4.4 technologies.
From: Dan Meltzer <parallelgrapefruit () gmail ! com>
Date: 2008-02-27 19:33:48
Message-ID: 20080227193348.8402.14944 () localhost
[Download RAW message or body]
> 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
>
>
_______________________________________________
Panel-devel mailing list
Panel-devel@kde.org
https://mail.kde.org/mailman/listinfo/panel-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic