Review Request: Plasma::Svg: Do not require exact match for size hinted elements.

[Ingomar Wesp]

> On 2010-10-28 15:53:45, Marco Martin wrote:
> > I'm still not sure wether this will be fast enough, but let's try it

Great! Thanks for reviewing.

Manuel sent me a few very good suggestions via PM and I noticed a low hanging performance fruit in SharedSvgRenderer::load, so I'm afraid there will be one more diff revision...


> On 2010-10-28 15:53:45, Marco Martin wrote:
> > /trunk/KDE/kdelibs/plasma/svg.cpp, line 294
> > <http://svn.reviewboard.kde.org/r/5689/diff/6/?file=40329#file40329line294>
> >
> >     >=

Whoops. Of course...


> On 2010-10-28 15:53:45, Marco Martin wrote:
> > /trunk/KDE/kdelibs/plasma/svg.cpp, line 292
> > <http://svn.reviewboard.kde.org/r/5689/diff/6/?file=40329#file40329line292>
> >
> >     instead of the rounding error prone aspect ratio compare i think comparing width >= our width AND height >= our height could be good enough (provided the theme author doesn't provide crazy stuff)

You are right. To ensure that this is (reasonably) well-defined, I'll add a side condition so that the smallest (w*h) rect, for which this is true, is chosen. As you said, theme makers could still break this by creating equally sized hinted elements (64x32 and 32x64, for example)...


- Ingomar


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


On 2010-10-27 22:05:50, Ingomar Wesp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5689/
> -----------------------------------------------------------
> 
> (Updated 2010-10-27 22:05:50)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Previously, if an SVG contained size hinted elements, they were only used when the display size matched the size hint exactly. This patch tries to relax this condition by searching for the smallest size hinted element that is still bigger than the display size (in order for the element to be chosen, it also has to have the same aspect ratio). If no such element can be found, it falls back to the normal element id as passed.
> 
> In order to speed up the lookup (and because it appears to be impossible to access the DOM of an already loaded SvgRenderer), all size hinted element ids are stored in SharedSvgRenderer at load time.
> 
> I think it would be good to change the QRegExp based id fetching into a proper DOM traversal. Are there any convenience functions in KDELibs that allow easy iterating over all elements (couldn't find any) or do I have to implement that myself based on Qt's DOM classes?
> 
> Please tell me what you think... Have I missed something?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/private/svg_p.h 1189821 
>   /trunk/KDE/kdelibs/plasma/svg.cpp 1189821 
>   /trunk/KDE/kdelibs/plasma/theme.h 1189821 
>   /trunk/KDE/kdelibs/plasma/theme.cpp 1189821 
> 
> Diff: http://svn.reviewboard.kde.org/r/5689/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ingomar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101028/a7f2d740/attachment.htm