----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.vidsolbach.de/r/296/#review284 -----------------------------------------------------------
i think the idea is a good one; and even on an arm system (float vs double) we still get over 16000 offscreen widgets this way, and if we wanted more we could put them in a two dimensional grid instead of a lineary array on the canvas. the implementation could use some tweaks perhaps though: /trunk/KDE/kdelibs/plasma/corona.cpp <http://reviewboard.vidsolbach.de/r/296/#comment235> there's no need to keep these sorted, so you could just use a QHash here.. /trunk/KDE/kdelibs/plasma/corona.cpp <http://reviewboard.vidsolbach.de/r/296/#comment236> contains? would prevent creating a default value, and is a bit clearer what's going on there. /trunk/KDE/kdelibs/plasma/corona.cpp <http://reviewboard.vidsolbach.de/r/296/#comment237> values() iterates over the list, makes a new list collection containing *all* the values, and then contains on that does a linear search over that list. oi, vey! much better: QMutableHashIterator<uint, QGraphicsWidget *> it(d->offscreenWidgets); while (it.hasNext()) { if (it.next() == widget) { it.remove(); return; } } - Aaron On 2008-12-09 11:03:40, Rob Scheepmaker wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.vidsolbach.de/r/296/ > ----------------------------------------------------------- > > (Updated 2008-12-09 11:03:40) > > > Review request for Plasma. > > > Summary > ------- > > Currently, offscreen widgets are managed by a QGraphicsGridLayout in the > topleft quadrant of the scene. This brings some issues though: > * As we know, QGL's have some known issues. Working with them can sometimes > be a little fragile, and there are workarounds in the code (in popupapplet > for example) to work around problems with them. > * Each time a widget changes size, other offscreen widgets inevitably move, > forcing all top level views to reposition and redraw themselves. When this > happens you can notice a slight lag. > * I've seen screenshots of a problem where certain widgets partly overlap in > a toplevel view. This might have been a temporary bug, or something that > happens rarely, but I think it demonstrates the fragile nature of the current > approach. > > Out of curiosity, I've tried experimenting with a different approach. With > this patch, corona will position each offscreen widget in their own reserved > sector with a size of QMAXWIDGETSIZE. This way, each widget can grow and > shrink without influencing others, and it will never require us to work > around layout bugs. After some changes to extenderitem (since it assumed it > would always be resized after moving offscreen, which was the case before), > this works quite good actually. It feels snappier while dragging extender > items around, and the code looks simpler and less hacky. > > This is a change with more impact then your avarage bugfix though, so I would > like to now what you think of this new offscreen widget implementation. > > > Diffs > ----- > > /trunk/KDE/kdelibs/plasma/corona.cpp > /trunk/KDE/kdelibs/plasma/extender.cpp > /trunk/KDE/kdelibs/plasma/extenderitem.cpp > /trunk/KDE/kdelibs/plasma/popupapplet.cpp > /trunk/KDE/kdelibs/plasma/private/extenderitem_p.h > > Diff: http://reviewboard.vidsolbach.de/r/296/diff > > > Testing > ------- > > > Thanks, > > Rob > > _______________________________________________ Plasma-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/plasma-devel
