[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: Plasma Mediacenter: use new plasma animations,
From: Alessandro Diaferia <alediaferia () gmail ! com>
Date: 2010-05-27 9:32:47
Message-ID: AANLkTimxFbvQj3zsq62tpHA75Hjlv2Bu9YCxDgdFeHcj () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
2010/5/27 Christophe Olinger <olingerc@binarylooks.com>
> 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.
>
Yeah, great idea! So we give them more visibility among eventual other
contributors.
I'd likely try to figure out which of them can represent junior jobs for new
contributors :-)
>
> Cheers,
>
Ciao :-)
>
> Christophe
>
>
>
>
> On Wed, May 26, 2010 at 11:15 PM, Alessandro Diaferia
> <alediaferia@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
> >>
> >>
> >
> >
>
--
Alessandro Diaferia
KDE Developer
KDE e.V. member
[Attachment #5 (text/html)]
<br><br><div class="gmail_quote">2010/5/27 Christophe Olinger <span dir="ltr"><<a \
href="mailto:olingerc@binarylooks.com">olingerc@binarylooks.com</a>></span><br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex;"> Wow, thanks. My 4th accepted patch. Yay! I'll try to \
commit this<br> evening or latest tomorrow.<br>
<br>
Concerning the bugs, I can collect them on our techbase page for \
now.<br></blockquote><div> </div><div>Yeah, great idea! So we give them more \
visibility among eventual other contributors.</div><div>I'd likely try to figure \
out which of them can represent junior jobs for new contributors :-) </div> \
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex;"> <br>
Cheers,<br></blockquote><div><br></div><div>Ciao :-) </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex;"> <font color="#888888"><br>
Christophe<br>
</font><div><div></div><div class="h5"><br>
<br>
<br>
<br>
On Wed, May 26, 2010 at 11:15 PM, Alessandro Diaferia<br>
<<a href="mailto:alediaferia@gmail.com">alediaferia@gmail.com</a>> wrote:<br>
><br>
> -----------------------------------------------------------<br>
> This is an automatically generated e-mail. To reply, visit:<br>
> <a href="http://reviewboard.kde.org/r/4152/#review5876" \
target="_blank">http://reviewboard.kde.org/r/4152/#review5876</a><br> > \
-----------------------------------------------------------<br> ><br>
> Ship it!<br>
><br>
><br>
> 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.<br>
><br>
> 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).<br> ><br>
> Here follows little issues i noticed.<br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5502" \
target="_blank">http://reviewboard.kde.org/r/4152/#comment5502</a>><br> ><br>
> please add a little comment and specify we're returning to browsing \
mode as soon as slide-show is finished<br> ><br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5503" \
target="_blank">http://reviewboard.kde.org/r/4152/#comment5503</a>><br> ><br>
> please prefer member accessors to array-type ones: \
m_pictureMedias.first();<br> ><br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5504" \
target="_blank">http://reviewboard.kde.org/r/4152/#comment5504</a>><br> ><br>
> you can put this as a static const qreal MARGINFACTOR = 0.2; at the \
beginning of the code<br> ><br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5505" \
target="_blank">http://reviewboard.kde.org/r/4152/#comment5505</a>><br> ><br>
> this line doesn't do anything? you probably want to do \
m_controlAutohide = !set;<br> ><br>
><br>
> - Alessandro<br>
><br>
><br>
> On 2010-05-26 08:19:03, Christophe Olinger wrote:<br>
>><br>
>> -----------------------------------------------------------<br>
>> This is an automatically generated e-mail. To reply, visit:<br>
>> <a href="http://reviewboard.kde.org/r/4152/" \
target="_blank">http://reviewboard.kde.org/r/4152/</a><br> >> \
-----------------------------------------------------------<br> >><br>
>> (Updated 2010-05-26 08:19:03)<br>
>><br>
>><br>
>> Review request for Plasma and Alessandro Diaferia.<br>
>><br>
>><br>
>> Summary<br>
>> -------<br>
>><br>
>> This patch covers the following:<br>
>><br>
>> Features:<br>
>> - Use new plasma animation classes<br>
>> - In floating mode, fade player between pictures (avoids painting \
artefacts)<br> >> - Return to browser after showing last picture of a \
slideshow<br> >> - Start at first picture again after slideshow has been played \
and is started again<br> >> - Background states are handled onExit of each \
state by checking the currentPlayback state<br> >> - Renamed layoutZone enum \
members<br> >><br>
>> Bugfixes:<br>
>> - Fixed bug where video player would be fully expanded horizontally, but not \
vertically<br> >> - Slideshow now works correctly<br>
>> - Bottom Panel should be correctly placed now at startup<br>
>><br>
>> Evil bugs:<br>
>> - Video player always remains at 1024 x 600 :-/. Alessandro, could you have \
a look please?<br> >> - Qt 4.7 beta 1 does not solve the blue skin bug when \
playing videos<br> >> - Layout errors of bottom bar (will fix that in the next \
days)<br> >><br>
>><br>
>> Diffs<br>
>> -----<br>
>><br>
>> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.h \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/private/mediahandler.cpp \
1130736<br> >> \
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp \
1130736<br> >><br>
>> Diff: <a href="http://reviewboard.kde.org/r/4152/diff" \
target="_blank">http://reviewboard.kde.org/r/4152/diff</a><br> >><br>
>><br>
>> Testing<br>
>> -------<br>
>><br>
>><br>
>> Thanks,<br>
>><br>
>> Christophe<br>
>><br>
>><br>
><br>
><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Alessandro \
Diaferia<br>KDE Developer<br>KDE e.V. member<br><br>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic