fix(plugins): keep globe projection when adding a Zarr layer via addZarrLayer - #1470
Conversation
…arrLayer addZarrLayerExclusively called ensureMercatorProjection, snapping the map out of globe whenever a plugin added a Zarr layer through addZarrLayer — while the Add Data → Zarr panel (same renderer) kept globe. @carbonplan/zarr-layer reprojects on the GPU and renders projected data correctly in globe, so the switch was unnecessary and made the two Zarr entry points inconsistent. Remove it; the COG / cloud-NetCDF / LiDAR deck.gl paths keep their own ensureMercatorProjection calls. Fixes opengeos#1466
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughZarr layer mounting no longer forces Mercator projection. The updated comment documents globe compatibility and aligns the plugin path with the Zarr panel behavior. ChangesZarr projection behavior
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Removes the forced Web Mercator projection switch when adding Zarr layers via the plugin addZarrLayer API, keeping the map’s current projection (including globe) consistent with the built-in Add Data → Zarr panel behavior.
Changes:
- Removed the
ensureMercatorProjection(app.getMap?.())call from theaddZarrLayerExclusivelypath. - Updated the inline rationale comment to reflect that Zarr rendering works correctly in globe and to document the intended consistency (ref #1466).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
addZarrLayerExclusively(the code path behind theaddZarrLayerplugin API added in #1447) calledensureMercatorProjection(app.getMap?.()), which snapped the map out of globe and pinned it in Web Mercator whenever a plugin added a Zarr layer. This PR removes that call.Why
The Add Data → Zarr panel renders through the same
ZarrLayerControland never switches projection — so a projected Zarr renders fine in globe there.@carbonplan/zarr-layerreprojects on the GPU (via proj4js), and it renders projected data correctly in globe. So forcing Mercator on theaddZarrLayerpath was both unnecessary and inconsistent with the panel: the same store rendered in globe via Add Data but snapped to Mercator via a plugin.The old comment ("the untiled Zarr renderer draws in Web Mercator") is disproven by the panel path and by external use of the same stack (
@carbonplan/zarr-layer+ MapLibre 6 in globe, renderingEPSG:32633data via proj4).Scope
addZarrLayerpath changes. The COG / cloud-NetCDF / LiDAR deck.gl paths keep their ownensureMercatorProjectioncalls (deck.gl genuinely needs Mercator), so the import stays in use.Validation
Tested locally against a projected Zarr (seNorge,
EPSG:32633, via proj4) added through a plugin while the map was in globe: it now renders in globe and stays there, matching the Add Data → Zarr panel. No regression on the other raster paths.Fixes #1466 (as discussed there — thanks for the go-ahead 🙌).
Summary by CodeRabbit