[Marble-devel] Review Request: GeoData: showcase test for pointer-based GeoData

[Thibaut Gridel]

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

(Updated 2010-06-27 21:46:09.252277)


Review request for marble.


Changes
-------

This updated diff is the result of the following changes (squashed together so that it fits reviewboard)

- GeoDataObject: provide parent()

- GeoDataContainer: store GeoDataFeature* in GeoDataContainer
    also provide a pointer-based api which doesn't detach() to access a pointer
    last, use iterators for GeoDataFeature* to iterate features

- GeoData: showcase test for pointer-based GeoDataContainer

- FileManager takes over geodatadocument ownership (hence FileManager::closeFile and not FileLoader does the delete, and KmlFileViewItem stores a pointer to this document)

- GeoDataMultiGeometry: store GeoDataGeometry* in GeoDataMultiGeometry and provides same pointer-based api

- GeoData: dead code for GeoDataObject, GeoDataFeature, GeoDataGeometry and GeoDataPlacemark

- GeoDataTreeModel, a model that works(TM)

- PositionTracking: append GeoDataDocument to FileManager directly (and so don't use geometrymodel)

- GeoRenderer: use the GeoDataTreeModel (and so don't use geometrymodel either)

- drop the MarbleGeometryModel


Remaining points:

- Parser code should use the pointer-based api to store features and geometries in a sane way
- GeoDataTreeModel should not have to fix parenting
- PlacemarkManager and model should use either pointer-based data or even better rely on GeoDataTreeModel

Tests/ New Features:

- GeoDataTreeModel passes the ModelTest test harnessing, meaning it is a real model, and supports a treeview
- Now multiple documents containing geometries can be rendered!
- PositionTracking feeds one of those, and it is available in the FileManager


Summary
-------

providing a (naive) pointer-based api for GeoDataContainer, and test case which shows early breaks...

Toggle between the two "document.append(folder);" shows how brittle the api still is.


Diffs (updated)
-----

  /trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/FileLoader.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/FileManager.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/FileManager.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/GeoDataTreeModel.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/GeoDataTreeModel.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/KmlFileViewItem.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/KmlFileViewItem.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleDataFacade.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleDataFacade.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleGeometryModel.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleGeometryModel.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataContainer.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataContainer.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataContainer_p.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFeature.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataFolder.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataGeometry.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataMultiGeometry.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataMultiGeometry.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataMultiGeometry_p.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataObject.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataObject.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPlacemark.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/data/GeoDataPlacemark.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/geodata/writers/kml/KmlDocumentTagWriter.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.h 1143093 
  /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/plugins/render/geodata/GeoRendererPlugin.cpp 1143093 
  /trunk/KDE/kdeedu/marble/src/plugins/render/geodata/GeoRendererView.h 1143093 
  /trunk/KDE/kdeedu/marble/src/plugins/render/geodata/GeoRendererView.cpp 1143093 
  /trunk/KDE/kdeedu/marble/tests/TestGeoData.cpp 1143093 

Diff: http://reviewboard.kde.org/r/4285/diff


Testing
-------

Run TestGeoData :(


Thanks,

Thibaut

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100627/a2280f67/attachment-0001.htm