Review Request: frame plasmoid: image load and scaling with threads

[Davide Bettio]

> On 2010-03-08 11:47:49, Sebastian Kügler wrote:
> > I'm not sure about this patch:
> > 
> > - The loading of images in a thread is nice
> > - The "loads huge picture and keeps them in mem" is not changed (in fact, it's worse now since QImage forces you to keep them in system memory, whereas QPixmaps can be stored on the graphics card)
> > - It basically disables hardware acceleration of the image scaling, QImage vs. QPixmap
> > 
> > With QPixmaps, we can blit into a smaller rect which is very fast, with QImage, we have to do the scaling in software and then convert it to a QPixmap to draw it. This happens on every resize, so it's a hot path and blocks direct user interaction. In my tests on different hardware (ATI, Intel, NVidia), the QPixmap based approach "felt" faster (smoother, more frames per second while resizing). The performance here depends on the painting backend used (image vs raster vs native) though, and thus per driver.
> > 
> > Maybe we can only do the image loading in a thread (since that can potentially take quite long for large images), and keep a smaller copy (screen resolution?) stored in a QPixmap which is then used to resize. (If a size larger than our screen resolution is required, we can on-demand load the full image (40000x30000px if you wish ;)) and scale from that. (Rationale upscaling == bad, downscaling == OK, saving mem == good)
> > 
> > The patch as it's now is not good enough IMO.

We must do software scaling because loading a 4000x3000 pixmap into X11 and asking X11 to scale the pixmap is a bad idea and it freezes the whole X11 environment including plasma and kwin for at least a couple of seconds (that happpens on a recent nvidia quadro, and I wonder what happens on an ATI video card powered with buggy drivers).
With this patch during resize paintInterface is called and the cached QPixmap is used, in the meantime another thread with a completely async behaviour scales the image.

> The "loads huge picture and keeps them in mem" is not changed (in fact, it's worse now since QImage forces you to keep them in system memory, whereas QPixmaps can be stored on the graphics card)
Do you know how to load our image on graphics card memory without blocking everything? (don't forget that 4000x3000 px images are about 45 mb of raw ARGB data) And moreover what's the point of using video card memory since system memory is larger (think about laptops too) when moreover we don't need to use any video card feature? I can try anyway to reduce the huge in memory image footprint, but honestly I think that I should commit this patch before and then I can solve that problem. Anyway a solution might be to start after N seconds a thread with really low priority that replaces the huge in memory image with a screen-resolution-one (doing this task during image load and display process might be a bad idea).

> Maybe we can only do the image loading in a thread (since that can potentially take quite long for large images).
This will prevent to freeze plasma during image loading but then X11 will be freezed anyway.

I might try to improve also the behaviour using a thread with a bit lower priority than the default priority.

Anyway don't forget that we are using software scaling also for wallpapers since KDE 4.0 and it works nicely.


- Davide


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


On 2010-03-08 02:27:47, Davide Bettio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3162/
> -----------------------------------------------------------
> 
> (Updated 2010-03-08 02:27:47)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> First of all: **This is a preview of the patch** I'm still working on it but I need some advices.
> Picture Frame does 2 really bad things (when used with huge images):
> * it loads huge images (in my case 4000x3000 pixels) into X11 pixmaps
> * plasma is blocked while the image is scaled
> As side effect also X11 freezes.
> 
> This patch solves both issues using threads.
> 
> I think that we should remove "smooth scaling" option from our configuration UI. what do you think?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeplasma-addons/applets/frame/CMakeLists.txt 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/appearanceSettings.ui 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/configdialog.h 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/configdialog.cpp 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/frame.h 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/imageloader.cpp PRE-CREATION 
>   /trunk/KDE/kdeplasma-addons/applets/frame/imagescaler.cpp PRE-CREATION 
>   /trunk/KDE/kdeplasma-addons/applets/frame/picture.h 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/picture.cpp 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/slideshow.h 1093624 
>   /trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp 1093624 
> 
> Diff: http://reviewboard.kde.org/r/3162/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Davide
> 
>