<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5689/">http://svn.reviewboard.kde.org/r/5689/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 25th, 2010, 8:43 p.m., <b>Marco Martin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/5689/diff/3/?file=40220#file40220line259" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/plasma/svg.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Theme *SvgPrivate::actualTheme()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">259</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">createRenderer</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this creates a renderer too often.
since the purpose of the cache is avoid to create renderers this is not good</pre>
</blockquote>
<p>On October 25th, 2010, 8:57 p.m., <b>Manuel Mommertz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Right, but this is solvable by inserting the size hints in SvgPrivate::localRectCache.</pre>
</blockquote>
<p>On October 25th, 2010, 11:28 p.m., <b>Ingomar Wesp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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. </pre>
</blockquote>
<p>On October 26th, 2010, 9:06 a.m., <b>Manuel Mommertz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On October 27th, 2010, 10:27 a.m., <b>Ingomar Wesp</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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 we want to use the cache without any information about the actual elements that are present, this would mean that we would have to pre-create entries for all sizes below the biggest size hinted element. So in the worst case, for a (w-h) size hinted element that would mean max(w,h) entries (well, since the aspect ratio needs to match, it's not w*h entries, at least ;)
BTW: Thinking about that, the algorithm I proposed for finding the best match will not work for non-square elements, because the aspect ratio of integer-sized elements will suffer from rounding errors that will be way beyond what qFuzzyCompare will tolerate :(
> If Plasma::Svg is created
> * Try to load size hints from Plasma::Theme's rectscache and store them localy
Do we really need to do that? The entries should "bubble down" into the local cache on demand in SvgPrivate::elementRect, no?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You got me wrong. On load, we make sure that all available size hints are stored in Plasma::Theme's rectscache. And with 'all' I mean the hints that are in the SVG not all we could generate.
If Plasma::Svg is created, there are exact to possibilitys:
* We didn't render this SVG before, so no size hints are found. So if there is something to render, the renderer is created and the size hints get loaded. No Problem so far.
* We did render this SVG before. This means we have ALL size hints in the rectscache and can your list from that. No need to load the SVG. And as we are sure we have all available hints, we can look for the best possible match in the same way you do it now with you algo. ;)
Just to be clear: We store the hints in the Theme's cache, but not in SvgPrivate's local cache. In SvgPrivate we still need a new List were the size hints get stored.
Oh, and soft feature freeze is comming, so please add this feature to http://techbase.kde.org/Schedules/KDE4/4.6_Feature_Plan as soon as possible so we don't need to wait for 4.7.</pre>
<br />
<p>- Manuel</p>
<br />
<p>On October 25th, 2010, 11:54 p.m., Ingomar Wesp wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Ingomar Wesp.</div>
<p style="color: grey;"><i>Updated 2010-10-25 23:54:14</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdelibs/plasma/private/svg_p.h <span style="color: grey">(1189821)</span></li>
<li>/trunk/KDE/kdelibs/plasma/svg.cpp <span style="color: grey">(1189821)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5689/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>