----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118717/#review64190 -----------------------------------------------------------
src/lib/marble/GeoGraphicsScene.cpp <https://git.reviewboard.kde.org/r/118717/#comment44827> Please pass a const reference src/lib/marble/GeoGraphicsScene.cpp <https://git.reviewboard.kde.org/r/118717/#comment44828> foreach? src/lib/marble/GeoGraphicsScene.cpp <https://git.reviewboard.kde.org/r/118717/#comment44829> const TileId &tileId src/lib/marble/GeoGraphicsScene.cpp <https://git.reviewboard.kde.org/r/118717/#comment44830> foreach? src/lib/marble/GeoGraphicsScene.cpp <https://git.reviewboard.kde.org/r/118717/#comment44831> is parent guaranteed to be nonzero? src/lib/marble/GeoGraphicsScene.cpp <https://git.reviewboard.kde.org/r/118717/#comment44832> Makes no sense here. If parent was 0, parent->nodeType above would crash already. If parent was != 0, static_cast will always be != 0 also. src/lib/marble/GeoGraphicsScene.cpp <https://git.reviewboard.kde.org/r/118717/#comment44833> this block looks very much like the one above, can you extract a method? src/lib/marble/MarbleModel.h <https://git.reviewboard.kde.org/r/118717/#comment44834> shouldn't this be called addHighlightStyle? src/lib/marble/MarbleModel.cpp <https://git.reviewboard.kde.org/r/118717/#comment44835> highlight src/lib/marble/MarbleModel.cpp <https://git.reviewboard.kde.org/r/118717/#comment44836> what's the intention here? src/lib/marble/MarbleWidget.h <https://git.reviewboard.kde.org/r/118717/#comment44837> ideally signal names are passive src/lib/marble/geodata/handlers/dgml/DgmlMapTagHandler.cpp <https://git.reviewboard.kde.org/r/118717/#comment44838> highlightBrushColor src/lib/marble/graphicsview/GeoGraphicsItem.cpp <https://git.reviewboard.kde.org/r/118717/#comment44839> check is not needed (calling delete 0 is perfectly fine) src/lib/marble/graphicsview/GeoGraphicsItem_p.h <https://git.reviewboard.kde.org/r/118717/#comment44840> m_highlighted is not initialized src/lib/marble/layers/GeometryLayer.h <https://git.reviewboard.kde.org/r/118717/#comment44841> ideally a passive name parameter should be a const reference src/lib/marble/layers/GeometryLayer.cpp <https://git.reviewboard.kde.org/r/118717/#comment44842> specify any highlight src/lib/marble/layers/GeometryLayer.cpp <https://git.reviewboard.kde.org/r/118717/#comment44843> highlight src/lib/marble/layers/GeometryLayer.cpp <https://git.reviewboard.kde.org/r/118717/#comment44844> specifies src/lib/marble/layers/GeometryLayer.cpp <https://git.reviewboard.kde.org/r/118717/#comment44845> this is not sufficient. Either check the nodeType (preferably) or use a dynamic_cast src/lib/marble/layers/GeometryLayer.cpp <https://git.reviewboard.kde.org/r/118717/#comment44846> highlight - Dennis Nienhüser On Aug. 10, 2014, 12:39 p.m., Abhinav Gangwar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118717/ > ----------------------------------------------------------- > > (Updated Aug. 10, 2014, 12:39 p.m.) > > > Review request for Marble, Dennis Nienhüser, Torsten Rahn, and Thibaut Gridel. > > > Repository: marble > > > Description > ------- > > Modifications: > > 1. Added support to specify highlight color in dgml file. > 2. Highlighting is done in the following way: > > -> Input handler emits mouseClickGeoPostion( qreal lon, qreal lat, > GeoDataCoordinates::Unit unit ) whenever a click > is detected on map. > > -> Signal mouseClickGeoPostion(..) is connected to the signal > MarbleMap::announceMouseClick(..) which > further triggers the slot GeometryLayer::hadnleHighlight(..) which > iterates over the tree model to find the > placemarks, from all GeoDataDocuments in which any of its style map has an > entry for highlight styleId, which > were under mouse when the click event happened. > > -> After searching for all such placemarks GeometryLayer::hadnleHighlight(..) > emits signal > announceHighlight(QVector<GeoDataPlacemarks*> which is caught by > GeoGraphicsScene to execute the slot > GeoGraphicsScene::applyHighlight(QVector< GeoDataPlacemark* > > selectedPlacemark). > > -> GeoGraphicsScene::applyHighlight(..) we find all GeoGraphicsItems for a > plcamark and execute > GeoGraphicsItem::setHighlightStyle( GeoDataStyle *highlightStyle ) for > each graphics items if the corresponding > placemark's style url is set to a style map which has a entry for > highlight styleId. > > -> GeoGraphicsItem::setHighlightStyle(..) assigns this style to > GeoGraphicsItemPrivate::m_highlightStyle makes > GeoGraphicsItemPrivate::m_isHighlighted true. m_isHighlight decides which > style ( normal or highlight ) to use > to paint the graphics item. > > > That's it . Need feedback :) > > > Diffs > ----- > > data/maps/earth/political/political.dgml 3bf3b1b > src/lib/marble/GeoGraphicsScene.h 869c3d8 > src/lib/marble/GeoGraphicsScene.cpp f84c532 > src/lib/marble/MarbleInputHandler.cpp 6a46a0d > src/lib/marble/MarbleMap.h ab51f62 > src/lib/marble/MarbleMap.cpp 25007ac > src/lib/marble/MarbleModel.h 9643186 > src/lib/marble/MarbleModel.cpp 3bd408c > src/lib/marble/MarbleWidget.h dfc1aa7 > src/lib/marble/MarbleWidget.cpp d4c7ff4 > src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp b161d06 > src/lib/marble/geodata/handlers/dgml/DgmlAttributeDictionary.h cf4dd76 > src/lib/marble/geodata/handlers/dgml/DgmlAttributeDictionary.cpp 85f10fb > src/lib/marble/geodata/handlers/dgml/DgmlMapTagHandler.cpp fb7ff0f > src/lib/marble/geodata/scene/GeoSceneMap.h 498bb3a > src/lib/marble/geodata/scene/GeoSceneMap.cpp 7f94c11 > src/lib/marble/graphicsview/GeoGraphicsItem.h 653fef0 > src/lib/marble/graphicsview/GeoGraphicsItem.cpp 1227316 > src/lib/marble/graphicsview/GeoGraphicsItem_p.h 618cddf > src/lib/marble/layers/GeometryLayer.h 8d86aed > src/lib/marble/layers/GeometryLayer.cpp fbfe9a3 > src/lib/marble/MarbleInputHandler.h 58107e5 > > Diff: https://git.reviewboard.kde.org/r/118717/diff/ > > > Testing > ------- > > > File Attachments > ---------------- > > pn2DataNew.zip > > https://git.reviewboard.kde.org/media/uploaded/files/2014/06/13/e541a355-80be-40a8-8158-afea0677bd0e__pn2DataNew.zip > color10.png > > https://git.reviewboard.kde.org/media/uploaded/files/2014/06/13/f04644e8-6318-4b9a-a8d1-2747b405f73b__color10.png > color11.png > > https://git.reviewboard.kde.org/media/uploaded/files/2014/06/13/5f07bac3-3bcb-4086-b8d6-5b836b8f18dc__color11.png > > > Thanks, > > Abhinav Gangwar > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
