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

[Manuel Mommertz]

> On 2010-10-25 20:43:20, Marco Martin wrote:
> > /trunk/KDE/kdelibs/plasma/svg.cpp, line 259
> > <http://svn.reviewboard.kde.org/r/5689/diff/3/?file=40220#file40220line259>
> >
> >     this creates a renderer too often.
> >     since the purpose of the cache is avoid to create renderers this is not good
> 
> Manuel Mommertz wrote:
>     Right, but this is solvable by inserting the size hints in SvgPrivate::localRectCache.
> 
> Ingomar Wesp wrote:
>     > Right, but this is solvable by inserting the size hints in SvgPrivate::localRectCache.
>     
>     To be honest, I'm not sure I understand the cache related code well enough, so please be patient with me here...
>     
>     From what I (think I) understand, you intend to move the search for the best fit to SvgPrivate::findAndCacheElementRect and insert it into the theme's rect cache? Because if I'm not mistaken, just adding it to the localRectCache would not allow the entry to be persisted and would thus not help prevent the creation of a renderer in the future.

It is a bit more work than I thought. But it is doable in the following way:

If the renderer is created:
* Load SVG and get size hints
* Store size hints in Plasma::SvgPrivate (not in the renderer) and in Plasma::Theme's rectscache

If Plasma::Svg is created
* Try to load size hints from Plasma::Theme's rectscache and store them localy

On Rendering:
* Use the size hints


For this to work we need support in Plasma::Theme to get a list of available rects. Should be easy to implement.


Questions:
* How fast is KConfig? If it is fast enaugh, it might not be needed to store size hints localy. But I doubt it is.
* Should Plasma::Theme provide a full list of all rects or should filtering for size hints take effect? Personaly I think a full list is better, as this might be used for other things in the future.


- Manuel


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


On 2010-10-25 23:54:14, Ingomar Wesp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5689/
> -----------------------------------------------------------
> 
> (Updated 2010-10-25 23:54:14)
> 
> 
> 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 
> 
> 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/20101026/32984bac/attachment-0001.htm