fix(CogSource): add zoom property and check in extentInsideLimit#2727
Open
ymoisan wants to merge 1 commit into
Open
fix(CogSource): add zoom property and check in extentInsideLimit#2727ymoisan wants to merge 1 commit into
ymoisan wants to merge 1 commit into
Conversation
CogSource was missing the `zoom` property that all other Source
subclasses set (TMSSource, WMTSSource, WMSSource, FileSource,
WFSSource). This caused a crash in TiledGeometryLayer.preUpdate
when CogSource was used as an ElevationLayer source, because the
tile system reads `source.zoom.max` to determine maximum subdivision
depth.
Additionally, `extentInsideLimit` was not checking the zoom parameter,
causing the source to claim data availability at all zoom levels
regardless of configuration.
Changes:
- Add `zoom` to CogSourceConfig type (default: { min: 0, max: Infinity })
- Add `zoom` class property and initialize from config in constructor
- Update `extentInsideLimit` to check zoom bounds, matching the
contract used by other Source subclasses
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jailln
approved these changes
Mar 26, 2026
Contributor
|
Thanks for the fix ! :) There is a ts error to fix though before we can merge: https://github.com/iTowns/itowns/actions/runs/23560566574/job/68677322715?pr=2727#step:7:26 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Again all AI generated (below this sentence), but this is a simple and quite focused fix that allows using CogSource for elevation.
CogSource was missing the
zoomproperty that all other Source subclasses set (TMSSource, WMTSSource, WMSSource, FileSource, WFSSource). This caused a crash in TiledGeometryLayer.preUpdate when CogSource was used as an ElevationLayer source, because the tile system readssource.zoom.maxto determine maximum subdivision depth.Additionally,
extentInsideLimitwas not checking the zoom parameter, causing the source to claim data availability at all zoom levels regardless of configuration.Changes:
zoomto CogSourceConfig type (default: { min: 0, max: Infinity })zoomclass property and initialize from config in constructorextentInsideLimitto check zoom bounds, matching the contract used by other Source subclassesBefore contributing
Read CONTRIBUTING.md and CODING.md to apply
iTownsconventions on PRs, Git history and code.Description
Motivation and Context
Screenshots (if appropriate)