<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/118076/">https://git.reviewboard.kde.org/r/118076/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been submitted with commit 47b087ad939b2f4ea70b06da74dd19ad5c097637 by Cruceru Calin-Cristian to branch master.</pre>
<br />
<p>- Commit Hook</p>
<br />
<p>On May 10th, 2014, 6:35 p.m. UTC, Cruceru Calin-Cristian wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Cruceru Calin-Cristian.</div>
<p style="color: grey;"><i>Updated May 10, 2014, 6:35 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
marble
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/apps/marble-ui/icons/draw-overlay.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/apps/marble-ui/marble.qrc <span style="color: grey">(39ad981)</span></li>
<li>src/lib/marble/FileManager.h <span style="color: grey">(ae648f7)</span></li>
<li>src/lib/marble/FileManager.cpp <span style="color: grey">(e5551a6)</span></li>
<li>src/lib/marble/FileViewWidget.cpp <span style="color: grey">(659a9d9)</span></li>
<li>src/lib/marble/MarbleMap.h <span style="color: grey">(7019bfa)</span></li>
<li>src/lib/marble/MarbleMap.cpp <span style="color: grey">(9859c02)</span></li>
<li>src/lib/marble/MarbleWidget.h <span style="color: grey">(067e9ce)</span></li>
<li>src/lib/marble/MarbleWidget.cpp <span style="color: grey">(f6baab4)</span></li>
<li>src/lib/marble/geodata/writers/kml/KmlOverlayTagWriter.cpp <span style="color: grey">(ccc13c0)</span></li>
<li>src/lib/marble/layers/TextureLayer.h <span style="color: grey">(90042cb)</span></li>
<li>src/plugins/render/annotate/AnnotatePlugin.h <span style="color: grey">(c4d1530)</span></li>
<li>src/plugins/render/annotate/AnnotatePlugin.cpp <span style="color: grey">(447e154)</span></li>
<li>src/plugins/render/annotate/AreaAnnotation.cpp <span style="color: grey">(7db109b)</span></li>
<li>src/plugins/render/annotate/CMakeLists.txt <span style="color: grey">(ac8b94e)</span></li>
<li>src/plugins/render/annotate/EditGroundOverlayDialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/render/annotate/EditGroundOverlayDialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/render/annotate/EditGroundOverlayDialog.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/render/annotate/GeoWidgetBubble.cpp <span style="color: grey">(f7acde7)</span></li>
<li>src/plugins/render/annotate/GroundOverlayFrame.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/render/annotate/GroundOverlayFrame.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/render/annotate/PlacemarkTextAnnotation.h <span style="color: grey">(996a2b2)</span></li>
<li>src/plugins/render/annotate/PlacemarkTextAnnotation.cpp <span style="color: grey">(bbfecea)</span></li>
<li>src/plugins/render/annotate/SceneGraphicsItem.h <span style="color: grey">(48e965b)</span></li>
<li>src/plugins/render/annotate/SceneGraphicsItem.cpp <span style="color: grey">(2c7f92b)</span></li>
<li>src/plugins/render/annotate/TextEditor.h <span style="color: grey">(3ae29a2)</span></li>
<li>src/plugins/render/annotate/TextEditor.cpp <span style="color: grey">(10d2ac6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118076/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/10/48689c21-49ab-4bae-b282-9204fa0b2b4e__draw-overlay.png">Draw overlay action picture</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>