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