-
-
Notifications
You must be signed in to change notification settings - Fork 714
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.
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
Allow pitch up to 85 #574
Allow pitch up to 85 #574
Conversation
Corrected version of #56. Please tell me if I missed something. |
Bundle size report: Size Change: 0 B
ℹ️ View Details
|
Thanks for this PR! |
P.S. Optimization can be done on another PR as the default wasn't changed as part of this PR and it's the user responsibility to make sure everything works as expected... |
I'm not sure I have a good solution for this, but it's worth noting somehow the the max pitch change is "at your own risk"...? |
I played around with overriding this in the past. Something worth calling out is that vector tiles that are in the distance may end up loading from an earlier zoom level. This particularly is noticeable when you setup zoom level ranges between two different layers. I wouldn't say this is a blocker, but something to be aware of. I also recall issues with the querySourceFeatures function not working correctly when pitched beyond 60 degrees in some cases, but can't seem to find the bug for this. |
Sounds good. Could you please tell me where to add this? |
I'm honestly not sure, the best idea I have is to write it as part of the change log item you added... |
add "at your own risk" note
@rbrundritt I know this is probably not a perfect solution to this issue and more work is needed but I think that adding this will allow users to start explore this, find issues and hopefully try and fix them. |
I think we should also add a warning to the documentation, as most users will not read old changelogs. Probably in https://maplibre.org/maplibre-gl-js-docs/api/map/#map#setmaxpitch. |
And the documentation for option.maxPitch in the source itself should also include a short warning. |
How about a warning like this. "Max pitch values greater than 60 degrees are experimental and may result in rendering issues. If you encounter any, please raise an issue with details in the MapLibre project.". |
add warning message about pitch greater than 60
I would also include this PR. Technically, I have no experience in working with the parameter. But since the default value remains and there is a warning, it should be fine. |
There's a couple of issues with high pitch values that I had found in the past; here's what I remember:
|
Wonderful ! |
Changed allowed maxPitch value from 60 to 85. Closes #47