Review Request: KPixmapSequence: painting spinners made easy

[Christoph Feck]

On 2009-08-23 08:28:10, Sebastian Trueg wrote:
> > I am not sure if "interval" should be a property of KPixmapSequence. If setting the speed on KPixmapSequence is limiting, you could optionally have a "speed" property (as QMovie) to alter the sequence's speed in the other classes.
> > 
> > Maybe even refactor KPixmapSequence to be able to load QMovie and be usable as the "backend" for KAnimatedButton.
> 
> Sebastian Trueg wrote:
>     The speed in QMovie is a percentage of the original movie speed. We do not have an original movie speed, except if we let one fall out of the sky. 
>     IMHO it makes more sense to add support for QMovie to the overlay painter instead of the sequence. The interval could be ignored in that case or something.
> 
> Christoph Feck wrote:
>     Sorry, my comment was confusing. With the current version you would be able to set different speeds for different widgets, because each has its own "interval" property. If that property was moved to the sequence, you would lose this ability. For this case, an additional "speed" property could be added to the painter/widget classes.
>     
>     About QMovie support: I was thinking about using your new class inside KAnimatedWidget, so that the pixmap splitting logic wasn't duplicated. But KAnimatedWidget could handle the QMovie case separately; making KPixmapSequence a QObject is probably overkill. So ignore my comment about QMovie.
>     
>     This reviewboard sucks, I cannot provide diffs for proposed changes, making me look too lazy to implement my suggestions. The growing edit boxes add to the sucking experience.
> 
> Sebastian Trueg wrote:
>     so you want a sequence to have its own interval which can be modified by a relative speed in the overlay painter?

I do neither want nor need the "relative" feature, having the "interval" property just in the sequence would be fine. It is just that I thought you had intentionally added this property to the other classes to be able to change them independed from the sequence.


- Christoph


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


On 2009-08-24 12:32:48, Sebastian Trueg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1345/
> -----------------------------------------------------------
> 
> (Updated 2009-08-24 12:32:48)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Showing a spinner to indicate a work in progrss is a typical task. Gwenview does have a nice spinner when loading images. Aurelien and I extracted the code from Gwenview and molded it into three nice classes that allow to create spinners very easily in any situation. At the moment the classes are used in Gwenview and in Nepomuk.
> 
> KPixmapSequence: a simple container class that loads a sequence of pixmaps and provides the frames through a simple interface.
> KPixmapSequenceOverlayPainter: Installs an event filter to paint a KPixmapSequence onto any widget using Qt::Alignment or a relative placement.
> KPixmapSequenceWidget: A simple widget using the overlay painter to draw a spinner while the widget is visible.
> 
> We propose an addition to kdeui.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/CMakeLists.txt 1013393 
>   trunk/KDE/kdelibs/kdeui/tests/CMakeLists.txt 1013393 
>   trunk/KDE/kdelibs/kdeui/tests/kpixmapsequenceoverlaypaintertest.h PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/tests/kpixmapsequenceoverlaypaintertest.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/util/kpixmapsequence.h PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/util/kpixmapsequence.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/util/kpixmapsequenceoverlaypainter.h PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/util/kpixmapsequenceoverlaypainter.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/util/kpixmapsequencewidget.h PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/util/kpixmapsequencewidget.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1345/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sebastian
> 
>