-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Port Esri wayback imagery from Rapid #11780
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: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,11 @@ import { prefs } from '../core/preferences'; | |||||||||||||||||||||||||||||||||||||||||
| import { fileFetcher } from '../core/file_fetcher'; | ||||||||||||||||||||||||||||||||||||||||||
| import { geoMetersToOffset, geoOffsetToMeters, geoExtent } from '../geo'; | ||||||||||||||||||||||||||||||||||||||||||
| import { rendererBackgroundSource } from './background_source'; | ||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||
| parseWaybackId, | ||||||||||||||||||||||||||||||||||||||||||
| createWaybackSource, | ||||||||||||||||||||||||||||||||||||||||||
| ESRI_WAYBACK_ID | ||||||||||||||||||||||||||||||||||||||||||
| } from './background_source_wayback.js'; | ||||||||||||||||||||||||||||||||||||||||||
| import { rendererTileLayer } from './tile_layer'; | ||||||||||||||||||||||||||||||||||||||||||
| import { utilQsString, utilStringQs } from '../util'; | ||||||||||||||||||||||||||||||||||||||||||
| import { utilRebind } from '../util/rebind'; | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -74,6 +79,14 @@ export function rendererBackground(context) { | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Only add EsriWayback if 'EsriWorldImagery' exists, inserting it right after it | ||||||||||||||||||||||||||||||||||||||||||
| const esriWorldImageryIndex = _imageryIndex.backgrounds.findIndex(s => s.id === 'EsriWorldImagery'); | ||||||||||||||||||||||||||||||||||||||||||
| if (esriWorldImageryIndex >= 0) { | ||||||||||||||||||||||||||||||||||||||||||
| const esriWorldImagerySource = _imageryIndex.backgrounds[esriWorldImageryIndex]; | ||||||||||||||||||||||||||||||||||||||||||
| const waybackSource = createWaybackSource(esriWorldImagerySource, context, dispatch); | ||||||||||||||||||||||||||||||||||||||||||
| _imageryIndex.backgrounds.splice(esriWorldImageryIndex + 1, 0, waybackSource); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Add 'None' | ||||||||||||||||||||||||||||||||||||||||||
| _imageryIndex.backgrounds.unshift(rendererBackgroundSource.None()); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -202,9 +215,13 @@ export function rendererBackground(context) { | |||||||||||||||||||||||||||||||||||||||||
| const y = +meters[1].toFixed(2); | ||||||||||||||||||||||||||||||||||||||||||
| let hash = utilStringQs(window.location.hash); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** @type {string|null} */ | ||||||||||||||||||||||||||||||||||||||||||
| let id = currSource.id; | ||||||||||||||||||||||||||||||||||||||||||
| if (id === 'custom') { | ||||||||||||||||||||||||||||||||||||||||||
| id = `custom:${currSource.template()}`; | ||||||||||||||||||||||||||||||||||||||||||
| } else if (id === ESRI_WAYBACK_ID) { | ||||||||||||||||||||||||||||||||||||||||||
| // Wayback sources include the date in their key (e.g., 'EsriWayback_2024-01-01') | ||||||||||||||||||||||||||||||||||||||||||
| id = currSource.key(); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if (id) { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -477,21 +494,37 @@ export function rendererBackground(context) { | |||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Helper function to get fallback background source | ||||||||||||||||||||||||||||||||||||||||||
| function getFallbackSource(preferredSource) { | ||||||||||||||||||||||||||||||||||||||||||
| return preferredSource || | ||||||||||||||||||||||||||||||||||||||||||
| best || | ||||||||||||||||||||||||||||||||||||||||||
| (isLastUsedValid && background.findSource(lastUsedBackground)) || | ||||||||||||||||||||||||||||||||||||||||||
| background.findSource('Bing') || | ||||||||||||||||||||||||||||||||||||||||||
| first || | ||||||||||||||||||||||||||||||||||||||||||
| background.findSource('none'); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Decide which background layer to display | ||||||||||||||||||||||||||||||||||||||||||
| if (requestedBackground && requestedBackground.indexOf('custom:') === 0) { | ||||||||||||||||||||||||||||||||||||||||||
| const template = requestedBackground.replace(/^custom:/, ''); | ||||||||||||||||||||||||||||||||||||||||||
| const custom = background.findSource('custom'); | ||||||||||||||||||||||||||||||||||||||||||
| background.baseLayerSource(custom.template(template)); | ||||||||||||||||||||||||||||||||||||||||||
| prefs('background-custom-template', template); | ||||||||||||||||||||||||||||||||||||||||||
| } else if (requestedBackground) { | ||||||||||||||||||||||||||||||||||||||||||
| const waybackInfo = parseWaybackId(requestedBackground); | ||||||||||||||||||||||||||||||||||||||||||
| if (waybackInfo.isWayback) { | ||||||||||||||||||||||||||||||||||||||||||
| const waybackSource = background.findSource(ESRI_WAYBACK_ID); | ||||||||||||||||||||||||||||||||||||||||||
| if (waybackSource) { | ||||||||||||||||||||||||||||||||||||||||||
| waybackSource.date(waybackInfo.date); | ||||||||||||||||||||||||||||||||||||||||||
| background.baseLayerSource(waybackSource); | ||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
| background.baseLayerSource(getFallbackSource(background.findSource(requestedBackground))); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
| background.baseLayerSource(getFallbackSource(background.findSource(requestedBackground))); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+514
to
+525
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading again, this can likely be…
Suggested change
FYI:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me to simplify those two branches. |
||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
| background.baseLayerSource( | ||||||||||||||||||||||||||||||||||||||||||
| background.findSource(requestedBackground) || | ||||||||||||||||||||||||||||||||||||||||||
| best || | ||||||||||||||||||||||||||||||||||||||||||
| isLastUsedValid && background.findSource(lastUsedBackground) || | ||||||||||||||||||||||||||||||||||||||||||
| background.findSource('Bing') || | ||||||||||||||||||||||||||||||||||||||||||
| first || | ||||||||||||||||||||||||||||||||||||||||||
| background.findSource('none') | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| background.baseLayerSource(getFallbackSource()); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const locator = imageryIndex.backgrounds.find(d => d.overlay && d.default); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
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.
The "only add if" is not the goal here but given we should always be present, I did not think we need an else-branch here.
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 guess one could argue that if one uses a customized instance of iD with it own set of custom imagery sources (that does not include ESRI World Imagery), it does indeed make sense to not insert the "ESRI Wayback" layer.
Maybe it would be slightly more elegant to define "ESRI Wayback" in
data/manual_imagery.jsonsomehow (which can be more easily customized by potential forks of iD) instead of hardcoding it here and relying on the presence of another (technically independent) layer. Instead, here we could just make sure that it is sorted properly next to the other ESRI layer. Feel free to leave it as is, though, if you think it is not worth the effort.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.
PS: I think it would make more sense to include the Wayback entry after the regular ESRI World Imagery layer: It is more common to use the latest imagery while mapping compared to older versions of the imagery.