diff --git a/src/annotations/annotationEngine.js b/src/annotations/annotationEngine.js index 62bbef31f..fdca91f08 100644 --- a/src/annotations/annotationEngine.js +++ b/src/annotations/annotationEngine.js @@ -5,6 +5,7 @@ import { greatCircleMeters } from '../geoDistance.js'; import { holdContinuousRender, releaseContinuousRender, + governorRequestRender, } from '../renderGovernor.js'; import { isRateLimitedOutcome, @@ -201,8 +202,11 @@ export function createAnnotationEngine({ // World annotations carry persistent per-frame scene animation (pulsing // live colors, camera-scaled rings, route-flow uniforms in the renderer), // so the scene must render continuously while any mark exists. (perf wave 2) + // Only a mark someone can see needs every frame drawn: a board of marks + // hidden from Draw's list lets the scene idle. function syncAnnotationHold() { - if (annotations.size > 0) holdContinuousRender('annotations'); + const visible = [...annotations.values()].some((anno) => !anno.hidden); + if (visible) holdContinuousRender('annotations'); else releaseContinuousRender('annotations'); } @@ -250,6 +254,9 @@ export function createAnnotationEngine({ if (changed) { renderer.sync(annotations); syncAnnotationHold(); + // The frame that takes a just-hidden mark off: with every mark hidden + // the hold is gone, and nothing else would draw it. + governorRequestRender('annotations'); } if (expired) notifyChange(); // Stop once the board is STABLE — persistent marks (persist=true is the default) @@ -811,6 +818,11 @@ export function createAnnotationEngine({ * degenerate geocode stacks on one point (several Capitol monuments) would erase each other. */ function findDuplicate(anno) { if (!anno.anchor) return null; + // A shape drawn by hand is always new: each click is meant, and every + // shape is unnamed until its row names it, so two nearby pins would + // otherwise read as one and the second would replace or vanish into the + // first. + if (anno.manual) return null; const near = (a, b) => a && b && @@ -1017,6 +1029,11 @@ export function createAnnotationEngine({ anno.hidden = Boolean(hidden); // Skip the fade-in, so showing it again is immediate. anno.bornAt = Math.min(anno.bornAt, performance.now() - 260); + // Take it off (or put it back) now, not when its alpha next changes: a + // mark hidden before its first frame has no alpha change to wait for. + renderer.sync(annotations); + syncAnnotationHold(); + governorRequestRender('annotations'); ensureTicking(); notifyChange(); return true; @@ -1043,6 +1060,7 @@ export function createAnnotationEngine({ } catch (error) { rollbackRendererState(anno); annotations.delete(id); + renderer.sync(annotations); syncAnnotationHold(); notifyChange(); throw error; diff --git a/src/annotations/annotationEngine.test.mjs b/src/annotations/annotationEngine.test.mjs index 1f0a6958f..833c66146 100644 --- a/src/annotations/annotationEngine.test.mjs +++ b/src/annotations/annotationEngine.test.mjs @@ -987,3 +987,108 @@ test('a drawn line that rounds up to the next unit is labelled in it, not as 100 assert.deepEqual(labels, ['990 m', '1.0 km', '9.9 km', '10 km']); engine.destroy(); }); + +test('hand-drawn marks: two unnamed pins close together are two pins, whatever their colours', async (t) => { + installAnimationFrameStubs(t); + _resetRenderGovernorForTest(); + t.after(() => _resetRenderGovernorForTest()); + const { renderer } = fakeRenderer(); + const engine = createAnnotationEngine({ viewer: {}, renderer }); + const pin = (lon, color) => ({ type: 'pin', manual: true, latitude: 30.27, longitude: lon, label: '', color }); + await engine.annotate([pin(-97.74, 'primary')], { persist: true }); + await engine.annotate([pin(-97.7402, 'primary')], { persist: true }); + await engine.annotate([pin(-97.7404, 'red')], { persist: true }); + assert.deepEqual( + engine.list().map(({ color }) => color), + ['primary', 'primary', 'red'], + 'no drawn pin is dropped or replaced by the next', + ); +}); + +test('hand-drawn marks: hiding a mark before its first frame takes it off at once', async (t) => { + installAnimationFrameStubs(t); + _resetRenderGovernorForTest(); + t.after(() => _resetRenderGovernorForTest()); + let syncs = 0; + const renderer = { add() {}, update() {}, remove() {}, sync() { syncs += 1; }, destroy() {} }; + const engine = createAnnotationEngine({ viewer: {}, renderer }); + await engine.annotate([{ type: 'pin', manual: true, latitude: 1, longitude: 1 }], { persist: true }); + const [mark] = engine.list(); + const before = syncs; + engine.setHidden(mark.id, true); + assert.ok(syncs > before, 'the renderer hears about it now'); + assert.ok( + !getRenderGovernorDiagnostics().holds.includes('annotations'), + 'and the render hold goes with the last visible mark', + ); +}); + +test('hand-drawn marks: with every mark hidden the scene idles, and showing one wakes it', async (t) => { + installAnimationFrameStubs(t); + _resetRenderGovernorForTest(); + t.after(() => _resetRenderGovernorForTest()); + let renders = 0; + installRenderGovernor({ scene: { requestRenderMode: true, requestRender: () => (renders += 1) } }); + const { renderer } = fakeRenderer(); + const engine = createAnnotationEngine({ viewer: {}, renderer }); + await engine.annotate([ + { type: 'pin', manual: true, latitude: 1, longitude: 1 }, + { type: 'pin', manual: true, latitude: 2, longitude: 2 }, + ], { persist: true }); + const [first, second] = engine.list(); + const held = () => getRenderGovernorDiagnostics().holds.includes('annotations'); + assert.ok(held(), 'visible marks keep the scene drawing'); + engine.setHidden(first.id, true); + assert.ok(held(), 'one visible mark is enough'); + const before = renders; + engine.setHidden(second.id, true); + assert.equal(held(), false, 'every mark hidden: the scene idles'); + assert.ok(renders > before, 'after one last frame that takes them off'); + engine.setHidden(second.id, false); + assert.ok(held(), 'showing one wakes it again'); +}); + +test('hand-drawn marks: a restyle the renderer cannot draw takes the mark off and says so', async (t) => { + installAnimationFrameStubs(t); + _resetRenderGovernorForTest(); + t.after(() => _resetRenderGovernorForTest()); + const { renderer, failAddOnce } = throwingRendererHarness(); + let syncs = 0; + renderer.sync = () => (syncs += 1); + const engine = createAnnotationEngine({ viewer: {}, renderer }); + await engine.annotate([{ type: 'pin', manual: true, latitude: 1, longitude: 1 }], { persist: true }); + const [mark] = engine.list(); + let changes = 0; + engine.onChange(() => (changes += 1)); + const before = syncs; + failAddOnce(); + assert.throws(() => engine.restyle(mark.id, { color: 'red' }), /WebGL context lost/); + assert.equal(engine.list().length, 0, 'no ghost row is left behind'); + assert.equal(changes, 1, 'the list hears about it'); + assert.ok(syncs > before, 'and the renderer is told'); +}); + +test('hand-drawn marks: a mark shown again right after it was drawn appears at once, not fading in', async (t) => { + _resetRenderGovernorForTest(); + t.after(() => _resetRenderGovernorForTest()); + const queued = []; + const raf = globalThis.requestAnimationFrame; + const caf = globalThis.cancelAnimationFrame; + globalThis.requestAnimationFrame = (callback) => queued.push(callback); + globalThis.cancelAnimationFrame = () => {}; + t.after(() => { + globalThis.requestAnimationFrame = raf; + globalThis.cancelAnimationFrame = caf; + }); + const tick = () => queued.splice(0).forEach((callback) => callback()); + const { renderer } = fakeRenderer(); + const engine = createAnnotationEngine({ viewer: {}, renderer }); + await engine.annotate([{ type: 'pin', manual: true, latitude: 1, longitude: 1 }], { persist: true }); + const [mark] = engine.list(); + engine.setHidden(mark.id, true); + tick(); + assert.equal(mark.alpha, 0); + engine.setHidden(mark.id, false); + tick(); + assert.equal(mark.alpha, 1, 'no fade-in after a hide'); +}); diff --git a/src/annotations/screenAnnotationRenderer.js b/src/annotations/screenAnnotationRenderer.js index f6c780c8f..2ae61f751 100644 --- a/src/annotations/screenAnnotationRenderer.js +++ b/src/annotations/screenAnnotationRenderer.js @@ -483,7 +483,9 @@ export function createScreenAnnotationRenderer( for (const rec of records.values()) { const { anno, group, parts } = rec; - if (rec.hidden) { + // A mark hidden from Draw's list is not projected, so it neither shows + // nor pushes a visible callout aside in the placement below. + if (rec.hidden || anno.hidden) { group.display = 'none'; continue; } diff --git a/src/annotations/screenAnnotationRenderer.test.mjs b/src/annotations/screenAnnotationRenderer.test.mjs index 8f2431b7f..ed1bb7948 100644 --- a/src/annotations/screenAnnotationRenderer.test.mjs +++ b/src/annotations/screenAnnotationRenderer.test.mjs @@ -390,3 +390,49 @@ test("an earlier renderer leaves a later renderer's board alone", async (t) => { assert.equal(groups().length, 1); assert.equal(groups()[0].querySelector('text').textContent, 'Kept'); }); + +test('a mark hidden from Draw is not drawn or placed, and comes back when shown', async (t) => { + // happy-dom measures every SVG box the same, so callout spreading cannot + // settle here; a hidden mark simply has to stay out of drawing and + // placement, which is what keeps it from pushing a visible callout aside. + const { store, svg } = board(t); + projectTo(t, { x: 200, y: 300 }); + const camera = cameraOver(0, 0); + let project = null; + const renderer = createScreenAnnotationRenderer( + { + scene: sceneFor(camera, { + postRender: { + addEventListener: (listener) => (project = listener), + removeEventListener() {}, + }, + }), + camera, + trackedEntity: null, + }, + { store, activeTrackedReadoutId: () => null }, + ); + t.after(() => renderer.destroy()); + const camp = { + id: 'camp', + type: 'pin', + color: 'amber', + label: 'Camp', + alpha: 1, + hidden: true, + anchor: { lon: 0, lat: 0, height: 0 }, + }; + renderer.add(camp); + project(); + await settle(); + const drawn = () => + [...svg().querySelectorAll('g.gev-anno')].some( + (group) => + group.textContent.includes('Camp') && group.style.display !== 'none', + ); + assert.equal(drawn(), false, 'the hidden mark is not drawn'); + camp.hidden = false; + project(); + await settle(); + assert.equal(drawn(), true, 'shown again, it is drawn'); +}); diff --git a/src/annotations/worldAnnotationRenderer.js b/src/annotations/worldAnnotationRenderer.js index 9e077acce..692b3d4ff 100644 --- a/src/annotations/worldAnnotationRenderer.js +++ b/src/annotations/worldAnnotationRenderer.js @@ -51,6 +51,8 @@ export function createWorldAnnotationRenderer(viewer) { const dataSource = new Cesium.CustomDataSource('gev-annotations'); viewer.dataSources.add(dataSource); const osmAnnotations = new Set(); + // Every mark with entities on the globe, so sync() can take a hidden one out. + const drawn = new Set(); // Register the GevRouteFlow fabric once so Cesium's Material.fromType() can build the // material the route pipeline renders. The animated `time` uniform is read straight @@ -89,6 +91,7 @@ export function createWorldAnnotationRenderer(viewer) { // path cannot reach them and they stay on the globe forever. const entities = []; anno._entities = entities; + drawn.add(anno); if ( anno.ring && @@ -247,6 +250,7 @@ export function createWorldAnnotationRenderer(viewer) { width: 9, material: new FlowMaterialProperty( PALETTE[anno.color] || PALETTE.primary, + () => anno.alpha ?? 1, ), clampToGround: true, classificationType: CLASSIFY, @@ -320,6 +324,7 @@ export function createWorldAnnotationRenderer(viewer) { function remove(anno) { hideOsmCredit(viewer, anno); osmAnnotations.delete(anno); + drawn.delete(anno); if (!anno?._entities) return; for (const entity of anno._entities) { try { @@ -331,13 +336,23 @@ export function createWorldAnnotationRenderer(viewer) { anno._entities = null; } + /** + * CallbackProperty drives per-frame alpha and pulse. A hidden mark is taken + * out whole, so no material that keeps its own colour (the route's flow), + * ground classification or pick can still show it. + */ function sync() { - // No-op: CallbackProperty drives per-frame alpha/pulse animation. + for (const anno of drawn) { + const show = !anno.hidden; + for (const entity of anno._entities || []) + if (entity.show !== show) entity.show = show; + } } function destroy() { for (const anno of osmAnnotations) hideOsmCredit(viewer, anno); osmAnnotations.clear(); + drawn.clear(); try { viewer.dataSources.remove(dataSource, true); } catch { @@ -407,8 +422,11 @@ export function ensureFlowFabricRegistered() { * returned a standalone Material Cesium never rendered, or treated `result` as a * Material — both left the real uniforms untouched, so nothing animated.) */ -export function FlowMaterialProperty(colorCss) { +export function FlowMaterialProperty(colorCss, alphaOf = () => 1) { this._color = Cesium.Color.fromCssColorString(colorCss).withAlpha(0.95); + // The mark's live alpha (fade in and out, hidden), applied every frame. + this._alphaOf = alphaOf; + this._live = this._color.clone(); this._definitionChanged = new Cesium.Event(); } Object.defineProperties(FlowMaterialProperty.prototype, { @@ -431,7 +449,8 @@ FlowMaterialProperty.prototype.getValue = function getValue(time, result) { // `time` is read straight from the wall clock so the dashes flow every rendered // frame (the scene renders continuously; no requestRenderMode here). if (!Cesium.defined(result)) result = {}; - result.color = this._color; + this._live.alpha = this._color.alpha * this._alphaOf(); + result.color = this._live; result.time = performance.now() / 1000; // the per-frame animated value result.repeat = 64.0; result.duty = 0.46; diff --git a/src/annotations/worldAnnotationRenderer.test.mjs b/src/annotations/worldAnnotationRenderer.test.mjs new file mode 100644 index 000000000..a05c52373 --- /dev/null +++ b/src/annotations/worldAnnotationRenderer.test.mjs @@ -0,0 +1,70 @@ +// A mark hidden from Draw's list leaves the globe whole: every entity it put +// there is taken out on sync and comes back when shown, and the line's +// flowing stroke, which keeps its own colour, follows the mark's alpha. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import * as C from 'cesium'; +import { + FlowMaterialProperty, + createWorldAnnotationRenderer, +} from './worldAnnotationRenderer.js'; + +const noop = () => {}; +function viewer(t) { + for (const name of ['HTMLCanvasElement', 'HTMLImageElement', 'ImageBitmap', 'OffscreenCanvas']) { + const original = globalThis[name]; + globalThis[name] = class {}; + t.after(() => { if (original === undefined) delete globalThis[name]; else globalThis[name] = original; }); + } + const before = globalThis.document; + globalThis.document = { addEventListener: noop, removeEventListener: noop }; + t.after(() => { if (before === undefined) delete globalThis.document; else globalThis.document = before; }); + return { + creditDisplay: { addStaticCredit: noop, removeStaticCredit: noop }, + entities: new C.EntityCollection(), + dataSources: new C.DataSourceCollection(), + camera: { moveEnd: new C.Event(), positionCartographic: { height: 1000 } }, + scene: { requestRender: noop, canvas: { addEventListener: noop, removeEventListener: noop }, preRender: new C.Event(), + primitives: { add: (p) => p, remove: noop }, screenSpaceCameraController: { enableInputs: true } }, + }; +} + +const marks = () => [ + { id: 'route', type: 'route', color: 'amber', alpha: 1, anchor: { lat: 0, lon: 0 }, path: [{ lat: 0, lon: 0 }, { lat: 0.01, lon: 0.01 }] }, + { id: 'area', type: 'area', color: 'green', alpha: 1, anchor: { lat: 0.005, lon: 0.005 }, ring: [[0, 0], [0.01, 0], [0.01, 0.01], [0, 0]] }, + { id: 'pin', type: 'pin', color: 'primary', alpha: 1, anchor: { lat: 0.02, lon: 0.02 } }, +]; + +test('a hidden mark has every entity taken off on sync, and back when shown', (t) => { + const renderer = createWorldAnnotationRenderer(viewer(t)); + const [route, area, pin] = marks(); + for (const anno of [route, area, pin]) renderer.add(anno); + const shown = (anno) => anno._entities.map((entity) => entity.show); + for (const anno of [route, area, pin]) assert.ok(anno._entities.length > 0, anno.id); + + route.hidden = true; + area.hidden = true; + renderer.sync(); + assert.ok(shown(route).every((show) => show === false), 'the line is off'); + assert.ok(shown(area).every((show) => show === false), 'the area is off'); + assert.ok(shown(pin).every((show) => show === true), 'a visible mark stays'); + + route.hidden = false; + renderer.sync(); + assert.ok(shown(route).every((show) => show === true), 'the line is back'); + renderer.remove(area); + renderer.sync(); + assert.equal(area._entities, null, 'a removed mark is no longer synced'); + renderer.destroy(); +}); + +test('the line\'s flowing stroke follows its mark\'s alpha', () => { + let alpha = 1; + const material = new FlowMaterialProperty('#ffb547', () => alpha); + assert.ok(Math.abs(material.getValue(null, {}).color.alpha - 0.95) < 1e-9); + alpha = 0; + assert.equal(material.getValue(null, {}).color.alpha, 0, 'hidden: nothing painted'); + alpha = 0.5; + assert.ok(Math.abs(material.getValue(null, {}).color.alpha - 0.475) < 1e-9, 'fading'); + assert.equal(new FlowMaterialProperty('#fff').getValue(null, {}).color.alpha, 0.95, 'no alpha source: opaque as before'); +}); diff --git a/src/cameraVerbs.js b/src/cameraVerbs.js index 231d9357d..e037472cb 100644 --- a/src/cameraVerbs.js +++ b/src/cameraVerbs.js @@ -1746,8 +1746,13 @@ export function flyRoute( const speed = ROUTE_M_S[String(args.speed || 'normal').toLowerCase()] ? String(args.speed || 'normal').toLowerCase() : 'normal'; + // A route hidden from Draw's list is not on screen, so it is not flown. const routes = (annoList || []).filter( - (a) => a.type === 'route' && Array.isArray(a.path) && a.path.length >= 2, + (a) => + a.type === 'route' && + !a.hidden && + Array.isArray(a.path) && + a.path.length >= 2, ); if (!routes.length) { return { diff --git a/src/routeCinematics.test.mjs b/src/routeCinematics.test.mjs index fa3e9d823..622c8ab63 100644 --- a/src/routeCinematics.test.mjs +++ b/src/routeCinematics.test.mjs @@ -1078,3 +1078,20 @@ test('a new viewer lifetime drops the previous view-target getter', () => { interruptCameraMotion('test-cleanup'); second.restore(); }); + +test('a route hidden from Draw is not flown; the newest visible one is', () => { + const viewer = createTickableViewer(); + initCameraVerbs(viewer.viewer, () => null); + const route = (label, hidden) => ({ + type: 'route', + label, + hidden, + path: TWO_TURN_ROUTE.map(([lon, lat]) => ({ lon, lat, height: 0 })), + }); + const onlyHidden = flyRoute([route('hidden walk', true)]); + assert.equal(onlyHidden.ok, false, 'a board of hidden routes has none to fly'); + const started = flyRoute([route('shown walk', false), route('hidden walk', true)]); + assert.equal(started.ok, true); + assert.match(JSON.stringify(started), /shown walk/, 'the visible route is flown'); + interruptCameraMotion?.('test'); +});