> On Jul 27, 2017, at 3:32 PM, Lubomir I. Ivanov <[email protected]> wrote: > > On 27 July 2017 at 19:26, Lubomir I. Ivanov <[email protected]> wrote: >> >> so, everything is pretty much in place including editing > > pull request made: > https://github.com/Subsurface-divelog/subsurface/pull/516 > > --------------------------------------- > > This is a side-by-side implementation of the Qt Location based map > next to our already existing Marble map. Both map implementations now > now exist in the codebase and can be swapped. > > It's a bit of patch bomb with ~120 commits and while it goes back and > forth here and there, the patches try to remain small and follow my > thinking process...still, if some refactoring is needed, let me know.
120 commits. Sure, let me review this real quick. Umm. Wow. > How it works: > > The best thing about this map widget is that it can be used for both > the mobile and desktop versions. The mobile version support is not > implemented, as it needs some decision making. The desktop version > support begins with the class dekstop-widgets/mapwidget.cpp which is a > QQuickWidget that loads > a QML file mobile-widgets/qml/MapWidget.qml. All the NO_MARBLE > abstraction is taken care of and MapWidget methods are used instead of > Globe methods. The code in mapwidget.cpp code is minimal as this class > desktop-version specific. The magic happens in the class > mobile-widgets/qmlmapwidgetheloper.cppwhich is the backend of the map So why is this in mobile-widgets and not in core ? If we run this in the desktop version as well, it shouldn't be in mobile-widgets > widget. It is instantiated in MapWidget.qml and both the mobile and > desktop version need to communicated with it. A new model is added - > qt-models/maplocationmodel.cppthat deals with the population of > markers on the map. That sounds good. > The map itself supports a context menu > (mobile-widgets/qml/MapWidgetContextMenu.qml) with easy support for > any number of options. Markers on this map can be edited by dragging > instead of a double-click. The right mouse button is not used, while > double-click is used to zoom in over a location or a marker. This is > easy to translate to the mobile version without abstraction, plus on > mobile the Map QML object should already support functionality like > pinch zoom. Extra cool > For the mobile version the infrastructure should be already in place. > the MapWidget needs to instantiated somewhere, some extra signal/slot > handling needs to be added and it should be good to go (hopefully). Famous last words :-) > How to build: > > - you need Qt 5.9 (i haven't tested with newer or on anything but Win32) > - build using cmake -DNO_MARBLE=1 > > Next steps would be: > > - receive code reviews That will be challenging, given the scope, but I'll sure try. > - receive feedback on the functionality That usually requires me pulling / building it. Which I don't really want to do until it had at least SOME level of review. I don't mind fixing stuff later (heck, that's all we ever do, right?), but at least the basics should be "roughly right" > - remove Marble from the codebase I'd wait with this for a little while until this had decent testing and feedback and is considered sufficiently feature complete. > also, it might be a good idea to test deployment before removing Marble. Yeah, no kidding /D _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
