Merge 9bd1631103 into 4f7d567571

When computing the variations in a background thread, the
code has to work on a copy of the dive plan and the deco
state. Instead of passing a copy via a unique_ptr<>, simply
use value semantics when calling computeVariations().

This does an unnecessary copy of the deco state, when
computeVariations is not run in the background, but so what.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

map-widget/qml/MapWidget.qml

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
-import QtQuick 2.5
-import QtLocation 5.3
-import QtPositioning 5.3
+import QtQuick 
+import QtLocation 
+import QtPositioning 
 import org.subsurfacedivelog.mobile 1.0
 
 Item {
@@ -15,7 +15,7 @@ Item {
 		id: mapHelper
 		map: map
 		editMode: false
-		onSelectedDivesChanged: rootItem.selectedDivesChanged(list)
+		onSelectedDivesChanged: (list) => { rootItem.selectedDivesChanged(list) }
 		onEditModeChanged: editMessage.isVisible = editMode === true ? 1 : 0
 		onCoordinatesChanged: {}
 		Component.onCompleted: {
@@ -29,7 +29,6 @@ Item {
 		id: map
 		anchors.fill: parent
 		zoomLevel: defaultZoomIn
-
 		property var mapType
 		readonly property var defaultCenter: QtPositioning.coordinate(0, 0)
 		readonly property real defaultZoomIn: 12.0
@@ -41,12 +40,46 @@ Item {
 		property real newZoomOut: 1.0
 		property var clickCoord: QtPositioning.coordinate(0, 0)
 		property bool isReady: false
-
 		Component.onCompleted: isReady = true
 		onZoomLevelChanged: {
 			if (isReady)
 				mapHelper.calculateSmallCircleRadius(map.center)
 		}
+        property geoCoordinate startCentroid
+        startCentroid: newCenter
+
+        PinchHandler {
+            id: pinch
+            target: null
+            onActiveChanged: if (active) {
+                map.startCentroid = map.toCoordinate(pinch.centroid.position, false)
+            }
+            onScaleChanged: (delta) => {
+                map.zoomLevel += Math.log2(delta)
+                map.alignCoordinateToPoint(map.startCentroid, pinch.centroid.position)
+            }
+            onRotationChanged: (delta) => {
+                map.bearing -= delta
+                map.alignCoordinateToPoint(map.startCentroid, pinch.centroid.position)
+            }
+            grabPermissions: PointerHandler.TakeOverForbidden
+        }
+        WheelHandler {
+            id: wheel
+            // workaround for QTBUG-87646 / QTBUG-112394 / QTBUG-112432:
+            // Magic Mouse pretends to be a trackpad but doesn't work with PinchHandler
+            // and we don't yet distinguish mice and trackpads on Wayland either
+            acceptedDevices: Qt.platform.pluginName === "cocoa" || Qt.platform.pluginName === "wayland"
+                             ? PointerDevice.Mouse | PointerDevice.TouchPad
+                             : PointerDevice.Mouse
+            rotationScale: 1/120
+            property: "zoomLevel"
+        }
+        DragHandler {
+            id: drag
+            target: null
+            onTranslationChanged: (delta) => map.pan(-delta.x, -delta.y)
+        }
 
 		MapItemView {
 			id: mapItemView
@@ -67,7 +100,9 @@ Item {
 					}
 					MouseArea {
 						drag.target: (mapHelper.editMode && model.isSelected) ? mapItem : undefined
-						anchors.fill: parent
+            anchors.fill: parent
+            hoverEnabled: true
+
 						onClicked: {
 							if (!mapHelper.editMode && model.divesite)
 								mapHelper.selectedLocationChanged(model.divesite)
@@ -122,8 +157,8 @@ Item {
 
 		MouseArea {
 			anchors.fill: parent
-			onPressed: { map.stopZoomAnimations(); mouse.accepted = false }
-			onWheel: { map.stopZoomAnimations(); wheel.accepted = false }
+			onPressed: (mouse) => { map.stopZoomAnimations(); mouse.accepted = false }
+			onWheel: (wheel) => { map.stopZoomAnimations(); wheel.accepted = false }
 			onDoubleClicked: map.doubleClickHandler(map.toCoordinate(Qt.point(mouseX, mouseY)))
 		}
 

map-widget/qmlmapwidgethelper.cpp

@@ -4,20 +4,20 @@
 #include <QDebug>
 #include <QVector>
 
-#include "qmlmapwidgethelper.h"
 #include "core/divefilter.h"
 #include "core/divelist.h"
 #include "core/divelog.h"
 #include "core/divesite.h"
 #include "core/qthelper.h"
 #include "core/range.h"
-#include "qt-models/maplocationmodel.h"
+#include "qmlmapwidgethelper.h"
 #include "qt-models/divelocationmodel.h"
+#include "qt-models/maplocationmodel.h"
 #ifndef SUBSURFACE_MOBILE
 #include "desktop-widgets/mapwidget.h"
 #endif
 
-#define SMALL_CIRCLE_RADIUS_PX            26.0
+#define SMALL_CIRCLE_RADIUS_PX 26.0
 
 MapWidgetHelper::MapWidgetHelper(QObject *parent) : QObject(parent)
 {
@@ -44,7 +44,7 @@ void MapWidgetHelper::centerOnDiveSite(struct dive_site *ds)
 	} else {
 		// dive site with GPS
 		m_mapLocationModel->setSelected(ds);
-		QGeoCoordinate dsCoord (ds->location.lat.udeg * 0.000001, ds->location.lon.udeg * 0.000001);
+		QGeoCoordinate dsCoord(ds->location.lat.udeg * 0.000001, ds->location.lon.udeg * 0.000001);
 		QMetaObject::invokeMethod(m_map, "centerOnCoordinate", Q_ARG(QVariant, QVariant::fromValue(dsCoord)));
 	}
 }
@@ -69,7 +69,7 @@ void MapWidgetHelper::centerOnSelectedDiveSite()
 	// find the most top-left and bottom-right dive sites on the map coordinate system.
 	qreal minLat = 0.0, minLon = 0.0, maxLat = 0.0, maxLon = 0.0;
 	int count = 0;
-	for(struct dive_site *dss: selDS) {
+	for (struct dive_site *dss : selDS) {
 		if (!has_location(&dss->location))
 			continue;
 		qreal lat = dss->location.lat.udeg * 0.000001;
@@ -92,7 +92,7 @@ void MapWidgetHelper::centerOnSelectedDiveSite()
 	// Pass coordinates to QML, either as a point or as a rectangle.
 	// If we didn't find any coordinates, do nothing.
 	if (count == 1) {
-		QGeoCoordinate dsCoord (selDS[0]->location.lat.udeg * 0.000001, selDS[0]->location.lon.udeg * 0.000001);
+		QGeoCoordinate dsCoord(selDS[0]->location.lat.udeg * 0.000001, selDS[0]->location.lon.udeg * 0.000001);
 		QMetaObject::invokeMethod(m_map, "centerOnCoordinate", Q_ARG(QVariant, QVariant::fromValue(dsCoord)));
 	} else if (count > 1) {
 		QGeoCoordinate coordTopLeft(minLat, minLon);
@@ -134,7 +134,7 @@ void MapWidgetHelper::selectedLocationChanged(struct dive_site *ds_in)
 		return;
 	QGeoCoordinate locationCoord = location->coordinate;
 
-	for (auto [idx, dive]: enumerated_range(divelog.dives)) {
+	for (auto [idx, dive] : enumerated_range(divelog.dives)) {
 		struct dive_site *ds = dive->dive_site;
 		if (!ds || !ds->has_gps_location())
 			continue;
@@ -151,9 +151,9 @@ void MapWidgetHelper::selectedLocationChanged(struct dive_site *ds_in)
 	}
 	int last; // get latest dive chronologically
 	if (!selectedDiveIds.isEmpty()) {
-		 last = selectedDiveIds.last();
-		 selectedDiveIds.clear();
-		 selectedDiveIds.append(last);
+		last = selectedDiveIds.last();
+		selectedDiveIds.clear();
+		selectedDiveIds.append(last);
 	}
 #endif
 	emit selectedDivesChanged(selectedDiveIds);
@@ -162,7 +162,7 @@ void MapWidgetHelper::selectedLocationChanged(struct dive_site *ds_in)
 void MapWidgetHelper::selectVisibleLocations()
 {
 	QList<int> selectedDiveIds;
-	for (auto [idx, dive]: enumerated_range(divelog.dives)) {
+	for (auto [idx, dive] : enumerated_range(divelog.dives)) {
 		struct dive_site *ds = dive->dive_site;
 		if (!ds || ds->has_gps_location())
 			continue;
@@ -171,7 +171,7 @@ void MapWidgetHelper::selectVisibleLocations()
 		QGeoCoordinate dsCoord(latitude, longitude);
 		QPointF point;
 		QMetaObject::invokeMethod(m_map, "fromCoordinate", Q_RETURN_ARG(QPointF, point),
-		                          Q_ARG(QGeoCoordinate, dsCoord));
+					  Q_ARG(QGeoCoordinate, dsCoord));
 		if (!qIsNaN(point.x()))
 #ifndef SUBSURFACE_MOBILE // indices on desktop
 			selectedDiveIds.append(idx);
@@ -181,9 +181,9 @@ void MapWidgetHelper::selectVisibleLocations()
 	}
 	int last; // get latest dive chronologically
 	if (!selectedDiveIds.isEmpty()) {
-		 last = selectedDiveIds.last();
-		 selectedDiveIds.clear();
-		 selectedDiveIds.append(last);
+		last = selectedDiveIds.last();
+		selectedDiveIds.clear();
+		selectedDiveIds.append(last);
 	}
 #endif
 	emit selectedDivesChanged(selectedDiveIds);
@@ -205,11 +205,11 @@ void MapWidgetHelper::calculateSmallCircleRadius(QGeoCoordinate coord)
 {
 	QPointF point;
 	QMetaObject::invokeMethod(m_map, "fromCoordinate", Q_RETURN_ARG(QPointF, point),
-	                          Q_ARG(QGeoCoordinate, coord));
+				  Q_ARG(QGeoCoordinate, coord));
 	QPointF point2(point.x() + SMALL_CIRCLE_RADIUS_PX, point.y());
 	QGeoCoordinate coord2;
 	QMetaObject::invokeMethod(m_map, "toCoordinate", Q_RETURN_ARG(QGeoCoordinate, coord2),
-	                          Q_ARG(QPointF, point2));
+				  Q_ARG(QPointF, point2));
 	m_smallCircleRadius = coord2.distanceTo(coord);
 }
 
@@ -251,8 +251,8 @@ QString MapWidgetHelper::pluginObject()
 {
 	QString lang = getUiLanguage().replace('_', '-');
 	QString cacheFolder = QString::fromStdString(system_default_directory() + "/googlemaps").replace("\\", "/");
-	return QStringLiteral("import QtQuick 2.0;"
-			      "import QtLocation 5.3;"
+	return QStringLiteral("import QtQuick;"
+			      "import QtLocation;"
 			      "Plugin {"
 			      "    id: mapPlugin;"
 			      "    name: 'googlemaps';"
@@ -263,5 +263,6 @@ QString MapWidgetHelper::pluginObject()
 			      "            console.warn('MapWidget.qml: cannot find a plugin named: ' + name);"
 			      "        }"
 			      "    }"
-			      "}").arg(lang, cacheFolder);
+			      "}")
+		.arg(lang, cacheFolder);
 }

qt-models/diveplannermodel.cpp

@@ -1118,38 +1118,25 @@ void DivePlannerPointsModel::updateDiveProfile()
 	if (diveplan.is_empty())
 		return;
 
+	// For calculating variations, we need a copy of the plan. We have to copy _before_
+	// calling plan(), because that adds deco stops.
+	bool computeVariations = isPlanner() && shouldComputeVariations();
+	struct diveplan plan_copy;
+	if (computeVariations)
+		plan_copy = diveplan;
+
 	deco_state_cache cache;
 	struct deco_state plan_deco_state;
 
 	plan(&plan_deco_state, diveplan, d, dcNr, decotimestep, cache, isPlanner(), false);
 	updateMaxDepth();
 
-	if (isPlanner() && shouldComputeVariations()) {
-		auto plan_copy = std::make_unique<struct diveplan>();
-		lock_planner();
-		*plan_copy = diveplan;
-		unlock_planner();
+	if (computeVariations) {
 #ifdef VARIATIONS_IN_BACKGROUND
-		// Since we're calling computeVariations asynchronously and plan_deco_state is allocated
-		// on the stack, it must be copied and freed by the worker-thread.
-		auto deco_copy = std::make_unique<deco_state>(plan_deco_state);
-
-		// Ideally, we would pass the unique_ptrs to the lambda for QtConcurrent::run().
-		// This, in principle, can be done as such:
-		//	[ptr = std::move(ptr)] () mutable { f(std::move(ptr)) };
-		// However, this make the lambda uncopyable and QtConcurrent::run() sadly
-		// uses copy semantics.
-		// So let's be pragmatic and do a release/reaquire pair.
-		// Somewhat disappointing, but what do you want to do?
-		// Note 1: this is now not exception safe, but Qt doesn't support
-		//	exceptions anyway.
-		// Note 2: We also can't use the function / argument syntax of QtConcurrent::run(),
-		//	because it likewise uses copy-semantics. How annoying.
-		QtConcurrent::run([this, plan = plan_copy.release(), deco = deco_copy.release()] ()
-				  { this->computeVariationsFreeDeco(std::unique_ptr<struct diveplan>(plan),
-								    std::unique_ptr<deco_state>(deco)); });
+		QtConcurrent::run([this, plan = std::move(plan_copy), deco = plan_deco_state] ()
+				  { this->computeVariations(std::move(plan), deco); });
 #else
-		computeVariations(std::move(plan_copy), &plan_deco_state);
+		computeVariations(std::move(plan_copy), plan_deco_state);
 #endif
 		final_deco_state = plan_deco_state;
 	}
@@ -1194,12 +1181,6 @@ int DivePlannerPointsModel::analyzeVariations(const std::vector<decostop> &min,
 	return (leftsum + rightsum) / 2;
 }
 
-void DivePlannerPointsModel::computeVariationsFreeDeco(std::unique_ptr<struct diveplan> original_plan, std::unique_ptr<struct deco_state> previous_ds)
-{
-	computeVariations(std::move(original_plan), previous_ds.get());
-	// Note: previous ds automatically free()d by virtue of being a unique_ptr.
-}
-
 // Return reference to second to last element.
 // Caller is responsible for checking that there are at least two elements.
 template <typename T>
@@ -1208,17 +1189,16 @@ auto &second_to_last(T &v)
 	return *std::prev(std::prev(v.end()));
 }
 
-void DivePlannerPointsModel::computeVariations(std::unique_ptr<struct diveplan> original_plan, const struct deco_state *previous_ds)
+void DivePlannerPointsModel::computeVariations(struct diveplan original_plan, struct deco_state ds)
 {
 	// nothing to do unless there's an original plan
-	if (!original_plan)
+	if (original_plan.dp.empty())
 		return;
 
 	auto dive = std::make_unique<struct dive>();
 	copy_dive(d, dive.get());
 	deco_state_cache cache, save;
 	struct diveplan plan_copy;
-	struct deco_state ds = *previous_ds;
 
 	int my_instance = ++instanceCounter;
 	save.cache(&ds);
@@ -1236,7 +1216,7 @@ void DivePlannerPointsModel::computeVariations(std::unique_ptr<struct diveplan>
 		depth_units = tr("ft");
 	}
 
-	plan_copy = *original_plan;
+	plan_copy = original_plan;
 	if (plan_copy.dp.size() < 2)
 		return;
 	if (my_instance != instanceCounter)
@@ -1244,7 +1224,7 @@ void DivePlannerPointsModel::computeVariations(std::unique_ptr<struct diveplan>
 	auto original = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false);
 	save.restore(&ds, false);
 
-	plan_copy = *original_plan;
+	plan_copy = original_plan;
 	second_to_last(plan_copy.dp).depth.mm += delta_depth.mm;
 	plan_copy.dp.back().depth.mm += delta_depth.mm;
 	if (my_instance != instanceCounter)
@@ -1252,6 +1232,7 @@ void DivePlannerPointsModel::computeVariations(std::unique_ptr<struct diveplan>
 	auto deeper = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false);
 	save.restore(&ds, false);
 
+	plan_copy = original_plan;
 	second_to_last(plan_copy.dp).depth.mm -= delta_depth.mm;
 	plan_copy.dp.back().depth.mm -= delta_depth.mm;
 	if (my_instance != instanceCounter)
@@ -1259,13 +1240,14 @@ void DivePlannerPointsModel::computeVariations(std::unique_ptr<struct diveplan>
 	auto shallower = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false);
 	save.restore(&ds, false);
 
-	plan_copy = *original_plan;
+	plan_copy = original_plan;
 	plan_copy.dp.back().time += delta_time.seconds;
 	if (my_instance != instanceCounter)
 		return;
 	auto longer = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false);
 	save.restore(&ds, false);
 
+	plan_copy = original_plan;
 	plan_copy.dp.back().time -= delta_time.seconds;
 	if (my_instance != instanceCounter)
 		return;
@@ -1308,15 +1290,16 @@ void DivePlannerPointsModel::createPlan(bool saveAsNew)
 	removeDeco();
 	createTemporaryPlan();
 
+	// For calculating variations, we need a copy of the plan. We have to copy _before_
+	// calling plan(), because that adds deco stops.
+	struct diveplan plan_copy;
+	if (shouldComputeVariations())
+		plan_copy = diveplan;
+
 	plan(&ds_after_previous_dives, diveplan, d, dcNr, decotimestep, cache, isPlanner(), true);
 
-	if (shouldComputeVariations()) {
-		auto plan_copy = std::make_unique<struct diveplan>();
-		lock_planner();
-		*plan_copy = diveplan;
-		unlock_planner();
-		computeVariations(std::move(plan_copy), &ds_after_previous_dives);
-	}
+	if (shouldComputeVariations())
+		computeVariations(std::move(plan_copy), ds_after_previous_dives);
 
 	// Fixup planner notes.
 	if (current_dive && d->id == current_dive->id) {

qt-models/diveplannermodel.h

@@ -131,8 +131,7 @@ private:
 	void createTemporaryPlan();
 	struct diveplan diveplan;
 	void computeVariationsDone(QString text);
-	void computeVariations(std::unique_ptr<struct diveplan> plan, const struct deco_state *ds);
-	void computeVariationsFreeDeco(std::unique_ptr<struct diveplan> plan, std::unique_ptr<struct deco_state> ds);
+	void computeVariations(struct diveplan plan, struct deco_state ds); // Note: works on copies of plan and ds
 	int analyzeVariations(const std::vector<decostop> &min, const std::vector<decostop> &mid, const std::vector<decostop> &max, const char *unit);
 	struct dive *d;
 	int dcNr;

scripts/get-dep-lib.sh

@@ -169,8 +169,9 @@ for package in "${PACKAGES[@]}" ; do
 			git_checkout_library breeze-icons $CURRENT_BREEZE_ICONS https://github.com/kde/breeze-icons.git
 			;;
 		googlemaps)
-			git_checkout_library googlemaps master https://github.com/Subsurface/googlemaps.git
-			;;
+			#git_checkout_library googlemaps master https://github.com/Subsurface/googlemaps.git
+			git_checkout_library googlemaps master https://github.com/vladest/googlemaps.git
+      ;;
 		hidapi)
 			git_checkout_library hidapi master https://github.com/libusb/hidapi.git
 			;;