diff --git a/src/data/localGeojsonCore.js b/src/data/localGeojsonCore.js index ddc52a9a3..7fb70cf67 100644 --- a/src/data/localGeojsonCore.js +++ b/src/data/localGeojsonCore.js @@ -365,6 +365,12 @@ export function createLocalGeoJsonLayer( * @type {Array|null} */ let _cachedFeatures = null; + /** + * The source a `replaceFeatures()` rebuild is replacing: it stays drawn + * until the new one is in the scene, and comes back if the rebuild fails. + * @type {{source: object, stemRecords: object[], count: number, lastUpdate: number|null, error: string|null}|null} + */ + let _outgoing = null; /** * Globe-LOD active set: the record ids allowed to carry a live stem right * now. This bounds geometry refreshes and ground-sample work to the @@ -449,7 +455,19 @@ export function createLocalGeoJsonLayer( * survive a disable is the per-frame visualizer walk and the entity memory. * @param {object} viewer */ + const releaseOutgoing = (viewer) => { + if (!_outgoing) return; + const { source } = _outgoing; + _outgoing = null; + try { + viewer?.dataSources?.remove(source, true); + } catch { + /* already gone */ + } + }; + const releaseDataSource = (viewer) => { + releaseOutgoing(viewer); if (!_dataSource) return; const source = _dataSource; _dataSource = null; @@ -489,7 +507,7 @@ export function createLocalGeoJsonLayer( } }; - return { + const layer = { id, name, icon, @@ -726,7 +744,7 @@ export function createLocalGeoJsonLayer( // Store references for bounded stem scaling and native picking. feature.__localBaseCarto = carto; feature.__localBaseCartesian = base; - registerEntityContext(feature, { + const context = registerEntityContext(feature, { id: `${id}:${recordId}`, layerId: id, layerName: name, @@ -769,6 +787,7 @@ export function createLocalGeoJsonLayer( _stemRecords.push({ id: recordId, entity: feature, + context, carto, base, tip, @@ -1111,6 +1130,99 @@ export function createLocalGeoJsonLayer( _cachedFeatures = null; }, + /** + * Draw a new feature set in place of the current one while the layer is + * on (a live loader's refresh). The current features stay drawn until + * the new ones are in the scene; a selected feature stays selected when + * the new set still has it (same id, or same place and label); a failed + * rebuild keeps the current features and rejects. Off, the set is only + * kept for the next enable(). + * @param {object} viewer + * @param {object[]} features GeoJSON features. + */ + replaceFeatures: async (viewer, features) => { + if (_destroyed) return; + const previousFeatures = _cachedFeatures; + _cachedFeatures = features; + if (!_enabled || !_dataSource || _loadPromise) return; + const selected = + viewer?.selectedEntity?.__localLayerId === id + ? viewer.selectedEntity + : null; + const selectedLabel = selected + ? _stemRecords.find((record) => record.entity === selected)?.context + ?.label + : undefined; + _outgoing = { + source: _dataSource, + stemRecords: _stemRecords, + count: _count, + lastUpdate: _lastUpdate, + error: _error, + }; + const outgoing = _outgoing; + _dataSource = null; + await coreEnable(viewer); + // Disabled or destroyed meanwhile: release took both sources. + if (_destroyed || !_enabled || _outgoing !== outgoing) return; + _outgoing = null; + if (!_dataSource) { + // The rebuild failed: the current features come back as they were. + const failure = _error; + _cachedFeatures = previousFeatures; + _dataSource = outgoing.source; + _stemRecords = outgoing.stemRecords; + _count = outgoing.count; + _lastUpdate = outgoing.lastUpdate; + _error = outgoing.error; + _stemGeometryDirty = true; + _lastVisibilityUpdate = Number.NEGATIVE_INFINITY; + for (const record of _stemRecords) + if (record.context) + registerEntityContext(record.entity, record.context); + if (selected && viewer.selectedEntity === selected) + selectEntityContext(selected); + throw new Error(failure || 'dataset unavailable'); + } + try { + viewer?.dataSources?.remove(outgoing.source, true); + } catch { + /* already gone */ + } + // Records the new set no longer has go with the old source. + removeEntityContextsForLayer(id, { + retainIds: new Set( + _stemRecords.map((record) => record.context?.id).filter(Boolean), + ), + }); + if (selected) { + const same = (record) => { + const old = selected.__localBaseCarto; + const next = record.entity.__localBaseCarto; + return ( + old && + next && + Math.abs(old.longitude - next.longitude) < 1e-9 && + Math.abs(old.latitude - next.latitude) < 1e-9 && + record.context?.label === selectedLabel + ); + }; + const match = _stemRecords.find((record) => + selected.id !== undefined && record.entity.id === selected.id + ? true + : same(record), + ); + if (match) { + viewer.selectedEntity = match.entity; + selectEntityContext(match.entity); + } else if (viewer.selectedEntity === selected) { + viewer.selectedEntity = undefined; + clearSelectedEntityContextForLayer(id); + } + } + viewer?.scene?.requestRender?.(); + }, + destroy: (viewer) => { if (_destroyed) return; _destroyed = true; @@ -1135,6 +1247,8 @@ export function createLocalGeoJsonLayer( _error = null; }, }; + const coreEnable = layer.enable; + return layer; } function compareLocalOverlayRecords(a, b) { diff --git a/src/library/datasetLayer.test.mjs b/src/library/datasetLayer.test.mjs index 8d143c2d9..a8f2f1922 100644 --- a/src/library/datasetLayer.test.mjs +++ b/src/library/datasetLayer.test.mjs @@ -15,21 +15,27 @@ const entry = { licence: { name: 'CC0', url: 'https://example.org/licence' }, }; -const point = (name) => ({ - type: 'Feature', - properties: { name }, - geometry: { - type: 'Polygon', - coordinates: [ - [ - [0, 0], - [0.01, 0], - [0, 0.01], - [0, 0], +// One small triangle per name, each in its own place. +const point = (name) => { + const x = name.charCodeAt(0) / 100; + return { + type: 'Feature', + properties: { name }, + geometry: { + type: 'Polygon', + coordinates: [ + [ + [x, 0], + [x + 0.01, 0], + [x, 0.01], + [x, 0], + ], ], - ], - }, -}); + }, + }; +}; +// A clock that is never 0, which reads as "never loaded". +const START = 1_000_000; const deferred = () => { let resolve; @@ -91,7 +97,7 @@ const result = (...names) => ({ }); test('a library dataset refreshes on the manager tick once older than cacheSeconds', async (t) => { - t.mock.timers.enable({ apis: ['Date'], now: 0 }); + t.mock.timers.enable({ apis: ['Date'], now: START }); const env = harness(t, [result('a'), result('b', 'c')]); assert.equal(env.layer.refreshInterval, 60_000); await env.layer.enable(env.viewer); @@ -105,8 +111,84 @@ test('a library dataset refreshes on the manager tick once older than cacheSecon assert.equal(env.sources.size, 1, 'the old source is replaced, not kept'); }); +test('a slow upstream still refreshes on every tick, not every other one', async (t) => { + t.mock.timers.enable({ apis: ['Date'], now: START }); + // Each request takes two seconds to answer. + const slow = (value) => async () => { + t.mock.timers.tick(2000); + return value; + }; + const env = harness(t, [ + slow(result('a')), + slow(result('b')), + slow(result('c')), + ]); + await env.layer.enable(env.viewer); + // The manager ticks every refreshInterval from when the layer came on. + t.mock.timers.setTime(START + 60_000); + await env.layer.update(env.viewer); + t.mock.timers.setTime(START + 120_000); + await env.layer.update(env.viewer); + assert.equal(env.calls(), 3); +}); + +test('a failed first load is not retried by the tick that follows enable', async (t) => { + t.mock.timers.enable({ apis: ['Date'], now: START }); + t.mock.method(console, 'error', () => {}); + const env = harness(t, [ + () => Promise.reject(new Error('HTTP 503')), + result('a'), + ]); + await env.layer.enable(env.viewer); + assert.ok(env.layer.getStats().error); + // Resolving (not rejecting) keeps the manager from failing the switch-on. + await env.layer.update(env.viewer); + assert.equal(env.calls(), 1); +}); + +test('a refresh keeps the old features drawn until the new ones are in, and the selection', async (t) => { + t.mock.timers.enable({ apis: ['Date'], now: START }); + const env = harness(t, [result('a', 'b'), result('a', 'c')]); + await env.layer.enable(env.viewer); + const [first] = env.sources; + const selected = first.entities.values.find( + (entity) => entity.properties.name.getValue() === 'a', + ); + env.viewer.selectedEntity = selected; + const sizes = []; + const add = env.viewer.dataSources.add; + env.viewer.dataSources.add = async (source) => { + sizes.push(env.sources.size); + return add(source); + }; + t.mock.timers.tick(60_000); + await env.layer.update(env.viewer); + assert.deepEqual(sizes, [1], 'the old source was still drawn'); + assert.equal(env.sources.size, 1); + const [next] = env.sources; + assert.notEqual(next, first); + assert.ok(next.entities.values.includes(env.viewer.selectedEntity)); + assert.equal(env.viewer.selectedEntity.properties.name.getValue(), 'a'); +}); + +test('a refresh whose rebuild fails keeps the drawn features', async (t) => { + t.mock.timers.enable({ apis: ['Date'], now: START }); + t.mock.method(console, 'error', () => {}); + const env = harness(t, [result('a'), result('b', 'c')]); + await env.layer.enable(env.viewer); + const [first] = env.sources; + env.viewer.dataSources.add = async () => { + throw new Error('scene full'); + }; + t.mock.timers.tick(60_000); + await assert.rejects(env.layer.update(env.viewer)); + assert.deepEqual([...env.sources], [first]); + assert.equal(env.layer.getStats().count, 1); + assert.equal(env.layer.getStats().error, null); +}); + test('a stale library dataset reloads on the next enable', async (t) => { - t.mock.timers.enable({ apis: ['Date'], now: 0 }); + t.mock.timers.enable({ apis: ['Date'], now: START }); const env = harness(t, [result('a'), result('b', 'c')]); await env.layer.enable(env.viewer); env.layer.disable(env.viewer); @@ -120,7 +202,7 @@ test('a stale library dataset reloads on the next enable', async (t) => { }); test('a failed refresh keeps the drawn features', async (t) => { - t.mock.timers.enable({ apis: ['Date'], now: 0 }); + t.mock.timers.enable({ apis: ['Date'], now: START }); const env = harness(t, [ result('a'), () => Promise.reject(new Error('HTTP 503')), @@ -133,7 +215,7 @@ test('a failed refresh keeps the drawn features', async (t) => { }); test('disable during a refresh cancels it and draws nothing late', async (t) => { - t.mock.timers.enable({ apis: ['Date'], now: 0 }); + t.mock.timers.enable({ apis: ['Date'], now: START }); const pending = deferred(); let refreshSignal = null; const env = harness(t, [ diff --git a/src/library/layers.js b/src/library/layers.js index ffbb257f2..64ee55247 100644 --- a/src/library/layers.js +++ b/src/library/layers.js @@ -161,13 +161,25 @@ export function createLibraryDatasetLayer( let rowListener = null; let legendCounts = null; // Live data: features older than the source's cacheSeconds are loaded - // again, on the next enable and on a timer while the layer is on. + // again, on the next enable and on the manager's refresh tick while the + // layer is on. Their age counts from when they were requested, and a tick + // allows a little slack, so a tick every cacheSeconds refreshes on every + // tick rather than every other one. const maxAgeMs = entry.source.cacheSeconds * 1000; + const tickSlackMs = Math.min(5000, maxAgeMs / 10); + /** When the drawn data was requested; 0 until a load succeeds. */ let loadedAt = 0; - let prefetched = null; let refreshController = null; let on = false; - const stale = () => Date.now() - loadedAt >= maxAgeMs; + const age = () => Date.now() - loadedAt; + function take(result, requestedAt) { + loadedAt = requestedAt; + truncated = result.truncated; + loaded = result.features.length; + extent = featureExtent(result.features); + if (colorBy) legendCounts = countColorKeys(result.features); + rowListener?.(); + } const colorBy = entry.display.colorBy || null; const layer = createLocalGeoJsonLayer( { @@ -186,15 +198,9 @@ export function createLibraryDatasetLayer( colorBy.colors[String(properties?.__gevColorKey ?? '')] || null : null, loadFeatures: async ({ signal }) => { - const result = - prefetched ?? (await source.loadDataset(entry, { signal })); - prefetched = null; - loadedAt = Date.now(); - truncated = result.truncated; - loaded = result.features.length; - extent = featureExtent(result.features); - if (colorBy) legendCounts = countColorKeys(result.features); - rowListener?.(); + const requestedAt = Date.now(); + const result = await source.loadDataset(entry, { signal }); + take(result, requestedAt); return result.features; }, }, @@ -207,7 +213,6 @@ export function createLibraryDatasetLayer( const cancelRefresh = () => { refreshController?.abort(); refreshController = null; - prefetched = null; }; return Object.assign(layer, { libraryEntryId: entry.id, @@ -215,27 +220,30 @@ export function createLibraryDatasetLayer( async enable(viewer, ...rest) { on = true; registerDynamicCredit(viewer, libraryCredit(entry)); - if (stale()) layer.dropCachedFeatures(); + if (loadedAt && age() >= maxAgeMs) layer.dropCachedFeatures(); return enable(viewer, ...rest); }, /** - * The manager's refresh tick: once the data is stale, load it again and - * swap it in only when the new set is ready, so the old features stay - * drawn while the request runs and stay if it fails. + * The manager's refresh tick: once the data is due, load it again and + * swap it in where it is drawn, so the old features (and a selected + * one) stay while the request runs and stay if it fails. Nothing to + * refresh before a first load has succeeded: the tick that follows + * enable() must not retry a failed load and fail the switch-on. */ async update(viewer) { - if (!on || !stale() || refreshController) return; + if (!on || !loadedAt || refreshController) return; + if (age() < maxAgeMs - tickSlackMs) return; const current = new AbortController(); refreshController = current; + const requestedAt = Date.now(); try { const result = await source.loadDataset(entry, { signal: current.signal, }); if (!on || current.signal.aborted) return; - prefetched = result; - layer.dropCachedFeatures(); - disable(viewer); - await enable(viewer); + await layer.replaceFeatures(viewer, result.features); + if (!on || current.signal.aborted) return; + take(result, requestedAt); } finally { if (refreshController === current) refreshController = null; }