> On April 28, 2014, 11:06 a.m., Mark Gaiser wrote: > > applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp, lines 61-63 > > <https://git.reviewboard.kde.org/r/117813/diff/1/?file=268852#file268852line61> > > > > Ahh, this is tricky to follow. You are invoking the init function here > > _and_ earlier on using a singleShot timer if this translates to the > > Host::init method. If that's the case then i think you might be safe in > > removing this line and the singleshot and just call init() from the > > constructor. > > Marco Martin wrote: > no, it isn't init of Host. > this is the init() of m_taskGraphicsObject, ie the init() of the > AppletInterface of the loaded plasmoid (is something that shouldn't even be > neded in theory, but preloading the applets seems to avoid some crash > scenarios)
Right, now it makes sense. Thank you for the clarification. > On April 28, 2014, 11:06 a.m., Mark Gaiser wrote: > > applets/systemtray/plugin/host.cpp, line 120 > > <https://git.reviewboard.kde.org/r/117813/diff/1/?file=268851#file268851line120> > > > > just init(); ? > > I don't see a reason why you need a single shot timer here. > > Marco Martin wrote: > this causes the tasks to be loaded after the applet is done, in turn, > making the whole workspace ui to load faster (and task icons appearing after > when the rest is done) is a little hack that does huge difference in > perceived startup speed Sounds like a good (undocumented) reason :) > On April 28, 2014, 11:06 a.m., Mark Gaiser wrote: > > applets/systemtray/plugin/tasklistmodel.cpp, line 99 > > <https://git.reviewboard.kde.org/r/117813/diff/1/?file=268854#file268854line99> > > > > Are you sure this is ok? > > For example, if you add one row (and have none) this should translate > > to: > > beginInsertRows(QModelIndex(), 0, 1); > > > > I think you need something like: > > int startCount = (rowCount() - 1 >= 0) ? rowCount() : 0; > > beginInsertRows(QModelIndex(), startCount, startCount + 1); > > David Edmundson wrote: > beginInsertRows(QModelIndex(), 0, 1) > > is saying you are inserting 2 rows, starting at 0 ending at 1. It's an > annoying QAbstractItemModel API. Ok, makes sense now. I'm always confused by the start and end numbers. > On April 28, 2014, 11:06 a.m., Mark Gaiser wrote: > > applets/systemtray/plugin/tasklistmodel.cpp, line 103 > > <https://git.reviewboard.kde.org/r/117813/diff/1/?file=268854#file268854line103> > > > > Remove, the model will notify QML. No need to do this yourself. > > David Edmundson wrote: > I have it as a property, so I need to tell Qt the property has changed. > > I could do connect(this, SIGNAL("rowsInserted(...), this, > SIGNAL(rowCountChanged()); > I could do connect(this, SIGNAL("rowsRemoved(...), this, > SIGNAL(rowCountChanged()); > + rowsMoved + layoutChanged + modelReset.. > so I deemed this easier. As explained in person. The view will know this so the line can be removed. > On April 28, 2014, 11:06 a.m., Mark Gaiser wrote: > > applets/systemtray/plugin/tasklistmodel.cpp, line 115 > > <https://git.reviewboard.kde.org/r/117813/diff/1/?file=268854#file268854line115> > > > > Remove, the model will notify QML. No need to do this yourself. > > Marco Martin wrote: > the model does, but the exported count property needs to be notified by > hand If you keep the property, yes. But there is no need for the property to exist at all because the view knows the model count. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117813/#review56724 ----------------------------------------------------------- On April 27, 2014, 10:51 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/117813/ > ----------------------------------------------------------- > > (Updated April 27, 2014, 10:51 p.m.) > > > Review request for Plasma. > > > Repository: plasma-workspace > > > Description > ------- > > Port QQmlListProperty to QAbstractListModel. > QQmlListProperty only has a signal that the list has changed.This means when > used in a ListView every delegate has to be redone whenever a single item is > inserted or removed rather than just moved. > > Given TaskDelegate is not the simplest of things this has a performance gain, > most noticeably on startup. Also rather than sorting all items after an > insert items are inserted in the right place using qLowerBound. Now we have > the correct signals we can remove the compression, they won't add anything. > > > Other commits: > > Avoid constructing a QString for comparing, use QLatin1String for == > operators. > > Remove useless include > > Do not construct a map inside a lessThan function > > lessThan functions have to be fast. > Also Map -> Hash as we're not using order here. > > > Diffs > ----- > > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 2ef180b > applets/systemtray/package/contents/ui/PlasmoidItem.qml 0eb1687 > applets/systemtray/package/contents/ui/StatusNotifierItem.qml fc889a8 > applets/systemtray/package/contents/ui/TaskDelegate.qml 913d8f1 > applets/systemtray/package/contents/ui/TaskListDelegate.qml 5501e02 > applets/systemtray/plugin/CMakeLists.txt f6e23b4 > applets/systemtray/plugin/host.h 02c5bbe > applets/systemtray/plugin/host.cpp eafd0b6 > applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp 2b846f2 > applets/systemtray/plugin/tasklistmodel.h PRE-CREATION > applets/systemtray/plugin/tasklistmodel.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/117813/diff/ > > > Testing > ------- > > Seems to work :) > > see branch davidedmundson/faster_systray to test > > > Thanks, > > David Edmundson > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
