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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to