[Marble-devel] Review Request: Move tileSize() from AbstractScanlineTextureMapper to GeoSceneTexture

Fri Jul 9 17:36:44 CEST 2010

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

Ship it!

The patch looks good to me. It is definitely an improvement over the curren=
t state.
Before committing, could you please check if there are performance regressi=
ons?

trunk/KDE/kdeedu/marble/src/lib/AbstractScanlineTextureMapper.h
<http://reviewboard.kde.org/r/4561/#comment6175>

    This might still have its merits. m_tileSize is used in some performanc=
e critical sections.

- jmho

On 2010-07-09 14:43:20, Bernhard Beschow wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4561/
> -----------------------------------------------------------
> =

> (Updated 2010-07-09 14:43:20)
> =

> =

> Review request for marble.
> =

> =

> Summary
> -------
> =

> This will make WMS support much cleaner.
> =

> =

> Diffs
> -----
> =

>   trunk/KDE/kdeedu/marble/src/lib/AbstractScanlineTextureMapper.h 1147994 =

>   trunk/KDE/kdeedu/marble/src/lib/AbstractScanlineTextureMapper.cpp 11479=
94 =

>   trunk/KDE/kdeedu/marble/src/lib/DownloadRegionDialog.cpp 1147994 =

>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.h 1147994 =

>   trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.cpp 11479=
94 =

>   trunk/KDE/kdeedu/marble/src/tilecreator/CMakeLists.txt 1147994 =

> =

> Diff: http://reviewboard.kde.org/r/4561/diff
> =

> =

> Testing
> -------
> =

> Works for me. Please test!
> =

> =

> Thanks,
> =

> Bernhard
> =

>

--===============0178892683145757273==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

<html>
 <body>
  <div style=3D"font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor=3D"#f9f3c9" width=3D"100%" cellpadding=3D"8" style=3D"bor=
der: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href=3D"http://reviewboard.kde.org/r/4561/">http://reviewboard.kde=
.org/r/4561/</a>
     </td>
    </tr>
   </table>
   <br />

 <p>Ship it!</p>

 <pre>The patch looks good to me. It is definitely an improvement over the =
current state.
Before committing, could you please check if there are performance regressi=
ons?</pre>
 <br />

<div>

<table width=3D"100%" border=3D"0" bgcolor=3D"white" style=3D"border: 1px s=
olid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan=3D"4" bgcolor=3D"#F0F0F0" style=3D"border-bottom: 1px solid =
#C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href=3D"/r/4561/diff/1/?file=3D30599#file30599line138" style=3D"colo=
r: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdeedu/=
marble/src/lib/AbstractScanlineTextureMapper.h</a>
    <span style=3D"font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style=3D"background-color: #e4d9cb; padding: 4px 8px; text-align: c=
enter;">
  <tr>

   <td colspan=3D"4"><pre style=3D"font-size: 8pt; line-height: 140%; margi=
n: 0; white-space: pre-wrap; word-wrap: break-word;">public:</pre></td>

  </tr>
 </tbody>

 =

 =

 <tbody>

  <tr>
    <th bgcolor=3D"#ebb1ba" style=3D"border-right: 1px solid #C0C0C0;" alig=
n=3D"right"><font size=3D"2">138</font></th>
    <td bgcolor=3D"#ffc5ce" width=3D"50%"><pre style=3D"font-size: 8pt; lin=
e-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"> =
   <span class=3D"n">QSize</span> <span class=3D"n">m_tileSize</span><span =
class=3D"p">;</span></pre></td>
    <th bgcolor=3D"#ebb1ba" style=3D"border-left: 1px solid #C0C0C0; border=
-right: 1px solid #C0C0C0;" align=3D"right"><font size=3D"2"></font></th>
    <td bgcolor=3D"#ffc5ce" width=3D"50%"><pre style=3D"font-size: 8pt; lin=
e-height: 140%; margin: 0; white-space: pre-wrap; word-wrap: break-word;"><=
/pre></td>
  </tr>

 </tbody>

</table>

<pre style=3D"margin-left: 2em">This might still have its merits. m_tileSiz=
e is used in some performance critical sections.</pre>
</div>
<br />

<p>- jmho</p>

<br />
<p>On July 9th, 2010, 2:43 p.m., Bernhard Beschow wrote:</p>

<table bgcolor=3D"#fefadf" width=3D"100%" cellspacing=3D"0" cellpadding=3D"=
8" style=3D"background-image: url('http://reviewboard.kde.orgrb/images/revi=
ew_request_box_top_bg.png'); background-position: left top; background-repe=
at: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for marble.</div>
<div>By Bernhard Beschow.</div>

<p style=3D"color: grey;"><i>Updated 2010-07-09 14:43:20</i></p>

<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Descripti=
on </h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style=3D"margin: 0; padding: 0;">This will make WMS support much cl=
eaner.</pre>
  </td>
 </tr>
</table>

<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing <=
/h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style=3D"margin: 0; padding: 0;">Works for me. Please test!</pre>
  </td>
 </tr>
</table>

<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b>=
 </h1>
<ul style=3D"margin-left: 3em; padding-left: 0;">

 <li>trunk/KDE/kdeedu/marble/src/lib/AbstractScanlineTextureMapper.h <span =
style=3D"color: grey">(1147994)</span></li>

 <li>trunk/KDE/kdeedu/marble/src/lib/AbstractScanlineTextureMapper.cpp <spa=
n style=3D"color: grey">(1147994)</span></li>

 <li>trunk/KDE/kdeedu/marble/src/lib/DownloadRegionDialog.cpp <span style=
=3D"color: grey">(1147994)</span></li>

 <li>trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.h <span =
style=3D"color: grey">(1147994)</span></li>

 <li>trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneTexture.cpp <spa=
n style=3D"color: grey">(1147994)</span></li>

 <li>trunk/KDE/kdeedu/marble/src/tilecreator/CMakeLists.txt <span style=3D"=
color: grey">(1147994)</span></li>

</ul>

<p><a href=3D"http://reviewboard.kde.org/r/4561/diff/" style=3D"margin-left=
: 3em;">View Diff</a></p>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

--===============0178892683145757273==--

-------------------------------------------------------------