[Marble-devel] Review Request 118717: Added 'highlight on click' feature.

Sun Jul 13 16:23:32 UTC 2014

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118717/
-----------------------------------------------------------

(Updated July 13, 2014, 10:53 p.m.)

Review request for Marble, Dennis Nienhüser, Torsten Rahn, and Thibaut Gridel.

Changes
-------

The previous patch fails to highlight the correct country if we click on country's name. This happened because of the following reason:
In MarbleDefaultInputHandler::lmbTimeout(), lmbeRequest(..) was emitted before the clicked geo coordinates could be determined. The order of execution was :

emit lmbRequest(d->m_leftPressedX, d->m_leftPressedY);
MarbleInputHandler::d->m_marblePresenter->map()->geoCoordinates( d->m_leftPressedX, d->m_leftPressedY,
                                                                        clickedLon, clickedLat,
                                                                        GeoDataCoordinates::Degree );

Screen positions d->m_leftPressedX and d->m_leftPressedY are used to determine the clicked geo coordinates clickedLon and clickedLat which are used to find placemarks which should be highlighted.

Problem:
Due to emission of lmbRequest(..),  a pop up window showing placemark description is displayed. The display of pop up moves the map. Due to movement of map the screen position ( d->m_leftPressedX, d->m_leftPressedY ) represent some different geo coordinates now compared to the geo coordinates it represented before movement of map. So, if the map moves the same screen position will represent different geo coordinates before and after map movement.
Different and wrong geo coordinate will result in wrong determination of placemarks which should be highlighted.

Fix:
Fixing it was simple - I just reverted the execution sequence of the above two code lines and saved the geo coordinate before the lmbRequest(..) is emitted and map moves.

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 (updated)
-----

  src/lib/marble/MarbleWidget.h dfc1aa7 
  src/lib/marble/MarbleWidget.cpp d4c7ff4 
  src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp b161d06 
  src/lib/marble/MarbleInputHandler.h 58107e5 
  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 d240bac 
  data/maps/earth/political/political.dgml 3bf3b1b 
  src/lib/marble/GeoGraphicsScene.h 869c3d8 
  src/lib/marble/GeoGraphicsScene.cpp f84c532 
  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 

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20140713/55a0c7ae/attachment.html>