[Marble-devel] Review Request 108632: Basic .kmz support

[Dennis Nienhüser]

> On Jan. 29, 2013, 9:14 a.m., Bernhard Beschow wrote:
> > src/plugins/runner/kml/KmlPlugin.cpp, line 95
> > <http://git.reviewboard.kde.org/r/108632/diff/1/?file=109457#file109457line95>
> >
> >     This creates a circular dependency: FileManager already uses FileLoader which uses MarbleRunnerManager which uses the KML plugin. So if the KML plugin used FileLoader, a circle is created, which might call for trouble in the future.
> >     
> >     To avoid the circular dependency, it should be possible to create a subclassed GeoDataDocument which removes the temporary file structure upon its deletion. GeoDataDocuments are created in the KmlParser class, which is fortunately part of the KML plugin. So this approach might actually work. In additon, this approach avoids the need to pass MarbleModel to the parsers.

I'd rather call it a circular relation, not a dependency. Not a real argument against the approach in my opinion.
Still the idea to introduce a GeoDataDocument derived class to track the temporary files is very good: It reduces the complexity and amount of code changes needed for the implementation significantly.


- Dennis


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


On Jan. 30, 2013, 6:49 p.m., Dennis Nienhüser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108632/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2013, 6:49 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> Extends the kml plugin to support opening .kmz files.
> 
> - ParseRunner gets access to MarbleModel
> - KML plugin has an optional dependency on quazip
> - When opening a .kmz file, quazip is used to extract the .kmz to a temporary location
> - Last .kml file found in the .kmz is opened
> - If the .kml is closed in Marble, the temporary files are removed
> 
> 
> This addresses bug 310059.
>     http://bugs.kde.org/show_bug.cgi?id=310059
> 
> 
> Diffs
> -----
> 
>   cmake/modules/Findquazip.cmake PRE-CREATION 
>   src/plugins/runner/kml/CMakeLists.txt 35e00dc 
>   src/plugins/runner/kml/KmlDocument.h PRE-CREATION 
>   src/plugins/runner/kml/KmlDocument.cpp PRE-CREATION 
>   src/plugins/runner/kml/KmlParser.h a5e4c5f 
>   src/plugins/runner/kml/KmlParser.cpp b2ff585 
>   src/plugins/runner/kml/KmlPlugin.cpp fc23402 
>   src/plugins/runner/kml/KmlRunner.cpp 3278c78 
>   src/plugins/runner/kml/KmzHandler.h PRE-CREATION 
>   src/plugins/runner/kml/KmzHandler.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/108632/diff/
> 
> 
> Testing
> -------
> 
> Opening some random .kmz files found on the Internet works.
> 
> TODO (for separate patches):
> - Resolve files (images) in .kmz files correctly
> - Fix Marble to delete plugins / close files correctly on shutdown
> - Register .kmz files for opening with Marble
> 
> 
> Thanks,
> 
> Dennis Nienhüser
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20130130/5ddba804/attachment.html>