<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/5539/">http://svn.reviewboard.kde.org/r/5539/</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;">Ok, I had a look at this patch yesterday already. The motivation is nice since we also need some similar action for taking camera focus into account. 

Now the disadvantage of this patch is that almost all the projection methods need to get moved into the Viewport class. This kind of replicates lots of methods and introduces the awareness of the existance of GeoCoordinates into the ViewPorts class. So in a way this floods this class a bit with lots of stuff that had been encapsulated away before. 
Still this step makes sense to some degree: The only other option would be to &quot;muddy&quot; the projection classes with stuff like focus/elevation calculation. Putting an additional screenpixel conversion class (which would handle focus, elevation etc.) between Viewport and Abstractprojection might be another option but of course that could lead to other disadvantages. Any comments from your side regarding these thoughts? 
</pre>
 <br />







<p>- Torsten</p>


<br />
<p>On October 6th, 2010, 3:29 p.m., Bernhard Beschow 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 marble.</div>
<div>By Bernhard Beschow.</div>


<p style="color: grey;"><i>Updated 2010-10-06 15:29:28</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;">Using ViewportParams rather than AbstractProjection simpliefies statements like viewport-&gt;currentProjection()-&gt;screenCoordinates(...,viewport,...) to viewport-&gt;screenCoordinates(...).

That way, in the future, ViewportParams could be used for projection calculations that need to take the current viewport (and possibly an elevation model) into account, while AbstractProjections represent the pure mathematical concept (i.e. no elevation model).</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Works for me (KDE version of Marble).</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/kdeedu/marble/src/lib/GeoPainter.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/MarbleMap.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/MarblePhysics.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/MarbleWidget.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/MarbleWidgetInputHandler.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/MeasureTool.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkLayout.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkPainter.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/Projections/MercatorProjection.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/VectorMap.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/ViewParams.h <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/ViewParams.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/ViewportParams.h <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/ViewportParams.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/graphicsview/MarbleGraphicsItem.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/graphicsview/MarbleGraphicsItem_p.h <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/lib/graphicsview/ScreenGraphicsItem_p.h <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/plasmoid/worldclock.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/plugins/render/crosshairs/CrosshairsPlugin.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/MapScaleFloatItem.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/plugins/render/overviewmap/OverviewMap.cpp <span style="color: grey">(1183221)</span></li>

 <li>/trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarker.cpp <span style="color: grey">(1183221)</span></li>

</ul>

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




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








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