----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118076/#review57844 -----------------------------------------------------------
Ship it! Ship It! - Torsten Rahn On May 10, 2014, 6:35 p.m., Cruceru Calin-Cristian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118076/ > ----------------------------------------------------------- > > (Updated May 10, 2014, 6:35 p.m.) > > > 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 > ------- > > > File Attachments > ---------------- > > Draw overlay action picture > > https://git.reviewboard.kde.org/media/uploaded/files/2014/05/10/48689c21-49ab-4bae-b282-9204fa0b2b4e__draw-overlay.png > > > Thanks, > > Cruceru Calin-Cristian > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
