<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 />
<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>
<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>