[Marble-devel] Review Request 110403: Add support for reading the KML Model tag
Dennis Nienhüser
earthwings at gentoo.org
Tue May 14 19:10:01 UTC 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110403/#review32515
-----------------------------------------------------------
Thanks, works fine. There are some changes needed in the object hierarchy to have the GeoData* classes conform with the KML hierarchy, see the comments about inheritance below.
src/lib/geodata/data/GeoDataAlias.h
<http://git.reviewboard.kde.org/r/110403/#comment24202>
It's not a GeoDataObject, see https://developers.google.com/kml/documentation/images/classTree52.gif
Please don't derive from any class
src/lib/geodata/data/GeoDataAlias.h
<http://git.reviewboard.kde.org/r/110403/#comment24190>
const QString &
src/lib/geodata/data/GeoDataAlias.h
<http://git.reviewboard.kde.org/r/110403/#comment24191>
const QString &
src/lib/geodata/data/GeoDataLocation.h
<http://git.reviewboard.kde.org/r/110403/#comment24201>
It's neither a GeoDataGeometry nor a GeoDataObject. Please don't derive from anything.
src/lib/geodata/data/GeoDataLocation.h
<http://git.reviewboard.kde.org/r/110403/#comment24192>
Please add a doxygen comment saying that the unit is meter
src/lib/geodata/data/GeoDataLocation.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24193>
Please call
GeoDataGeometry::operator=( other );
before
src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24194>
Copyright year is missing
src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24196>
Please rename map => resourceMap and provide two methods
const GeoDataResourceMap& resourceMap() const;
and
GeoDataResourceMap& resourceMap();
src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24197>
Please rename setResourceMap
src/lib/geodata/data/GeoDataModel.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24198>
Copyright year is missing
src/lib/geodata/data/GeoDataOrientation.h
<http://git.reviewboard.kde.org/r/110403/#comment24200>
It's neither a GeoDataGeometry nor a GeoDataObject. Please don't derive from anything
src/lib/geodata/data/GeoDataOrientation.h
<http://git.reviewboard.kde.org/r/110403/#comment24199>
Please mention the unit, range, and rotation direction here and for tilt() and roll() as doxygen comments (see https://developers.google.com/kml/documentation/kmlreference#orientation)
src/lib/geodata/data/GeoDataResourceMap.h
<http://git.reviewboard.kde.org/r/110403/#comment24207>
It's not a GeoDataObject, please don't derive from it
src/lib/geodata/data/GeoDataResourceMap.h
<http://git.reviewboard.kde.org/r/110403/#comment24206>
Two methods needed here as well for const correctness
src/lib/geodata/data/GeoDataResourceMap.h
<http://git.reviewboard.kde.org/r/110403/#comment24205>
const QString &
src/lib/geodata/data/GeoDataResourceMap.h
<http://git.reviewboard.kde.org/r/110403/#comment24204>
const QString &
src/lib/geodata/data/GeoDataScale.h
<http://git.reviewboard.kde.org/r/110403/#comment24211>
Coypright year is missing
src/lib/geodata/data/GeoDataScale.h
<http://git.reviewboard.kde.org/r/110403/#comment24214>
Please derive from GeoDataObject here
src/lib/geodata/data/GeoDataScale.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24212>
Coypright year is missing
src/lib/geodata/data/GeoDataScale.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24215>
Please call GeoDataObject::operator= here as well
src/lib/geodata/handlers/kml/KmlElementDictionary.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24181>
Please keep the alphabetic order and move kmlTag_ResourceMap and kmlTag_Alias up.
src/lib/geodata/handlers/kml/KmlLinkTagHandler.h
<http://git.reviewboard.kde.org/r/110403/#comment24182>
No changes in this file, please remove.
src/lib/geodata/handlers/kml/KmlLinkTagHandler.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24183>
Copyright year is missing
src/lib/geodata/handlers/kml/KmlLongitudeTagHandler.h
<http://git.reviewboard.kde.org/r/110403/#comment24184>
No changes in this file, please remove.
src/lib/geodata/handlers/kml/KmlRefreshModeTagHandler.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24216>
Please add an else { } part which sets onChange as value and warns (mDebug) that the value set in the kml file is invalid. Or initialize mode to onChange as before and only warn.
src/lib/geodata/handlers/kml/KmlRollTagHandler.h
<http://git.reviewboard.kde.org/r/110403/#comment24186>
No changes in this file, please remove
src/lib/geodata/handlers/kml/KmlRollTagHandler.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24185>
Copyright year is missing
src/lib/geodata/parser/GeoDataTypes.h
<http://git.reviewboard.kde.org/r/110403/#comment24188>
Please keep the alphabetic order for all elements added
src/lib/geodata/parser/GeoDataTypes.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24187>
Please keep the alphabetic order for all elements added
tests/TestModel.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24189>
Copyright year is missing
- Dennis Nienhüser
On May 12, 2013, 12:18 p.m., Sanjiban Bairagya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110403/
> -----------------------------------------------------------
>
> (Updated May 12, 2013, 12:18 p.m.)
>
>
> Review request for Marble.
>
>
> Description
> -------
>
> Previously there was no way to read the <Model> tag in the kml files, if any. I added a few GeoData classes and tag-handlers which makes it possible now.
>
>
> This addresses bug 318288.
> http://bugs.kde.org/show_bug.cgi?id=318288
>
>
> Diffs
> -----
>
> src/lib/geodata/CMakeLists.txt 1644d87
> src/lib/geodata/data/GeoDataAlias.h PRE-CREATION
> src/lib/geodata/data/GeoDataAlias.cpp PRE-CREATION
> src/lib/geodata/data/GeoDataGeometry.h 19cdd29
> src/lib/geodata/data/GeoDataLink.h 6cba0da
> src/lib/geodata/data/GeoDataLink.cpp 2c8f08b
> src/lib/geodata/data/GeoDataLocation.h PRE-CREATION
> src/lib/geodata/data/GeoDataLocation.cpp PRE-CREATION
> src/lib/geodata/data/GeoDataModel.h PRE-CREATION
> src/lib/geodata/data/GeoDataModel.cpp PRE-CREATION
> src/lib/geodata/data/GeoDataOrientation.h PRE-CREATION
> src/lib/geodata/data/GeoDataOrientation.cpp PRE-CREATION
> src/lib/geodata/data/GeoDataResourceMap.h PRE-CREATION
> src/lib/geodata/data/GeoDataResourceMap.cpp PRE-CREATION
> src/lib/geodata/data/GeoDataScale.h PRE-CREATION
> src/lib/geodata/data/GeoDataScale.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlAliasTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlAliasTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlAltitudeModeTagHandler.cpp 827293b
> src/lib/geodata/handlers/kml/KmlAltitudeTagHandler.h e34fd61
> src/lib/geodata/handlers/kml/KmlAltitudeTagHandler.cpp 6884dba
> src/lib/geodata/handlers/kml/KmlCoordinatesTagHandler.cpp 1d479fe
> src/lib/geodata/handlers/kml/KmlElementDictionary.h 11d9f1d
> src/lib/geodata/handlers/kml/KmlElementDictionary.cpp 36a1b0e
> src/lib/geodata/handlers/kml/KmlHeadingTagHandler.cpp a0c4801
> src/lib/geodata/handlers/kml/KmlHrefTagHandler.cpp 052d5b0
> src/lib/geodata/handlers/kml/KmlLatitudeTagHandler.h c11ca2b
> src/lib/geodata/handlers/kml/KmlLatitudeTagHandler.cpp 8040b97
> src/lib/geodata/handlers/kml/KmlLinkTagHandler.h 414face
> src/lib/geodata/handlers/kml/KmlLinkTagHandler.cpp a23acda
> src/lib/geodata/handlers/kml/KmlLocationTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlLocationTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlLongitudeTagHandler.h e8e81e4
> src/lib/geodata/handlers/kml/KmlLongitudeTagHandler.cpp 88f534b
> src/lib/geodata/handlers/kml/KmlModelTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlModelTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlOrientationTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlOrientationTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlRefreshModeTagHandler.cpp 21447fe
> src/lib/geodata/handlers/kml/KmlResourceMapTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlResourceMapTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlRollTagHandler.h 7aa69f3
> src/lib/geodata/handlers/kml/KmlRollTagHandler.cpp 22404bf
> src/lib/geodata/handlers/kml/KmlScaleTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlScaleTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlSourceHrefTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlSourceHrefTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlTargetHrefTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlTargetHrefTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlTiltTagHandler.cpp 806593c
> src/lib/geodata/handlers/kml/KmlXTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlXTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlYTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlYTagHandler.cpp PRE-CREATION
> src/lib/geodata/handlers/kml/KmlZTagHandler.h PRE-CREATION
> src/lib/geodata/handlers/kml/KmlZTagHandler.cpp PRE-CREATION
> src/lib/geodata/parser/GeoDataTypes.h 1bc7828
> src/lib/geodata/parser/GeoDataTypes.cpp ec7c414
> tests/CMakeLists.txt d4cfec5
> tests/TestModel.cpp PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/110403/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sanjiban Bairagya
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20130514/22d95d0e/attachment-0001.html>
More information about the Marble-devel
mailing list