Review Request: Improve rendering of Plasma Image-Wallpaper list items using QTextDocument to layout the text

[Aaron Seigo]

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

Ship it!


looks much better. i don't think we need to worry about the performance of that code in this particular case.

i do wonder, long term, if we really should stick with a vertical list in this case or if an icon grid would be nicer as you could view more items at once. we could keep the Delegate, but move the painting of the item from on the right to below the icon. i've just played with this and it looks trivial to do. if you can commit your patch here, then i'll continue to experiment with this.

- Aaron


On 2010-03-28 16:36:48, Darío Andrés wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3417/
> -----------------------------------------------------------
> 
> (Updated 2010-03-28 16:36:48)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Update BackgroundDelegate to use QTextDocument in order to improve text layouting.
> Now, all the text is properly centered and long titles are wordwrapped correctly, maximizing the space usage.
> 
> Compare http://bugsfiles.kde.org/attachment.cgi?id=42291 (from the bug report) and the attached screenshot
> 
> I also cached some values to avoid a lot of calculations (useless as the ratio is not going to change)
> - Should some other values be cached too ? (the smallest font size?)
> 
> 
> This addresses bug 232348.
>     https://bugs.kde.org/show_bug.cgi?id=232348
> 
> 
> Diffs
> -----
> 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/backgrounddelegate.h 1107102 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/backgrounddelegate.cpp 1107102 
> 
> Diff: http://reviewboard.kde.org/r/3417/diff
> 
> 
> Testing
> -------
> 
> I have tested it and it works OK.
> There could be a minimal performance drop (as I guess that using QTextDocument takes more time than a few drawText() and QRect constructor calls); however I couldn't really notice it
> 
> 
> Screenshots
> -----------
> 
> Layouting improved
>   http://reviewboard.kde.org/r/3417/s/344/
> 
> 
> Thanks,
> 
> Darío
> 
>