[Marble-devel] Review Request 118405: Added polygon moving/node deletion/drawing polygon holes features to annotate plugin

Fri May 30 09:24:44 UTC 2014

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

(Updated May 30, 2014, 12:24 p.m.)

Review request for Marble.

Changes
-------

Forgot to add the changes to GeoDataCoordinates from my working repository. Now it should work.

Repository: marble

Description
-------

I added the following features to polygon editing:
- moving the whole polygon. I had already submitted a review request for this, but Dennis mentioned some issues so I discarded it and added the changes to this patch. It is important to mention that this feature does not fully work - there are some problems when moving the polygon near the poles.
- node selection and deletion. I increased the size of the bullets representing the nodes in order to ease their selection. The selected ones are painted with a different color. 
- adding holes to polygons (more precisely, adding inner boundaries). For this I added a new option in the menu, "Add Polygon Hole". It works similar to drawing polygons: check it, draw the hole then un-check it so that it knows that you finished drawing the hole. Hopefully, when I will start working on the user interface, this will not remain like this - it will be more user-friendly.

Also, I added the following new actions (grouped into two kind of menus: polygon menu - which shows the options for the entire polygon and node menu which shows the options for the right clicked node):
- select/unselect node: this comes to complete the node selection done by clicking using the left button of the mouse.
- delete node: deletes the right-clicked node. If the polygon (or inner boundary ring) has less than 3 nodes after deletion, the whole polygon(inner boundary) will be removed. I think having only a line on the screen is not what a user may expect - but if you find it may have a purpose, it is easy to modify this. Also, here I dealt with the situation in which after deleting a node from the polygon's outer boundary, one of its inner boundaries would not be an inner boundary anymore (they would intersect). I used a QMessageBox warning to announce the user about this. Initially I tried to disable the action when this happened ("Delete node" and "Delete selected nodes") but this proved very inefficient in my opinion since the whole deletion should have been "simulated" before showing the menu. It is important to mention, also, that there are still some problems with this testing and I could not figure out so far what causes that: sometimes the function which tests if the polygon's outer boundary contains all the coordinates of its inner boundaries returns true even though it doesn't (or at least it doesn't look like from what is painted on the globe, e.g: http://imgur.com/a/KCoXO)

- unselect all nodes: this is an action from the polygon rmb menu. I think it its useful if you have too many nodes selected and you want to select other in order to delete them at the same time, for example.
- delete selected nodes: it behaves similar to delete node, only that is deletes all selected ones.
- remove polygon: removes the entire polygon.

I also intensively tried to clean the code and make it look more organized and better structured. I've also written some documentation to some classes and added many comments - I hope they don't bother; many of them I've written for me, to better understand the code, but may be useful for others too.

I'm really looking forward to seeing your reviews and issues, as well as potential recommendations for the features which are not fully working.

Diffs (updated)
-----

  src/lib/marble/geodata/data/GeoDataCoordinates.h d151a66 
  src/lib/marble/geodata/data/GeoDataCoordinates.cpp 576bf1d 
  src/plugins/render/annotate/AnnotatePlugin.h 3d8b9b5 
  src/plugins/render/annotate/AnnotatePlugin.cpp e1add48 
  src/plugins/render/annotate/AreaAnnotation.h 105e2e7 
  src/plugins/render/annotate/AreaAnnotation.cpp 0beff80 
  src/plugins/render/annotate/CMakeLists.txt 387a92d 
  src/plugins/render/annotate/EditGroundOverlayDialog.ui d25d74a 
  src/plugins/render/annotate/GeoWidgetBubble.h 944f3e3 
  src/plugins/render/annotate/GeoWidgetBubble.cpp 4837f96 
  src/plugins/render/annotate/GroundOverlayFrame.h 4b1337d 
  src/plugins/render/annotate/GroundOverlayFrame.cpp 30b1d0b 
  src/plugins/render/annotate/PlacemarkTextAnnotation.h 492bd4b 
  src/plugins/render/annotate/PlacemarkTextAnnotation.cpp 2d08ff7 
  src/plugins/render/annotate/SceneGraphicsItem.h 3e9ea08 
  src/plugins/render/annotate/SceneGraphicsItem.cpp a5d9da7 
  src/plugins/render/annotate/SceneGraphicsTypes.h PRE-CREATION 
  src/plugins/render/annotate/SceneGraphicsTypes.cpp PRE-CREATION 
  src/plugins/render/annotate/TextEditor.cpp 6820330 

Diff: https://git.reviewboard.kde.org/r/118405/diff/

Testing
-------

Thanks,

Cruceru Calin-Cristian

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