[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: Plasma::Svg: Do not require exact match for size
From: "Manuel Mommertz" <2kmm () gmx ! de>
Date: 2010-10-25 8:03:48
Message-ID: 20101025080348.12979.59679 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5689/#review8338
-----------------------------------------------------------
If you want to use the DOM to find the size hints, you can take a look at K=
GameSvgDocument. It implements a way to find elements by id. This could sho=
uld be usable for you.
And can you please measure how long it takes to load the SVG and to look fo=
r an element with and without your patch?
/trunk/KDE/kdelibs/plasma/svg.cpp
<http://svn.reviewboard.kde.org/r/5689/#comment8615>
Please move this to the bottom too if you don't need the DOM, as it tak=
es a long time to load it and we don't want this to happen if it isn't need=
ed.
- Manuel
On 2010-10-25 00:17:45, Ingomar Wesp wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5689/
> -----------------------------------------------------------
> =
> (Updated 2010-10-25 00:17:45)
> =
> =
> 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 r=
elax 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 chose=
n, it also has to have the same aspect ratio). If no such element can be fo=
und, 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 elemen=
t ids are stored in SharedSvgRenderer at load time.
> =
> I think it would be good to change the QRegExp based id fetching into a p=
roper DOM traversal. Are there any convenience functions in KDELibs that al=
low easy iterating over all elements (couldn't find any) or do I have to im=
plement 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 1189389 =
> /trunk/KDE/kdelibs/plasma/svg.cpp 1189389 =
> =
> Diff: http://svn.reviewboard.kde.org/r/5689/diff
> =
> =
> Testing
> -------
> =
> =
> Thanks,
> =
> Ingomar
> =
>
[Attachment #5 (text/html)]
<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 />
<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 you want to use the \
DOM to find the size hints, you can take a look at KGameSvgDocument. It implements a \
way to find elements by id. This could should be usable for you.
And can you please measure how long it takes to load the SVG and to look for an \
element with and without your patch?</pre> <br />
<div>
<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/2/?file=40113#file40113line78" \
style="color: black; font-weight: bold; text-decoration: \
underline;">/trunk/KDE/kdelibs/plasma/svg.cpp</a> <span style="font-weight: \
normal;">
(Diff revision 2)
</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; \
">SharedSvgRenderer::SharedSvgRenderer(const QByteArray &contents, const QString \
&styleSheet, QObject *parent)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">75</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">QDomDocument</span> <span class="n">svg</span><span \
class="p">;</span></pre></td> <th bgcolor="#f0f0f0" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">75</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="n">QDomDocument</span> <span \
class="n">svg</span><span class="p">;</span></pre></td> </tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">76</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> \
<span class="p">(</span><span class="o">!</span><span class="n">svg</span><span \
class="p">.</span><span class="n">setContent</span><span class="p">(</span><span \
class="n">contents</span><span class="p">))</span> <span \
class="p">{</span></pre></td> <th bgcolor="#f0f0f0" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">76</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span \
class="p">(</span><span class="o">!</span><span class="n">svg</span><span \
class="p">.</span><span class="n">setContent</span><span class="p">(</span><span \
class="n">contents</span><span class="p">))</span> <span \
class="p">{</span></pre></td> </tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">77</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">return</span> <span class="kc">false</span><span \
class="p">;</span></pre></td> <th bgcolor="#f0f0f0" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">77</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span \
class="kc">false</span><span class="p">;</span></pre></td> </tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">78</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="p">}</span></pre></td> <th bgcolor="#f0f0f0" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">78</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please move \
this to the bottom too if you don't need the DOM, as it takes a long time to load \
it and we don't want this to happen if it isn't needed.</pre> </div>
<br />
<p>- Manuel</p>
<br />
<p>On October 25th, 2010, 12:17 a.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 00:17:45</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">(1189389)</span></li>
<li>/trunk/KDE/kdelibs/plasma/svg.cpp <span style="color: \
grey">(1189389)</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>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic