Review Request 118769: Use RotationAnimator in BusyIndicator

[David Edmundson]

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

Ship it!


+100.
I have something I'd like you to check first though.


src/declarativeimports/plasmacomponents/qml/BusyIndicator.qml
<https://git.reviewboard.kde.org/r/118769/#comment41900>

    I'm not 100% sure this will still work as now the rotation property isn't being updated.
    
    From the docs:
    "The value of the QML property will be updated after the animation has finished. The property is not updated while the animation is running."
    
    
    We're doing this when the animation changes, which is a documentation grey area. I think it will be OK, but I'd like you to check before shipping.
    
    Start the animation then pause + start and check it resumes from the same place.


- David Edmundson


On June 15, 2014, 7:19 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118769/
> -----------------------------------------------------------
> 
> (Updated June 15, 2014, 7:19 p.m.)
> 
> 
> Review request for Plasma, David Edmundson, Elias Probst, and Jan Grulich.
> 
> 
> Bugs: 311799 and 336274
>     http://bugs.kde.org/show_bug.cgi?id=311799
>     http://bugs.kde.org/show_bug.cgi?id=336274
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Use RotationAnimator [1] for the BusyIndicator animation which operates directly on the scenegraph.
> 
> This helped reducing the massive plasmashell CPU usage during file copying (ie. notifications spinning). The results varied greatly but it can't hurt to use the Animator nonetheless since it doesn't propagate the new rotation angle constantly.
> 
> [1] http://qt-project.org/doc/qt-5/qml-qtquick-rotationanimator.html
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/plasmacomponents/qml/BusyIndicator.qml fafd031 
> 
> Diff: https://git.reviewboard.kde.org/r/118769/diff/
> 
> 
> Testing
> -------
> 
> Put notification in panel and systray, requested a view from JobViewServer, ran top. Tried with different configurations, such as LIBGL_ALWAYS_SOFTWARE=1 or QML_FORCE_THREADED_RENDERER=1. Results varied but generally indicated a CPU relief.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140615/591b619a/attachment.html>