[Marble-devel] Review Request 118885: Added 'Merging Nodes' feature

[Cruceru Calin-Cristian]

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

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


Repository: marble


Description
-------

Added Merging Nodes option on polygons. 
I designed this new feature as a new "state" of the editing mode. This means that one cannot merge nodes and draw polygon holes at the same time since these are two different states. Initially I tried to implement a basic functionality by adding a new option within the RMB menu on polygons but it would have been pretty hard to decide which nodes to merge (the two selected ones? And what if I need to have more than two marked as selected?).
So, to enter this new 'Merging Nodes' state simply check the 'Merging Nodes' entry in the View menu (also make sure that there is only one option checked from the polygons editing tools - I let as a TODO not allow having more than one checked, but this is not as trivial as it may sound, since I also have to take into consideration the other editing mode tools like adding placemarks, etc). Then simply start clicking the nodes you want to merge in pairs of two. I also treated all special cases like trying to merge a node from the outer boundary with a node from the polygon's inner boundary or to merge two nodes from two different inner boundaries (this will result in a warning). The visual effect when merging nodes is basic so far: I only draw differently the first selected node (I am waiting suggestions on how would you like this merging to "look", visually speaking). 
One important thing to mention is that I did not restrict the merging on neighbor nodes because I thought that one may also find useful how it works on nodes which are not neighbor.


This patch also contains a new design for handling events in Annotate Plugin class which, in my opinion, is more intuitive and easily extensible (I even experienced this while implementing 'Merging Nodes'). To describe this new approach I have to say first that I was inspired by Adam's changes in MarbleInputHandler. Since AnnotatePlugin::eventFilter would have become larger and larger as new features are added, something for sure should have been modified. Tell me if you think the split technique I approached is a good solution to this.


Diffs
-----

  src/plugins/render/annotate/AnnotatePlugin.h 0b426a7 
  src/plugins/render/annotate/AnnotatePlugin.cpp d6e5e8a 
  src/plugins/render/annotate/AreaAnnotation.h d349a3c 
  src/plugins/render/annotate/AreaAnnotation.cpp 3952a91 
  src/plugins/render/annotate/GroundOverlayFrame.h 5037430 
  src/plugins/render/annotate/PlacemarkTextAnnotation.h cdc9a0e 

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


Testing
-------

I tested most of the possibilities: simple polygon with only outer boundary, polygon with an inner boundary, polygon with multiple inner boundaries, polygon with selected nodes on the outer boundary, polygon with selected nodes on each inner boundary, etc. In all my tests it worked as expected. However, the chances of crash increase proportionally with the number of features added to this edit mode, so some more testing is always good.


Thanks,

Cruceru Calin-Cristian

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