Add a factor to limit tilt when offset value is important#16448
Add a factor to limit tilt when offset value is important#16448philemonmerlet wants to merge 1 commit intomapbox:masterfrom
Conversation
…ere is too much depth.
|
Other related issue : mapbox/mapbox-gl-native-android#282 |
| // 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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions. |
With an important tilt, when using a top padding to offset the map center, like in GPS apps, currently the view depth becomes very far away, and the apps become very laggy (unusable on Android devices).
PR #15195 limits the depth, but this is far from enough.
As described in mapbox/mapbox-gl-native-android#162 , from the demo sample :
As a result, the view depth is too far away :
By adding a factor of 2.4 (value to be discussed) to the top offset when calculating the maximum tilt for a specified inset, the depth is less far away and the behavior is much more reasonable, allowing to use the app without lag while maintaining a view depth similar as when there is no padding, and also similar as what we had before the padding behavior was changed (post Android 7.4 release).

Note that on this particular screen, the resulting max tilt is approximately 45 degrees, which could be set manually using the MapboxMapOptions. However, this means the value would be set even when padding values do not need it (what about landscape mode ?). Also I think the current behavior looks like a bug, and users should not have to dig into mapbox issues to find this workaround.
Regards !