QtLocation map updates

Dirk Hohndel dirk at hohndel.org
Thu Jul 27 15:54:05 PDT 2017


> On Jul 27, 2017, at 3:32 PM, Lubomir I. Ivanov <neolit123 at gmail.com> wrote:
> 
> On 27 July 2017 at 19:26, Lubomir I. Ivanov <neolit123 at gmail.com> 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


More information about the subsurface mailing list