Skip to content

[WIP] Address feedback on locking access to CameraZoomConfig #894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
maerki merged 7 commits into from
Dec 10, 2025
Merged

[WIP] Address feedback on locking access to CameraZoomConfig #894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
14212b6
Initial plan
Copilot Dec 4, 2025
7db85ec
Fix nested locking by creating unlocked helper methods
Copilot Dec 4, 2025
5137a0f
Changes before error encountered
Copilot Dec 4, 2025
fffb3ce
delete codeql
maerki Dec 10, 2025
1a071da
delete codeql
maerki Dec 10, 2025
363ad6a
delete codeql
maerki Dec 10, 2025
1e3f522
Delete _codeql_build_dir/_deps directory
maerki Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions shared/src/map/camera/MapCamera3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1817,17 +1817,25 @@ void MapCamera3d::updateZoom(double zoom_) {

std::lock_guard<std::recursive_mutex> lock(paramMutex);
zoom = std::clamp(zoom_, zoomMax, zoomMin);
cameraVerticalDisplacement = getCameraVerticalDisplacement();
cameraPitch = getCameraPitch();
cameraVerticalDisplacement = getCameraVerticalDisplacementUnlocked();
cameraPitch = getCameraPitchUnlocked();
}

double MapCamera3d::getCameraVerticalDisplacement() {
std::lock_guard<std::recursive_mutex> lock(paramMutex);
return getCameraVerticalDisplacementUnlocked();
}

double MapCamera3d::getCameraVerticalDisplacementUnlocked() {
return valueForZoom(cameraZoomConfig.verticalDisplacementInterpolationValues, zoom);
}

double MapCamera3d::getCameraPitch() {
std::lock_guard<std::recursive_mutex> lock(paramMutex);
return getCameraPitchUnlocked();
}

double MapCamera3d::getCameraPitchUnlocked() {
return valueForZoom(cameraZoomConfig.pitchInterpolationValues, zoom);
}

Expand Down
6 changes: 5 additions & 1 deletion shared/src/map/camera/MapCamera3d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@ class MapCamera3d : public MapCameraInterface,
double getCameraVerticalDisplacement();
double getCameraPitch();

private:
// Unlocked versions - caller must hold paramMutex
double getCameraVerticalDisplacementUnlocked();
double getCameraPitchUnlocked();

Vec2F calculateDistance(double latTopLeft, double lonTopLeft, double latBottomRight, double lonBottomRight);
double haversineDistance(double lat1, double lon1, double lat2, double lon2);
double zoomForMeterWidth(Vec2I sizeViewport, Vec2F sizeMeters);
Expand All @@ -213,7 +218,6 @@ class MapCamera3d : public MapCameraInterface,
Coord focusPointPosition;
double cameraVerticalDisplacement = 0.0;
double cameraPitch = 0; // looking up or down
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fieldOfView member variable has been removed, but this doesn't appear to be related to the locking changes described in the PR. The method getCameraFieldOfView() returns a constant value (42), so this member was likely unused. However, if this deletion is intentional, it should be mentioned in the PR description. If unintentional, the line should be restored.

Copilot uses AI. Check for mistakes.
double fieldOfView = 10.0f;

double zoom;
double angle = 0;
Expand Down