----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118885/#review60943 -----------------------------------------------------------
Ship it! Ship It! - Torsten Rahn On June 22, 2014, 8:09 p.m., Cruceru Calin-Cristian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118885/ > ----------------------------------------------------------- > > (Updated June 22, 2014, 8:09 p.m.) > > > 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 > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
