QtLocation map updates

Dirk Hohndel dirk at hohndel.org
Thu Jul 27 16:41:20 PDT 2017


> On Jul 27, 2017, at 4:07 PM, Lubomir I. Ivanov <neolit123 at gmail.com> wrote:
> 
> On 28 July 2017 at 01:54, Dirk Hohndel <dirk at hohndel.org> wrote:
>>> 
>>> 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
> 
> because Qt Location has mobile as it's main platform target.
> also, this code works for both mobile and desktop.

We have other code like this under core, e.g. core/subsurface-qt or 
core/downloadfromdcthread.cpp
One might argue that the code for the map maybe should get its own
directory, like profile-widget

Fundamentally, when building for desktop, nothing in mobile-widgets 
should be needed and vice versa for desktop-widgets when building 
Subsurface-mobile

This isn't a show stopper, we can move things later. Just saying that
this would be my preference (unless there is some QML weirdness
that prevents that).

> as some of the few early commits suggest we can move this to a new
> folder - e.g. shared-widgets.
> but i wonder how much my commit history will break with this
> change...maybe a patch on top can do that in the cleanest fashion.

Yes, that's fine. I'm just starting the review - let's see if there are things
that have me more worried :-)

>>> 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 :-)
> 
> indeed.
> i just removed some stuff from the PR, as the mobile version was broken.
> 
> which confirms that i haven't tested it all.

Ha :-)

/D


More information about the subsurface mailing list