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

[Commit Hook]

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


This review has been submitted with commit 177370067498b0229881c8fe2ceba099509f5750 by Dennis Nienhüser on behalf of Cruceru Calin-Cristian to branch master.

- Commit Hook


On May 30, 2014, 9:24 a.m., Cruceru Calin-Cristian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118405/
> -----------------------------------------------------------
> 
> (Updated May 30, 2014, 9:24 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> 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
> -----
> 
>   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/20140606/3a62dcf6/attachment-0001.html>