----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117813/#review56724 -----------------------------------------------------------
applets/systemtray/plugin/host.cpp <https://git.reviewboard.kde.org/r/117813/#comment39587> Looks like it can all be removed. applets/systemtray/plugin/host.cpp <https://git.reviewboard.kde.org/r/117813/#comment39588> just init(); ? I don't see a reason why you need a single shot timer here. applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp <https://git.reviewboard.kde.org/r/117813/#comment39599> 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. applets/systemtray/plugin/tasklistmodel.h <https://git.reviewboard.kde.org/r/117813/#comment39596> Not needed, the model will notify QML when the row count changes. applets/systemtray/plugin/tasklistmodel.cpp <https://git.reviewboard.kde.org/r/117813/#comment39593> Perhaps try do these test before calling this function via qLowerBound. It would certainly be faster, but also more complicated so i don't know if it's worth it. applets/systemtray/plugin/tasklistmodel.cpp <https://git.reviewboard.kde.org/r/117813/#comment39594> I don't think you plan on changing this at runtime - ever. You might want to change this to a const and use initializer lists (a C++11 feature). It works like this: const QHash<int, QByteArray> roleNames { {Qt::UserRole, "name"}, ..., ...}; applets/systemtray/plugin/tasklistmodel.cpp <https://git.reviewboard.kde.org/r/117813/#comment39595> 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); applets/systemtray/plugin/tasklistmodel.cpp <https://git.reviewboard.kde.org/r/117813/#comment39597> Remove, the model will notify QML. No need to do this yourself. applets/systemtray/plugin/tasklistmodel.cpp <https://git.reviewboard.kde.org/r/117813/#comment39598> Remove, the model will notify QML. No need to do this yourself. - Mark Gaiser 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