mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-05 19:04:02 +00:00
fix(globe): harden the Navara engine after review
- maps: photoreal hides the base imagery layer instead of deleting it, and a new base layer goes in before the old one goes: deleting the last raster layer stops Navara 0.2.1 loading terrain at the current view, which left terrainHeight undefined under the tiles - Google 3D tiles open Navara's attribution card so the tiles' copyright lines are on screen, as Cesium shows them; onScreen credits do the same - setCameraInputs merges into the camera options set through the engine (new setCameraOptions extension) instead of resetting them - createNavaraGlobe disposes the view and canvas if the engine throws - the pin test checks the installed Navara version and catches bracketed private access - QA: draw-match checks at the nadir clamp, after a Navara flight and after a mouse drag; terrain under photoreal set right away; the copyright lines on screen; create/destroy/create leaves no canvas, frames, animation frames, workers or console errors (54/54)
This commit is contained in:
1 parent
90c8e7763a
commit
a4ac0b9b0d
8 files changed
+148
-25
No files matched your search
@@ -1,9 +1,10 @@
|
||||
// Static credits on Navara's attribution UI (view.attribution, the
|
||||
// AttributionPlugin). Navara matches entries by content and drops duplicates,
|
||||
// so one HTML string is shown once and counted here, staying until its last
|
||||
// holder removes it. Navara has no separate on-screen strip: `onScreen` credits
|
||||
// go into the same list; logos that must stay visible (Google) are sources
|
||||
// with a `logo`, added by maps.js.
|
||||
// holder removes it. Navara has no separate on-screen strip: an `onScreen`
|
||||
// credit opens the attribution card (collapsed by default) so the list is on
|
||||
// screen; logos that must stay visible (Google) are sources with a `logo`,
|
||||
// added by maps.js.
|
||||
|
||||
/**
|
||||
* @param {{ attribution?: { add: Function, remove: Function } }} view
|
||||
@@ -19,11 +20,12 @@ export function createNavaraCredits(view) {
|
||||
view.attribution?.remove([{ attributionHtml: html }]);
|
||||
};
|
||||
const credits = {
|
||||
addStatic(html) {
|
||||
addStatic(html, { onScreen = false } = {}) {
|
||||
const text = String(html);
|
||||
const count = counts.get(text) ?? 0;
|
||||
counts.set(text, count + 1);
|
||||
if (count === 0) view.attribution?.add([{ attributionHtml: text }]);
|
||||
if (onScreen) view.attribution?.show();
|
||||
let removed = false;
|
||||
return () => {
|
||||
if (removed) return;
|
||||
|
||||
@@ -4,8 +4,12 @@ import { createNavaraCredits } from './credits.js';
|
||||
|
||||
function fakeAttribution() {
|
||||
const shown = [];
|
||||
return {
|
||||
const attribution = {
|
||||
shown,
|
||||
opened: 0,
|
||||
show() {
|
||||
attribution.opened++;
|
||||
},
|
||||
add(items) {
|
||||
for (const item of items)
|
||||
if (!shown.some((entry) => entry.attributionHtml === item.attributionHtml))
|
||||
@@ -20,6 +24,7 @@ function fakeAttribution() {
|
||||
}
|
||||
},
|
||||
};
|
||||
return attribution;
|
||||
}
|
||||
|
||||
test('addStatic shows HTML in the attribution and the remover is idempotent', () => {
|
||||
@@ -55,3 +60,12 @@ test('destroy removes every credit; a view without attribution UI is a no-op', (
|
||||
const remove = bare.credits.addStatic('A');
|
||||
remove();
|
||||
});
|
||||
|
||||
test('onScreen credits open the attribution list on screen; others do not', () => {
|
||||
const attribution = fakeAttribution();
|
||||
const { credits } = createNavaraCredits({ attribution });
|
||||
credits.addStatic('Quiet');
|
||||
assert.equal(attribution.opened, 0);
|
||||
credits.addStatic('Loud', { onScreen: true });
|
||||
assert.equal(attribution.opened, 1);
|
||||
});
|
||||
@@ -42,6 +42,7 @@ export async function createNavaraGlobe({ container, effects, pixelRatio }) {
|
||||
const plan = navaraScenePlan(effects);
|
||||
const canvas = preservedCanvas(container);
|
||||
let view;
|
||||
let engine;
|
||||
try {
|
||||
view = new ThreeView(navaraViewOptions({ container, canvas, pixelRatio }));
|
||||
const plugin = new DefaultPlugin();
|
||||
@@ -53,15 +54,15 @@ export async function createNavaraGlobe({ container, effects, pixelRatio }) {
|
||||
for (const name of plan.lights) view.addLight({ [name]: {} });
|
||||
for (const name of plan.effects) view.addEffect({ [name]: {} });
|
||||
}
|
||||
engine = createNavaraEngine(view, {
|
||||
hooks: navaraHooks(view),
|
||||
container,
|
||||
});
|
||||
} catch (error) {
|
||||
view?.dispose();
|
||||
canvas.remove();
|
||||
throw error;
|
||||
}
|
||||
const engine = createNavaraEngine(view, {
|
||||
hooks: navaraHooks(view),
|
||||
container,
|
||||
});
|
||||
let destroyed = false;
|
||||
return {
|
||||
view,
|
||||
|
||||
@@ -32,6 +32,7 @@ export function createNavaraInput(view, { onCameraInput = () => {} } = {}) {
|
||||
let attached = false;
|
||||
let destroyed = false;
|
||||
let cameraInputs = true;
|
||||
let cameraOptions = {};
|
||||
let downAt = null;
|
||||
let last = null;
|
||||
|
||||
@@ -108,11 +109,20 @@ export function createNavaraInput(view, { onCameraInput = () => {} } = {}) {
|
||||
},
|
||||
setCameraInputs(enabled) {
|
||||
cameraInputs = Boolean(enabled);
|
||||
view.camera.options = {
|
||||
input.setCameraOptions({
|
||||
enableSpin: cameraInputs,
|
||||
enableZoom: cameraInputs,
|
||||
enableTilt: cameraInputs,
|
||||
};
|
||||
});
|
||||
},
|
||||
/**
|
||||
* Navara-only extension: camera controller options (zoom distances,
|
||||
* speeds, ...). Navara has no getter, so the engine keeps every option
|
||||
* set through it and always sends the merged set.
|
||||
*/
|
||||
setCameraOptions(options) {
|
||||
cameraOptions = { ...cameraOptions, ...options };
|
||||
view.camera.options = { ...cameraOptions };
|
||||
},
|
||||
};
|
||||
return {
|
||||
|
||||
@@ -141,3 +141,24 @@ test('destroy removes every DOM listener and later subscriptions are inert', ()
|
||||
assert.equal(typeof off, 'function');
|
||||
assert.equal(canvas.listenerCount, 0);
|
||||
});
|
||||
|
||||
test('setCameraInputs merges into the camera options set through the engine', () => {
|
||||
const { input, cameraOptions } = setup();
|
||||
input.setCameraOptions({ minimumZoomDistance: 50, zoomSpeed: 2 });
|
||||
input.setCameraInputs(false);
|
||||
assert.deepEqual(cameraOptions.at(-1), {
|
||||
minimumZoomDistance: 50,
|
||||
zoomSpeed: 2,
|
||||
enableSpin: false,
|
||||
enableZoom: false,
|
||||
enableTilt: false,
|
||||
});
|
||||
input.setCameraOptions({ zoomSpeed: 3 });
|
||||
assert.deepEqual(cameraOptions.at(-1), {
|
||||
minimumZoomDistance: 50,
|
||||
zoomSpeed: 3,
|
||||
enableSpin: false,
|
||||
enableZoom: false,
|
||||
enableTilt: false,
|
||||
});
|
||||
});
|
||||
@@ -11,8 +11,23 @@ test('Navara internals are pinned: package.json and internals.js name the same e
|
||||
/NAVARA_INTERNALS_VERSION = '([^']+)'/,
|
||||
)?.[1];
|
||||
assert.equal(declared, '0.2.1');
|
||||
for (const name of ['@navaramap/three', '@navaramap/three-default-plugin'])
|
||||
for (const name of ['@navaramap/three', '@navaramap/three-default-plugin']) {
|
||||
assert.equal(pkg.dependencies[name], declared, name);
|
||||
const installed = JSON.parse(
|
||||
read(`../../../node_modules/${name}/package.json`),
|
||||
);
|
||||
assert.equal(installed.version, declared, `installed ${name}`);
|
||||
}
|
||||
});
|
||||
|
||||
// `x._member`, `x?._member` and `x['_member']`.
|
||||
const PRIVATE_ACCESS = /\.\s*_[A-Za-z]|\[\s*['"`]_[A-Za-z]/;
|
||||
|
||||
test('the private-member pattern catches dotted and bracketed access', () => {
|
||||
for (const code of ['view._isIdle', 'view?._scenes', "view['_isIdle']", 'v[ "_x" ]'])
|
||||
assert.ok(PRIVATE_ACCESS.test(code), code);
|
||||
for (const code of ['view.isIdle', 'a_b.c', "map['key']"])
|
||||
assert.ok(!PRIVATE_ACCESS.test(code), code);
|
||||
});
|
||||
|
||||
test('only internals.js reaches private Navara members', () => {
|
||||
@@ -20,7 +35,7 @@ test('only internals.js reaches private Navara members', () => {
|
||||
(name) =>
|
||||
name.endsWith('.js') &&
|
||||
name !== 'internals.js' &&
|
||||
/\.\s*_[A-Za-z]/.test(read(name).replace(/\/\/.*$/gm, '')),
|
||||
PRIVATE_ACCESS.test(read(name).replace(/\/\/.*$/gm, '')),
|
||||
);
|
||||
assert.deepEqual(offenders, []);
|
||||
});
|
||||
+39
-11
@@ -11,9 +11,11 @@
|
||||
// token), or none (the ellipsoid). Navara 0.2.1 panics on a quantized-mesh
|
||||
// URL without the {z}/{x}/{y}.terrain template (S2), so URLs always carry it.
|
||||
// - photoreal: Google 3D tiles with a browser key only in this phase (the
|
||||
// server proxy of S3 comes later), with Google's logo kept on screen and the
|
||||
// tiles' own copyright lines (creditLayerId). Showing them removes the base
|
||||
// imagery, as MapSourceController does; clearing them does not restore it.
|
||||
// server proxy of S3 comes later), with Google's logo and the tiles' own
|
||||
// copyright lines (creditLayerId) on screen. Showing them hides the base
|
||||
// imagery, as MapSourceController removes it; clearing them does not bring
|
||||
// it back until setBaseImagery. Hidden, not deleted: deleting the last
|
||||
// raster layer stops Navara 0.2.1 loading terrain at the current view.
|
||||
|
||||
const OSM_URL = 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
|
||||
const OSM_CREDIT = {
|
||||
@@ -68,24 +70,47 @@ export function createNavaraMaps(
|
||||
const generations = { imagery: 0, terrain: 0, photoreal: 0 };
|
||||
let destroyed = false;
|
||||
|
||||
function clear(name) {
|
||||
const slot = slots[name];
|
||||
function retire(slot) {
|
||||
if (!slot) return;
|
||||
slots[name] = null;
|
||||
slot.layer.delete();
|
||||
slot.source.delete();
|
||||
if (slot.credits.length) view.attribution?.remove(slot.credits);
|
||||
if (slot.credits.length && !slot.hidden)
|
||||
view.attribution?.remove(slot.credits);
|
||||
}
|
||||
|
||||
function clear(name) {
|
||||
const slot = slots[name];
|
||||
slots[name] = null;
|
||||
retire(slot);
|
||||
}
|
||||
|
||||
// The new layer goes in before the old one goes: deleting the last raster
|
||||
// layer stops Navara 0.2.1 loading terrain at the current view.
|
||||
function show(name, key, sourceDesc, layerDesc, credits = []) {
|
||||
clear(name);
|
||||
const previous = slots[name];
|
||||
const source = view.addSource(sourceDesc);
|
||||
const layer = view.addLayer({ ...layerDesc, source });
|
||||
const shown = credits.map((credit) =>
|
||||
typeof credit === 'function' ? credit(layer) : credit,
|
||||
);
|
||||
retire(previous);
|
||||
if (shown.length) view.attribution?.add(shown);
|
||||
slots[name] = { key, source, layer, credits: shown };
|
||||
slots[name] = { key, source, layer, layerDesc, credits: shown };
|
||||
view.forceUpdate();
|
||||
}
|
||||
|
||||
// Base imagery under photoreal is hidden rather than deleted (see show).
|
||||
function setImageryShown(shown) {
|
||||
const slot = slots.imagery;
|
||||
if (!slot || slot.hidden === !shown) return;
|
||||
slot.hidden = !shown;
|
||||
slot.layer.update({
|
||||
...slot.layerDesc,
|
||||
source: slot.source,
|
||||
raster: { show: shown },
|
||||
});
|
||||
if (slot.credits.length)
|
||||
view.attribution?.[shown ? 'add' : 'remove'](slot.credits);
|
||||
view.forceUpdate();
|
||||
}
|
||||
|
||||
@@ -126,7 +151,7 @@ export function createNavaraMaps(
|
||||
const generation = ++generations.imagery;
|
||||
const { url, credit } = imageryFor(source);
|
||||
if (destroyed || generation !== generations.imagery) return;
|
||||
if (slots.imagery?.key === url) return;
|
||||
if (slots.imagery?.key === url) return setImageryShown(true);
|
||||
show('imagery', url, { type: 'raster-tile', url }, { type: 'raster' }, [
|
||||
credit,
|
||||
]);
|
||||
@@ -157,7 +182,7 @@ export function createNavaraMaps(
|
||||
);
|
||||
if (destroyed || generation !== generations.photoreal) return;
|
||||
const url = `${GOOGLE_TILES_ROOT}?key=${encodeURIComponent(key)}`;
|
||||
clear('imagery');
|
||||
setImageryShown(false);
|
||||
if (slots.photoreal?.key === url) return;
|
||||
show(
|
||||
'photoreal',
|
||||
@@ -172,6 +197,9 @@ export function createNavaraMaps(
|
||||
}),
|
||||
],
|
||||
);
|
||||
// Google's terms want its copyright lines on screen, as Cesium shows
|
||||
// them; Navara lists them in its attribution card, collapsed by default.
|
||||
view.attribution?.show();
|
||||
},
|
||||
photorealActive: () => slots.photoreal != null,
|
||||
};
|
||||
|
||||
@@ -8,6 +8,8 @@ function fakeView() {
|
||||
layers: [],
|
||||
shown: [],
|
||||
frames: 0,
|
||||
opened: 0,
|
||||
rasterCounts: [],
|
||||
addSource(desc) {
|
||||
const source = {
|
||||
id: `source-${view.sources.length}`,
|
||||
@@ -24,14 +26,21 @@ function fakeView() {
|
||||
const layer = {
|
||||
id: `layer-${view.layers.length}`,
|
||||
desc,
|
||||
update(next) {
|
||||
layer.desc = next;
|
||||
},
|
||||
delete() {
|
||||
view.layers = view.layers.filter((entry) => entry !== layer);
|
||||
view.rasterCounts.push(layersOf(view, 'raster').length);
|
||||
},
|
||||
};
|
||||
view.layers.push(layer);
|
||||
return layer;
|
||||
},
|
||||
attribution: {
|
||||
show() {
|
||||
view.opened++;
|
||||
},
|
||||
add(items) {
|
||||
view.shown.push(...items);
|
||||
},
|
||||
@@ -159,9 +168,14 @@ test('setPhotorealTiles shows Google 3D tiles with a browser key and the Google
|
||||
'https://tile.googleapis.com/v1/3dtiles/root.json?key=browser%20key',
|
||||
);
|
||||
assert.deepEqual(tiles.desc.model, { maxSse: 16 });
|
||||
assert.equal(layersOf(view, 'raster').length, 0, 'base imagery removed');
|
||||
// Hidden, not deleted: Navara 0.2.1 stops loading terrain at the current
|
||||
// view when the last raster layer is deleted.
|
||||
const [base] = layersOf(view, 'raster');
|
||||
assert.equal(base.desc.raster?.show, false, 'base imagery not hidden');
|
||||
assert.deepEqual(view.rasterCounts, []);
|
||||
const google = view.shown.find((item) => item.creditLayerId === tiles.id);
|
||||
assert.ok(google?.logo, 'no Google logo');
|
||||
assert.ok(view.opened > 0, 'the copyright lines are not opened on screen');
|
||||
assert.equal(maps.photorealActive(), true);
|
||||
await maps.setPhotorealTiles(null);
|
||||
assert.equal(maps.photorealActive(), false);
|
||||
@@ -169,6 +183,24 @@ test('setPhotorealTiles shows Google 3D tiles with a browser key and the Google
|
||||
assert.ok(!view.shown.some((item) => item.creditLayerId));
|
||||
});
|
||||
|
||||
test('base imagery hidden by photoreal comes back; a new one replaces it without a raster gap', async () => {
|
||||
const view = fakeView();
|
||||
const { maps } = createNavaraMaps(view);
|
||||
await maps.setBaseImagery({ kind: 'osm' });
|
||||
await maps.setPhotorealTiles({ googleApiKey: 'k' });
|
||||
const [osm] = layersOf(view, 'raster');
|
||||
await maps.setPhotorealTiles(null);
|
||||
await maps.setBaseImagery({ kind: 'osm' });
|
||||
assert.deepEqual(layersOf(view, 'raster'), [osm]);
|
||||
assert.notEqual(osm.desc.raster?.show, false, 'OSM still hidden');
|
||||
view.rasterCounts.length = 0;
|
||||
await maps.setBaseImagery({ kind: 'esri-imagery' });
|
||||
const rasters = layersOf(view, 'raster');
|
||||
assert.equal(rasters.length, 1);
|
||||
assert.match(rasters[0].desc.source.desc.url, /World_Imagery/);
|
||||
assert.deepEqual(view.rasterCounts, [1], 'a raster gap while replacing');
|
||||
});
|
||||
|
||||
test('photoreal without a browser key rejects and changes nothing', async () => {
|
||||
const view = fakeView();
|
||||
const { maps } = createNavaraMaps(view);
|
||||
|
||||
Reference in new issue
Block a user