----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118076/ -----------------------------------------------------------
Review request for Marble. Repository: marble Description ------- I started from this existing patch submitted by Adrian Draghici who worked on implementing (alongside other things) the overlay editing within marble, during GSoC 2013: https://git.reviewboard.kde.org/r/112454/. Most of it was simply to adapt since there were little changes to the modified files since last year. The harder part was after I finished fully adapting every piece of code and it even compiled. It was getting a segmentation error when trying to modify an overlay after it has been projected on the map. I finally found the problem (I wrote a comment within the code, where the missing line was) and it seems to work pretty good now. Or at least there are no more SIGSEGVs. One thing I am especially waiting opinions is the current functionality of moving an overlay. At the moment, if one wants to move an overlay, if they click the overlay, they will be prompted with the EditGroundOverlayDialog. When exiting from this dialog, the overlay will be selected, so this is the moment one could drag the overlay to another position on the map or to right click it and catch the event which shows the two options: remove and edit. So, here are 2 issues: - it is pretty annoying to cancel the EditGroundOverlayDialog every time you want to move the overlay by dragging it. - the rmbMenu which is poped-up when you right-click the selected overlay shows the edit options which at least at the moment is useless and redundant since you could obtain the same result by simply clicking the overlay. Two options regarding these issues would be, in my opinion: - since the EditGroundOverlayDialog includes the possibility to modify the coordinates of the overlay, the dragging could be replaced by simply introducing the coordinates you want to move the overlay to (so to not catch the mouse move event anymore). At the same time, the usual user may find easier to drag and drop the overlay and since we have to look at this from the user's point of view, the above solution may not be suitable. So I'm waiting from you to tell me how you, as a Marble user, would find easily and intuitively to both move the overlay by drag and droping and to be prompted by the dialog (to change name, description, etc) and, why not, some implementation details. - for the rmbMenu, if there are no future plans of extending its actions, we may totally remove it and add another event (for example pressing delete, or some other key) to remove the overlay (which, by the way, is already implemented: View->Remove item). To conclude, I'm waiting for both an overall opinion (since the whole implementation will be wrapped up within an Dockwidget or some other menu, alongside Polygons Editing tool on which I will proceed to work, so there will still be modifications) as well as a more detailed one, so I can easily fix the mentioned issues. Diffs ----- src/apps/marble-ui/icons/draw-overlay.png PRE-CREATION src/apps/marble-ui/marble.qrc 39ad981 src/lib/marble/FileManager.h ae648f7 src/lib/marble/FileManager.cpp e5551a6 src/lib/marble/FileViewWidget.cpp 659a9d9 src/lib/marble/MarbleMap.h 7019bfa src/lib/marble/MarbleMap.cpp 9859c02 src/lib/marble/MarbleWidget.h 067e9ce src/lib/marble/MarbleWidget.cpp f6baab4 src/lib/marble/geodata/writers/kml/KmlOverlayTagWriter.cpp ccc13c0 src/lib/marble/layers/TextureLayer.h 90042cb src/plugins/render/annotate/AnnotatePlugin.h c4d1530 src/plugins/render/annotate/AnnotatePlugin.cpp 447e154 src/plugins/render/annotate/AreaAnnotation.cpp 7db109b src/plugins/render/annotate/CMakeLists.txt ac8b94e src/plugins/render/annotate/EditGroundOverlayDialog.h PRE-CREATION src/plugins/render/annotate/EditGroundOverlayDialog.cpp PRE-CREATION src/plugins/render/annotate/EditGroundOverlayDialog.ui PRE-CREATION src/plugins/render/annotate/GeoWidgetBubble.cpp f7acde7 src/plugins/render/annotate/GroundOverlayFrame.h PRE-CREATION src/plugins/render/annotate/GroundOverlayFrame.cpp PRE-CREATION src/plugins/render/annotate/PlacemarkTextAnnotation.h 996a2b2 src/plugins/render/annotate/PlacemarkTextAnnotation.cpp bbfecea src/plugins/render/annotate/SceneGraphicsItem.h 48e965b src/plugins/render/annotate/SceneGraphicsItem.cpp 2c7f92b src/plugins/render/annotate/TextEditor.h 3ae29a2 src/plugins/render/annotate/TextEditor.cpp 10d2ac6 Diff: https://git.reviewboard.kde.org/r/118076/diff/ Testing ------- Thanks, Cruceru Calin-Cristian
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
