Review Request 117813: Make the system tray faster

2014-04-27 Thread David Edmundson 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/
---

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

Re: Review Request 117813: Make the system tray faster

2014-05-02 Thread Marco Martin 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review57097
---


what's the status of this?

- Marco Martin


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

Re: Review Request 117813: Make the system tray faster

2014-05-02 Thread David Edmundson 


> On May 2, 2014, 8:39 a.m., Marco Martin wrote:
> > what's the status of this?

I have a crash when combined with Martin's new notifications.

Fixing first.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review57097
---


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

Re: Review Request 117813: Make the system tray faster

2014-05-02 Thread Martin Klapetek 


> On May 2, 2014, 10:39 a.m., Marco Martin wrote:
> > what's the status of this?
> 
> David Edmundson wrote:
> I have a crash when combined with Martin's new notifications.
> 
> Fixing first.
>

Speaking of which https://git.reviewboard.kde.org/r/117903/ ;)


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review57097
---


On April 28, 2014, 12:51 a.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117813/
> ---
> 
> (Updated April 28, 2014, 12:51 a.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

Re: Review Request 117813: Make the system tray faster

2014-05-02 Thread David Edmundson 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/
---

(Updated May 2, 2014, 4:10 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 (updated)
-

  applets/systemtray/package/contents/ui/CompactRepresentation.qml 01308e7 
  applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 646b908 
  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/package/contents/ui/main.qml d1a6851 
  applets/systemtray/plugin/CMakeLists.txt f6e23b4 
  applets/systemtray/plugin/host.h 02c5bbe 
  applets/systemtray/plugin/host.cpp eafd0b6 
  applets/systemtray/plugin/task.h 68dcd12 
  applets/systemtray/plugin/task.cpp 1f8e3ca 
  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

Re: Review Request 117813: Make the system tray faster

2014-05-05 Thread Martin Klapetek 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review57296
---

Ship it!



applets/systemtray/package/contents/ui/StatusNotifierItem.qml


This could be removed, sebas said



applets/systemtray/package/contents/ui/TaskDelegate.qml


This is used (remove the fixme)



applets/systemtray/plugin/host.cpp


Remove this please, it adds nothing useful to the output


- Martin Klapetek


On May 2, 2014, 6:10 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117813/
> ---
> 
> (Updated May 2, 2014, 6:10 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/CompactRepresentation.qml 01308e7 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 646b908 
>   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/package/contents/ui/main.qml d1a6851 
>   applets/systemtray/plugin/CMakeLists.txt f6e23b4 
>   applets/systemtray/plugin/host.h 02c5bbe 
>   applets/systemtray/plugin/host.cpp eafd0b6 
>   applets/systemtray/plugin/task.h 68dcd12 
>   applets/systemtray/plugin/task.cpp 1f8e3ca 
>   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

Re: Review Request 117813: Make the system tray faster

2014-05-06 Thread David Edmundson 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/
---

(Updated May 6, 2014, 11:03 a.m.)


Status
--

This change has been marked as submitted.


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/CompactRepresentation.qml 01308e7 
  applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 646b908 
  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/package/contents/ui/main.qml d1a6851 
  applets/systemtray/plugin/CMakeLists.txt f6e23b4 
  applets/systemtray/plugin/host.h 02c5bbe 
  applets/systemtray/plugin/host.cpp eafd0b6 
  applets/systemtray/plugin/task.h 68dcd12 
  applets/systemtray/plugin/task.cpp 1f8e3ca 
  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

Re: Review Request 117813: Make the system tray faster

2014-05-06 Thread Commit Hook 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review57400
---


This review has been submitted with commit 
45eaffacab8f4badf27d82ec8134d51f748f375f by David Edmundson to branch master.

- Commit Hook


On May 2, 2014, 4:10 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117813/
> ---
> 
> (Updated May 2, 2014, 4:10 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/CompactRepresentation.qml 01308e7 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 646b908 
>   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/package/contents/ui/main.qml d1a6851 
>   applets/systemtray/plugin/CMakeLists.txt f6e23b4 
>   applets/systemtray/plugin/host.h 02c5bbe 
>   applets/systemtray/plugin/host.cpp eafd0b6 
>   applets/systemtray/plugin/task.h 68dcd12 
>   applets/systemtray/plugin/task.cpp 1f8e3ca 
>   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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread Aleix Pol Gonzalez 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review56718
---


All in all, looks like a good improvement to me.


applets/systemtray/package/contents/ui/TaskDelegate.qml


!



applets/systemtray/package/contents/ui/TaskDelegate.qml


!


- Aleix Pol Gonzalez


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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread Martin Klapetek 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review56714
---


Overall +1 from me


applets/systemtray/package/contents/ui/TaskDelegate.qml


No.



applets/systemtray/plugin/host.cpp


This reminds me - didn't we decide at the Jan sprint that the popup (the 
"hidden" here?) should contain /all/ the systray plasmoids?

Also sorry for hijacking the review, we could just fix it at once?



applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp


Whitespace leftover it seems



applets/systemtray/plugin/tasklistmodel.h


remove ;)



applets/systemtray/plugin/tasklistmodel.cpp


const QModelIndex& index -> const QModelIndex &index



applets/systemtray/plugin/tasklistmodel.cpp


const QModelIndex& parent -> const QModelIndex &parent

also Q_UNUSED(parent) to spare compiler warning



applets/systemtray/plugin/tasklistmodel.cpp


Task* task -> Task *task



applets/systemtray/plugin/tasklistmodel.cpp


Task* task -> Task *task


- Martin Klapetek


On April 28, 2014, 12:51 a.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117813/
> ---
> 
> (Updated April 28, 2014, 12:51 a.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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread Marco Martin 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review56721
---


very good direction, just a couple of minor issues


applets/systemtray/plugin/host.cpp


did you try disabling and then reenabling a category? (didn't try the 
patch, just not sure looking at it how correctly reenables stuff that doesn't 
keep track of?)



applets/systemtray/plugin/tasklistmodel.h


as a convention, models exported in qml export their size as "count" (just 
because the primitive ListModel calls it that way)


- Marco Martin


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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread Sebastian Kügler 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review56719
---


I like muchos. Haven't tested it yet, as I'm still resurrecting my builds, so 
I'm ok with shipping it, but don't feel comfortable "ship it"ing it myself. :)

Good work!


applets/systemtray/package/contents/ui/StatusNotifierItem.qml


The answer is easy: It also wastes memory! ;-)

More seriously, I think it's a leftover from the old implementation and can 
be removed throughout.



applets/systemtray/package/contents/ui/TaskDelegate.qml


It is used to determine whether we're a delegate in the hidden section (so 
with text), or in the panel section (square).



