Review Request: Plasma Mediacenter: use new plasma animations, fix some bugs

[Christophe Olinger]

Wow, thanks. My 4th accepted patch. Yay! I'll try to commit this
evening or latest tomorrow.

Concerning the bugs, I can collect them on our techbase page for now.

Cheers,

Christophe




On Wed, May 26, 2010 at 11:15 PM, Alessandro Diaferia
<alediaferia at gmail.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4152/#review5876
> -----------------------------------------------------------
>
> Ship it!
>
>
> This is a good patch: the move to new animator API is a really good thing and i wanted this to be done as soon as possible. I still didn't have a look at the video widget size bug as i still don't have videos on my new installation. Anyway i'd say go for this patch and i'll try to fix the bug in the next patch i'm working on.
>
> Also i'd like you to take note of such bugs you encounter as we don't have a component on bko still (i probably should ask for one).
>
> Here follows little issues i noticed.
>
>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp
> <http://reviewboard.kde.org/r/4152/#comment5502>
>
>    please add a little comment and specify we're returning to browsing mode as soon as slide-show is finished
>
>
>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp
> <http://reviewboard.kde.org/r/4152/#comment5503>
>
>    please prefer member accessors to array-type ones: m_pictureMedias.first();
>
>
>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp
> <http://reviewboard.kde.org/r/4152/#comment5504>
>
>    you can put this as a static const qreal MARGINFACTOR = 0.2; at the beginning of the code
>
>
>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp
> <http://reviewboard.kde.org/r/4152/#comment5505>
>
>    this line doesn't do anything? you probably want to do m_controlAutohide = !set;
>
>
> - Alessandro
>
>
> On 2010-05-26 08:19:03, Christophe Olinger wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviewboard.kde.org/r/4152/
>> -----------------------------------------------------------
>>
>> (Updated 2010-05-26 08:19:03)
>>
>>
>> Review request for Plasma and Alessandro Diaferia.
>>
>>
>> Summary
>> -------
>>
>> This patch covers the following:
>>
>> Features:
>> - Use new plasma animation classes
>> - In floating mode, fade player between pictures (avoids painting artefacts)
>> - Return to browser after showing last picture of a slideshow
>> - Start at first picture again after slideshow has been played and is started again
>> - Background states are handled onExit of each state by checking the currentPlayback state
>> - Renamed layoutZone enum members
>>
>> Bugfixes:
>> - Fixed bug where video player would be fully expanded horizontally, but not vertically
>> - Slideshow now works correctly
>> - Bottom Panel should be correctly placed now at startup
>>
>> Evil bugs:
>> - Video player always remains at 1024 x 600 :-/. Alessandro, could you have a look please?
>> - Qt 4.7 beta 1 does not solve the blue skin bug when playing videos
>> - Layout errors of bottom bar (will fix that in the next days)
>>
>>
>> Diffs
>> -----
>>
>>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.h 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/private/mediahandler.cpp 1130736
>>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp 1130736
>>
>> Diff: http://reviewboard.kde.org/r/4152/diff
>>
>>
>> Testing
>> -------
>>
>>
>> Thanks,
>>
>> Christophe
>>
>>
>
>