-
Notifications
You must be signed in to change notification settings - Fork 22
Support "crs" for vector source in style/tile.json #910
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
base: main
Are you sure you want to change the base?
Conversation
…on of raster tile source description
ae41717 to
165f65e
Compare
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae41717fa4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| Tiled2dMapZoomLevelInfo prevZoom = zoomLevelInfos.back(); | ||
|
|
||
| for (auto it = zoomLevelInfos.rbegin() + maxZoomLevel; it != zoomLevelInfos.rend() + minZoomLevel; ++it) { |
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.
Correct reverse-iterator bounds in zoom mapping
In the irregular zoom mapping path (used by EPSG:2056/21781), the loop starts at rbegin() + maxZoomLevel and ends at rend() + minZoomLevel. For the common case where levels is [0..N], that range only visits the first element (or an otherwise incorrect subset), so getZoomIdentifier ends up interpolating between the last and first zoom levels instead of the adjacent levels around zoom. This produces incorrect zoom identifiers and therefore wrong tile selection for any intermediate zoom.
Useful? React with 👍 / 👎.
|
|
||
| Tiled2dMapZoomLevelInfo prevZoom = zoomLevelInfos.back(); | ||
|
|
||
| for (auto it = zoomLevelInfos.begin() + minZoomLevel; it != zoomLevelInfos.end() + maxZoomLevel; ++it) { |
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.
Fix out-of-bounds iteration in zoom factor lookup
The loop terminator uses zoomLevelInfos.end() + maxZoomLevel, which advances past the end of the vector and causes the loop to compare and dereference iterators that are out of range. For irregular CRS sources, any call to getZoomFactorAtIdentifier that requires interpolation can walk into invalid memory, leading to undefined behavior or crashes when rendering EPSG:2056/21781 layers.
Useful? React with 👍 / 👎.
Reorganize implementation of layer configs. Combine redundant implementation of default (webmercator), WebMercator again and Epsg4326 cases. For raster sources, enabling EPSG:4326 was already supported before. Now also support EPSG:2056 and EPSG:21871. Support specifying in field "crs" in addition to previously supported "metadata"."crs".
165f65e to
a1c4dc3
Compare
For the EPSG 3857 / WebMercator tile pyramid we now consistently use the base zoom value 559082264.029 instead of 500000000.0. Previously. both values where used in different places. Note: the webmercator tile pyramid is specified as urn:ogc:def:wkss:OGC:1.0:GoogleMapsCompatible in OGC WMTS Simple Profile, with the used base zoom value as ScaleDenominator of TileMatrix 0.
Reorganize implementation of layer configs. Combine redundant implementation of default (webmercator), WebMercator again and Epsg4326 cases.
For the EPSG 3857 / WebMercator tile pyramid we now consistently use the base zoom value 559082264.029 instead of 500000000. Previously, both values where used in different places.
(Note: the webmercator tile pyramid is specified as urn:ogc:def:wkss:OGC:1.0:GoogleMapsCompatible in OGC WMTS Simple Profile, with the used base zoom value as ScaleDenominator of TileMatrix 0.)
For raster sources, enabling EPSG:4326 was already supported before.
Now also support EPSG:2056 and EPSG:21871.
Support specifying in field "crs" in addition to previously supported "metadata"."crs".