-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[MapboxLayer] integrate mapbox-gl's near plane fix #3490
Conversation
useMapboxProjection | ||
? { | ||
// match mapbox's projection matrix | ||
nearZMultiplier: deck.height ? 1 / deck.height : 1, | ||
farZMultiplier: 1 | ||
// A change of near plane was made in 1.3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: These nested conditionals are starting to look a little messy, maybe time to write a helper function for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should some of this be codified in viewport-mercator-project? One of its main purposes has been to contain the knowledge of how to generate mapbox compatible matrices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mapbox has changed its projection matrix multiple times. We added nearZMultiplier
and farZMultiplier
to viewport-mercator-project so that we could accommodate all these use cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sounds like we should just add a mapboxVersion
parameter instead ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep it for now. deck.gl is currently using the settings from mapbox 0.29. We have internal use cases where apps need to tweak the near/far planes to achieve 180 degree pitch rotation. We might want to further decouple the defaults of WebMercatorViewport
from mapbox in v8.
For mapbox/mapbox-gl-js#7573
This needs to be patched before mapbox-gl's 1.3.0 release.
Change List