<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/6040/">http://svn.reviewboard.kde.org/r/6040/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Didn't spot anything going wrong when testing it on the N900. And it's indeed faster.
See the comments on IRC (addressing the patch series on gitorious).
</pre>
<br />
<p>- Dennis</p>
<br />
<p>On December 4th, 2010, 12:14 a.m., Thibaut Gridel 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 Thibaut Gridel.</div>
<p style="color: grey;"><i>Updated 2010-12-04 00:14:25</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;">Please review this code refactor in the model and geodata dept.
The goal is to use the tree model and proxies above geodata documents, and deprecate the awkward MarblePlacemarkModel.
For this purpose, the following is done:
- the datafacade does not keep a MarblePlacemarkModel just to have a flat list of placemark, it uses the stolen KDescendantsProxyModel above the treemodel.
- the users of the placemarkModel: PlacemarkLayout and NavigationWidget, keep their own sorting and filtering proxy models with them (unclutters MarbleModel)
- Where placemarkModel::data() was abused to retrieve placemark specific data, a pointer to a placemark is retrieved and direct methods of the placemark are used (lot faster too). Only the basic roles which have a filtering interest (name, coordinates, popularity...) should stay.
- MarbleModel, Map and Widget had a centerOn(QModelIndex) without referring to which model those indexes were. There was no use either...
- GeoDataTreeModel has a 5times speed gain while using nodeType as const char* for type id rather than dynamic_cast !!!
- A trick is done to connect the models only after all files have finished loading, another launch boost
- the PlacemarkManager is removed: specific code to give population figures, roles and extra info is moved to FileLoader, and the remaining code was there to fill the MarblePlacemarkModel.
Todo after this:
- Continue removing MarblePlacemarkModel in Runners and Routing Model. 2 Options:
- either a List of Placemarks is enough as a model, or
- a treemodel and proxies above a GeoDataDocument</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;">Everything should work as usual.
Checked the following:
- the NavigationWidget list of placemark is OK (should display updated list of placemarks as the treemodel is populated)
- the PlacemarkLayout also displays placemarks from the new proxies stack
- Search and Routing Runners still work
- Routing model, routing points... work
- Placemark Dialog (left click on a placemark) still display placemark properties
- profiled and benchmarked for loading
- (not checked) memory usage should have decreased</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/CMakeLists.txt <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/FileLoader.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/FileLoader.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/FileManager.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/FileManager.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/FileViewWidget.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/GeoDataTreeModel.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/GoToDialog.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleDataFacade.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleDataFacade.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleMap.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleMap.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarblePlacemarkModel.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleSearchListView.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleSearchListView.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleWidget.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleWidget.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/NavigationWidget.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/NavigationWidget.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkInfoDialog.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkLayout.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkLayout.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkManager.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkManager.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PlacemarkPainter.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/VisiblePlacemark.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataColorStyle.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataColorStyle.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataContainer.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataContainer.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataContainer_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataData.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataData.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataData_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataDocument.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataDocument.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataDocument_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataExtendedData_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFolder.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFolder.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataGeometry.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataGeometry.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataGeometry_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataHotSpot.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataHotSpot.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataIconStyle.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataIconStyle.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLabelStyle.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLabelStyle.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLatLonAltBox.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLatLonAltBox.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLatLonBox.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLatLonBox.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLineString.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLineString.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLineString_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLineStyle.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLineStyle.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLinearRing.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLinearRing.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLinearRing_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLod.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLod.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLod_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLookAt.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataLookAt.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataMultiGeometry.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataMultiGeometry.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataMultiGeometry_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataObject.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataObject.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPlacemark.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPlacemark.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPlacemark_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPoint.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPoint.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPolyStyle.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPolyStyle.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPolygon.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPolygon.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPolygon_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataRegion.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataRegion.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataRegion_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataStyle.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataStyle.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataStyleMap.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataStyleMap.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataStyleSelector.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataStyleSelector.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimePrimitive.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimePrimitive.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimePrimitive_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimeSpan.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimeSpan.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimeSpan_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimeStamp.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimeStamp.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataTimeStamp_p.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/kbihash_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/kdescendantsproxymodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/kdescendantsproxymodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.h <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.cpp <span style="color: grey">(1202435)</span></li>
<li>/trunk/KDE/kdeedu/marble/tests/TestGeoData.cpp <span style="color: grey">(1202435)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6040/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>