Review Request: Plasma + Qt Kinetic GSoC Project - Attempt 2

[Adenilson Cavalcanti]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1512/#review2647
-----------------------------------------------------------



/trunk/KDE/kdelibs/plasma/animations/animation.h
<http://reviewboard.kde.org/r/1512/#comment1966>

    There are 2 cases where the interface doesn't cover requirements for some more complex animations. E.g. for rotation (we need to known the rotation angle along with the axis and might to known what is the widget behind the animated one, to do the flipping correctly) and pulse (where we need a copy of the animated object). Those parameters should have an interface in the upper class if we implement a factory, so the user can set the parameters using a pointer to Animation instead of the derived class.



/trunk/KDE/kdelibs/plasma/animations/animation.h
<http://reviewboard.kde.org/r/1512/#comment1965>

    See my comment bellow.



/trunk/KDE/kdelibs/plasma/animations/animation.h
<http://reviewboard.kde.org/r/1512/#comment1964>

    It should return a QAbstractAnimation so it covers the case that the animation is complex and it really is a QAnimationGroup. Since later the ::start() method will be called, it should work with both single property animation objects and composed animation groups (where more than just 1 property is transformed).


- Adenilson


On 2009-10-13 15:06:04, makmanalp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1512/
> -----------------------------------------------------------
> 
> (Updated 2009-10-13 15:06:04)
> 
> 
> Review request for Plasma, Aaron Seigo, Alexis Menard, and Adenilson Cavalcanti.
> 
> 
> Summary
> -------
> 
> Okay, this is the "oh cr*p Alexis is leaving Tokamak" version, which means I'm missing a couple of stuff but I want to get it in in time. Sorry about this, but I have school now so I can't give my attention to this as much as I could. What's missing is:
> 
> - Didn't dptr the private members yet because I got stuck trying to figure out how I'd override render() (which would be in AnimationPrivate) in the subclasses such as Grow etc. I guess I could have a function with the same name in AnimationDerivedPrivate but it wouldn't exactly be overriding?
> - Didn't add factory methods for each animation type yet.
> - Didn't map the existing enums to new stuff (eg AppearAnimation etc)
> - Didn't figure out checking animation status / stop and pause etc.
> 
> Now that that's over, here's the good news:
> 
> - Oodles of general tidying up:
>     - Moved everything into animations/ so it's neater.
>     - Added license headers to everything.
>     - Tidied up includes.
>     - Split each class into its own file.
>     - Added missing getters.
>     - Consted as much as I can.
>     - No more unnecessary this->es.
> - BaseAnimationElement is now AbstractAnimation.
> - Moved the duration variable (m_duration) down the hierarchy into Animation because setting duration for groups is meaningless.
> - setObject() is called setWidget() now and is moved up the hierarchy into AbstractAnimation since it was doing the same for both Animation and AnimationGroup.
> - render() is no longer what it used to be, it's now a protected pure virtual function in Animation that the subclasses must override. getQtAnimation() does some common checking and then calls the render() of that subclass. getQtAnimation() is the exposed interface.
> - getQtAnimation() takes a parent object to pass to Animations and AnimationGroups to use when generating the QPropertyAnimations and QAnimationGroups. When getQtAnimation() is used on AnimationGroups, the generated sub-animations are now owned by the generated group.
> - The MovementDirection enum is now called AnimationDirection and is in plasma.h
> 
> This is the essence of all changes. Documentation might be lagging behind, I'll try to clean it up later on and comments on what's wrong are much appreciated.
> 
> Thanks a lot in advance!
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1018731 
>   /trunk/KDE/kdelibs/plasma/animations/abstractanimation.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/abstractanimation.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/animation.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/animation.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/animationgroup.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/animationgroup.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/expand.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/expand.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/fade.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/fade.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/grow.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/grow.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/slide.h PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animations/slide.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/animator.h 1018731 
>   /trunk/KDE/kdelibs/plasma/animator.cpp 1018731 
>   /trunk/KDE/kdelibs/plasma/deprecated/animator.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/plasma/plasma.h 1018731 
> 
> Diff: http://reviewboard.kde.org/r/1512/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> makmanalp
> 
>