<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://git.reviewboard.kde.org/r/105828/">http://git.reviewboard.kde.org/r/105828/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 2nd, 2012, 4:52 p.m., <b>Torsten Rahn</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://git.reviewboard.kde.org/r/105828/diff/1/?file=75788#file75788line127" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/graphicsview/GeoGraphicsItem.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class MARBLE_EXPORT GeoGraphicsItem : public MarbleGraphicsItem</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class MARBLE_EXPORT GeoGraphicsItem</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">120</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Returns all coordinates of the item in view coordinates according to the given projection.</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">126</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Paints the item in item coordinates. This has to be reimplemented by the subclass</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 comment should probably outline the difference in behavior
paint() has different semantics for ScreenGraphicsItems/AbstractDataPluginItems and GeoGraphicsItems
The GeoPainter used in "real" GeoGraphicsItems repeats items in x-direction if neccessary.
For ScreenGraphicsItems/AbstractDataPluginItems, this is done in paintEvent() presumably due to caching.</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;">I updated the documentation to:
> Paints the item using the given GeoPainter.
>
> Note that depending on the projection and zoom level, the item may be visible more than once. GeoPainter will therefore automatically "repeat" primitives which have a geo position (GeoDataCoordinates).</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 2nd, 2012, 4:52 p.m., <b>Torsten Rahn</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://git.reviewboard.kde.org/r/105828/diff/1/?file=75795#file75795line134" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/graphicsview/MarbleGraphicsItem.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">class MARBLE_EXPORT MarbleGraphicsItem</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">133</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Paints the item in item coordinates. This has to be reimplemented by the subclass</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">133</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Paints the item in item coordinates. This has to be reimplemented by the subclass</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;">You might want to add this information here:
paint() has different semantics for ScreenGraphicsItems/AbstractDataPluginItems and GeoGraphicsItems
The GeoPainter used in "real" GeoGraphicsItems repeats items in x-direction if neccessary.
For ScreenGraphicsItems/AbstractDataPluginItems, this is done in paintEvent() presumably due to caching.</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;">This information is still correct.</pre>
<br />
<p>- Bernhard</p>
<br />
<p>On August 2nd, 2012, 4:23 p.m., Bernhard Beschow wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 Aug. 2, 2012, 4:23 p.m.</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;">This review request includes two patches:
1. API change: don't have GeoGraphicsItem inherit MarbleGraphicsItem; introduce GeoLabelGraphicsItem as a placeholder
GeoGraphicsItems and MarbleGraphicsItems shouldn't be mixed for the following reasons:
* paintEvent() is NOT called for real GeoGraphicsItems
Instead, paint() is called directly. As a result, "real" GeoGraphicsItems don't make use of caching.
* paint() has different semantics for ScreenGraphicsItems/AbstractDataPluginItems and GeoGraphicsItems
The GeoPainter used in "real" GeoGraphicsItems repeats items in x-direction if neccessary.
For ScreenGraphicsItems/AbstractDataPluginItems, this is done in paintEvent() presumably due to caching.
* ScreenGraphicsItems/AbstractDataPluginItems are 2D objects, whereas GeoGraphicsItems are 3D objects
ScreenGraphicsItems/AbstractDataPluginItems represent two-dimensional labels whose size() is independent of the zoom level and they always face the screen (a.k.a. billboards).
GeoGraphicsItems don't make use of size(). Instead, their size and shape changes depending on the projection.
Since AbstractDataPluginItem has coordinates(), it was derived from GeoGraphicsItem.
In order to keep a hight level of source-compatibilty, this patch introduces the placeholder class GeoLabelGraphicsItem.
It implements the methods GeoGraphicsItem used to deliver to the AbstractDataPluginItems.
2. API change: reflect in the API that MarbleGraphicsItems are now 2D objects
All MarbleGraphicsItems now take a QPainter rather than a GeoPainter, which is sufficient for 2D items.
In addition, the ViewportParams, renderPosition and GeoSceneLayer parameters have been removed from all paint() methods.
* using plain 2D API rather than GeoPainter avoids possible side effects of GeoPainter such as repeating in x-direction, which could disturb caching
* plain 2D items also shouldn't depend on the current viewport, especially on the projection
Reacting to viewport changes is currently only possible by subclassing AbstractFloatItem and by reimplementing changeViewport(), where this method is called always before paint().
* the renderPosition and GeoSceneLayer parameters were never used and were probably overdesigned</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;">Online plugins and float items still seem to be rendered properly.
Question: Should GeoLabelGraphicsItem be renamed, e.g. to BillboardGraphicsItem?</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>.reviewboardrc <span style="color: grey">(285adfe926c9b544d711997e10624059252f3961)</span></li>
<li>src/lib/AbstractDataPluginItem.h <span style="color: grey">(dbaeeb3b71f4f6bd649d84bdd71130d041c9104b)</span></li>
<li>src/lib/AbstractDataPluginItem.cpp <span style="color: grey">(511a11b4e23f8cf256ac849b1a5f888c6688bfe1)</span></li>
<li>src/lib/AbstractFloatItem.cpp <span style="color: grey">(e54b72f05210b5ded2fda0eb33c82c4b9fef328e)</span></li>
<li>src/lib/graphicsview/CMakeLists.txt <span style="color: grey">(8d51bb841b573f5c49646965c389b593f2cf98e4)</span></li>
<li>src/lib/graphicsview/FrameGraphicsItem.h <span style="color: grey">(891512bd955ff218d17ce004e0abd4e841975c2f)</span></li>
<li>src/lib/graphicsview/FrameGraphicsItem.cpp <span style="color: grey">(f91281f219d6b5141f82802eafb4fa9a2f5d563e)</span></li>
<li>src/lib/graphicsview/GeoGraphicsItem.h <span style="color: grey">(6d888c75816c4abb46771cbccb11e805d4e7b3d4)</span></li>
<li>src/lib/graphicsview/GeoGraphicsItem.cpp <span style="color: grey">(399a10a69baaac820b858f0d09931fedb548a54f)</span></li>
<li>src/lib/graphicsview/GeoGraphicsItem_p.h <span style="color: grey">(6e9657fdaa9fbfbd08a537b3c073093ae796c47a)</span></li>
<li>src/lib/graphicsview/GeoLabelGraphicsItem.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/graphicsview/GeoLabelGraphicsItem.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/graphicsview/LabelGraphicsItem.h <span style="color: grey">(187cb265549167c5c1ab02df5839f9fd4b4aa38a)</span></li>
<li>src/lib/graphicsview/LabelGraphicsItem.cpp <span style="color: grey">(ca0943a6bb1e4f7ceba161dc37d9e6ee6201de5b)</span></li>
<li>src/lib/graphicsview/MarbleGraphicsItem.h <span style="color: grey">(83ca8cd5de8bf155c0eedf08b7a7ba8c306742b4)</span></li>
<li>src/lib/graphicsview/MarbleGraphicsItem.cpp <span style="color: grey">(f136b7ed04305bfcfdd0953122165f84012c8a17)</span></li>
<li>src/lib/graphicsview/MarbleGraphicsItem_p.h <span style="color: grey">(2b0f50e1746ab2d852de545ee5bcdd53b1778d16)</span></li>
<li>src/lib/graphicsview/ScreenGraphicsItem_p.h <span style="color: grey">(1f84c0f013dfc90d8941d0e06890977f3df2fcce)</span></li>
<li>src/lib/graphicsview/WidgetGraphicsItem.h <span style="color: grey">(d98b6320b7541077de5821e2f9f3bb02260f49c7)</span></li>
<li>src/lib/graphicsview/WidgetGraphicsItem.cpp <span style="color: grey">(0ea63460520ae590c5760fe62059e03bd3cd02af)</span></li>
<li>src/lib/graphicsview/WidgetGraphicsItem_p.h <span style="color: grey">(81eb026118a60bbea370310e8142ab00f0527b44)</span></li>
<li>src/plugins/render/compass/CompassFloatItem.h <span style="color: grey">(afdad8e276b5721db598c51443b737465293cbd9)</span></li>
<li>src/plugins/render/compass/CompassFloatItem.cpp <span style="color: grey">(d1b4b149ff4ba184710b4345205b950eb02a770c)</span></li>
<li>src/plugins/render/earthquake/EarthquakeItem.h <span style="color: grey">(3a23e7ab1b104ea5aaf430066ccd748a19a15ac2)</span></li>
<li>src/plugins/render/earthquake/EarthquakeItem.cpp <span style="color: grey">(305063de4f6a808cdcd88866f12429a802ae0c04)</span></li>
<li>src/plugins/render/elevationprofilefloatitem/ElevationProfileFloatItem.h <span style="color: grey">(788faaab46577b8b8816ab5312bf4498fdb4b0e3)</span></li>
<li>src/plugins/render/elevationprofilefloatitem/ElevationProfileFloatItem.cpp <span style="color: grey">(eb13aee3ff654fb961a34ce79294fca43b78e330)</span></li>
<li>src/plugins/render/elevationprofilemarker/ElevationProfileMarker.h <span style="color: grey">(59f8139764a03facf6003d252463311fdc969f26)</span></li>
<li>src/plugins/render/mapscale/MapScaleFloatItem.h <span style="color: grey">(b68a46b0386809166ffcc03b8ae0159908f07067)</span></li>
<li>src/plugins/render/mapscale/MapScaleFloatItem.cpp <span style="color: grey">(1c17ed05ec7c25c37ea2d29dc2b75e9b4d0b94b2)</span></li>
<li>src/plugins/render/opendesktop/OpenDesktopItem.h <span style="color: grey">(78078d91299fa899450cea25f006eac6ed7aec8a)</span></li>
<li>src/plugins/render/opendesktop/OpenDesktopItem.cpp <span style="color: grey">(b7930ee3c1173bdb3331f8244ea3efaa4a068fb3)</span></li>
<li>src/plugins/render/overviewmap/OverviewMap.h <span style="color: grey">(9db547d10714a692172d4b2f1e292e9ca7bce5a1)</span></li>
<li>src/plugins/render/overviewmap/OverviewMap.cpp <span style="color: grey">(bc9aea455b65381d9aef19aec70d4212186180cd)</span></li>
<li>src/plugins/render/postalcode/PostalCodeItem.h <span style="color: grey">(999f1d23e1bb3c21cf12f533f8879995fedfdf78)</span></li>
<li>src/plugins/render/postalcode/PostalCodeItem.cpp <span style="color: grey">(13702d7f3860b8244c5b3402a5c08264f475fb27)</span></li>
<li>src/plugins/render/progress/ProgressFloatItem.h <span style="color: grey">(326ae7853eccb84472b87e9db48ad8d8f21b2d89)</span></li>
<li>src/plugins/render/progress/ProgressFloatItem.cpp <span style="color: grey">(251d2402ccc35c4e27974a31dd93fd4d1db1f4fb)</span></li>
<li>src/plugins/render/wikipedia/WikipediaItem.h <span style="color: grey">(38f00a892f214819ee6760f7ab7d793785f265bd)</span></li>
<li>src/plugins/render/wikipedia/WikipediaItem.cpp <span style="color: grey">(65bdac91792b3021c8a315dac470c24fdd6730ca)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105828/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>