applets/systemtray/plugin/host.cpp


no spaces around Task



applets/systemtray/plugin/tasklistmodel.h


"a model representing items for the system tray"?



applets/systemtray/plugin/tasklistmodel.cpp


see above


- Sebastian Kügler


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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread Mark Gaiser 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117813/#review56724
---



applets/systemtray/plugin/host.cpp


Looks like it can all be removed.



applets/systemtray/plugin/host.cpp


just init(); ?
I don't see a reason why you need a single shot timer here.



applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp


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


Not needed, the model will notify QML when the row count changes.



applets/systemtray/plugin/tasklistmodel.cpp


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


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 roleNames { {Qt::UserRole, "name"}, ..., ...};



applets/systemtray/plugin/tasklistmodel.cpp


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


Remove, the model will notify QML. No need to do this yourself.



applets/systemtray/plugin/tasklistmodel.cpp


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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread David Edmundson 


> On April 28, 2014, 11:06 a.m., Mark Gaiser wrote:
> > applets/systemtray/plugin/tasklistmodel.cpp, line 103
> > 
> >
> > Remove, the model will notify QML. No need to do this yourself.

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.


> On April 28, 2014, 11:06 a.m., Mark Gaiser wrote:
> > applets/systemtray/plugin/tasklistmodel.cpp, line 99
> > 
> >
> > 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);

beginInsertRows(QModelIndex(), 0, 1)

is saying you are inserting 2 rows, starting at 0 ending at 1. It's an annoying 
QAbstractItemModel API.


- David


---
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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread Marco Martin 


> On April 28, 2014, 11:06 a.m., Mark Gaiser wrote:
> > applets/systemtray/plugin/host.cpp, line 120
> > 
> >
> > just init(); ?
> > I don't see a reason why you need a single shot timer here.

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


> On April 28, 2014, 11:06 a.m., Mark Gaiser wrote:
> > applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp, lines 61-63
> > 
> >
> > 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.

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)


> On April 28, 2014, 11:06 a.m., Mark Gaiser wrote:
> > applets/systemtray/plugin/tasklistmodel.cpp, line 115
> > 
> >
> > Remove, the model will notify QML. No need to do this yourself.

the model does, but the exported count property needs to be notified by hand


- Marco


---
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

Re: Review Request 117813: Make the system tray faster

2014-04-28 Thread Mark Gaiser 


> On April 28, 2014, 11:06 a.m., Mark Gaiser wrote:
> > applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp, lines 61-63
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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 funct