Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Add a factor to limit tilt when offset value is important #16448

Closed
wants to merge 1 commit into from
Closed

Add a factor to limit tilt when offset value is important #16448

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/mbgl/map/transform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,8 @@ double Transform::getMaxPitchForEdgeInsets(const EdgeInsets& insets) const {
// We use half of fov, as it is field of view above perspective center.
// With inset, this angle changes and tangentOfFovAboveCenterAngle = (h/2 + centerOffsetY) / (height * 1.5).
// 1.03 is a bit extra added to prevent parallel ground to viewport clipping plane.
const double tangentOfFovAboveCenterAngle = 1.03 * (height / 2.0 + centerOffsetY) / (1.5 * height);
// 2.4 is an arbitrary factor to prevent too much depth when the top offset is important.
const double tangentOfFovAboveCenterAngle = 1.03 * (height / 2.0 + centerOffsetY * 2.4) / (1.5 * height);
Copy link
Contributor

Choose a reason for hiding this comment

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

This change effectively moves the camera’s focal point closer to where it was prior to #14664. Unfortunately, just modifying tangentOfFovAboveCenterAngle probably causes the pitch and contentInsets (edgePadding) properties to become inaccurate or inconsistent. This would technically be a consideration for #15195 as well, but in that case it’s only an edge case with very large padding, whereas here it would affect the camera no matter the amount of padding.

I agree that the pitch should vary by zoom level (#6908), but ideally it would be limited by the visible distance rather than a screen-based factor, and the pitch should be explicitly limited, affecting the camera model, not just the internal transform.

In the meantime, the common workaround for mapbox/mapbox-gl-native-android#162 is to reduce the pitch to something like 45°. That certainly isn’t optimal, but at least the map is internally consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback ! I understand that there is no easy solution, and this change would involve regressions for other use cases.
A separate, explicit parameterfor offset or for max visible distance sounds like a good idea, I hope it will be available at some time.
Actually the padding is not so "very large", it is easily reached if we want to have the map centered around 1/4 of the screen height.
The problem I have with reducing the pitch to 45° is that it doesn't take real padding size into account, sometimes it is not necessary to reduce the pitch so much. I think I will keep my "fix" in our fork while there is no alternative.

const double fovAboveCenter = std::atan(tangentOfFovAboveCenterAngle);
return M_PI * 0.5 - fovAboveCenter;
// e.g. Maximum pitch of 60 degrees is when perspective center's offset from the top is 84% of screen height.
Expand Down