From 620ada6df3fc89bf849a86d52b198e4273062cf9 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Sat, 17 Oct 2026 13:26:42 +0400 Subject: [PATCH] fix(globe): Navara picks rank as drawn and check model hits at their drawn radius MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T11 review follow-up to ed9ea7e6. CPU hits rank as the engine draws them: sets without a depth test over depth-tested ones, then by set order (the tracked bank over the fleet, flights over local ADS-B), then depth — so the CPU's top hit that checks a GPU answer is the instance drawn on top. A model hit is no longer taken as answered: the model mover keeps each instance's drawn diameter (after minimumPixelSize/maximumScale) and the CPU checks the hit against its projected position within that radius (+3 px); the CPU pick sees models at that radius too. --- src/globe/cpuPicks.js | 51 ++++++++++--- src/globe/navara/movers/index.js | 5 +- src/globe/navara/movers/modelMover.js | 13 ++++ src/globe/navara/movers/modelMover.test.mjs | 5 +- src/globe/navara/picking.js | 6 +- src/globe/navara/picking.test.mjs | 81 +++++++++++++++++++++ 6 files changed, 146 insertions(+), 15 deletions(-) diff --git a/src/globe/cpuPicks.js b/src/globe/cpuPicks.js index a37b9f2b1..7ca47dc09 100644 --- a/src/globe/cpuPicks.js +++ b/src/globe/cpuPicks.js @@ -1,8 +1,13 @@ // CPU picks of movers and lines for every engine that needs them (Navara's // hover, drill and synchronous pick, the fake engine): projected positions // within the drawn size (+3 px, Navara's PICK_RADIUS), lines within 8 px -// (or half their width + 3 px when wider), hidden behind the globe excluded, nearest to the -// camera first. Pure: src/geo and the engine's projector only. +// (or half their width + 3 px when wider), hidden behind the globe excluded. +// Hits rank as they are drawn on top: sets drawn without a depth test +// ('always') over depth-tested ones, then by set order (higher drawn later, +// on top: the tracked bank over the fleet, flights over local ADS-B), then +// nearest to the camera. Models are checked against their drawn radius when +// the engine knows it (pickRadiusPx). Pure: src/geo and the engine's +// projector only. import * as geo from '../geo/index.js'; @@ -62,7 +67,9 @@ export function distanceScale(scaleByDistance, distance) { return nearScale + (farScale - nearScale) * t; } -function moverRadius(options, scale, distance) { +function moverRadius(options, scale, distance, modelRadiusPx = null) { + if (options.kind === 'model' && modelRadiusPx != null) + return Math.max(modelRadiusPx, PICK_RADIUS_PX) + PICK_RADIUS_PX; const style = options.kind === 'sprite' ? options.sprite @@ -101,8 +108,10 @@ export function createCpuPicker({ projector, camera, movers, lines }) { geo.cartesian3.distance(position, camera.livePosition); function moverHits(at, hits) { - for (const { options, store } of movers()) { + for (const { options, store, pickRadiusPx } of movers()) { if (!options.pickable) continue; + const onTop = options.depthTest === 'always' ? 1 : 0; + const order = Number(options.order) || 0; const { positions, shows, scales, pickIds } = store; for (let index = 0; index < store.count; index++) { if (shows[index] !== 1) continue; @@ -116,8 +125,14 @@ export function createCpuPicker({ projector, camera, movers, lines }) { if (!screen || !projector.horizonVisible(scratch)) continue; const distance = Math.hypot(screen.x - at.x, screen.y - at.y); const depth = depthOf(scratch); - if (distance > moverRadius(options, scales[index], depth)) continue; + if ( + distance > + moverRadius(options, scales[index], depth, pickRadiusPx?.(index)) + ) + continue; hits.push({ + onTop, + order, depth, result: { id: pickIds[index], owner: options.id, instance: index }, }); @@ -150,6 +165,9 @@ export function createCpuPicker({ projector, camera, movers, lines }) { } if (best !== null) hits.push({ + // Lines are depth-tested and drawn in their own order. + onTop: 0, + order: Number(options.order) || 0, depth: best, result: { id: options.pickId, owner: options.id }, }); @@ -164,18 +182,23 @@ export function createCpuPicker({ projector, camera, movers, lines }) { const hits = []; if (includeMovers) moverHits(at, hits); if (includeLines) lineHits(at, hits); - hits.sort((a, b) => a.depth - b.depth); + hits.sort( + (a, b) => b.onTop - a.onTop || b.order - a.order || a.depth - b.depth, + ); return hits.slice(0, limit).map(({ result }) => result); }; /** * Whether a mover instance is drawn near a window position: within its - * pick radius times `slack` (a check of a GPU answer). Unknown sets and - * models (whose drawn size the CPU does not know) count as near. + * pick radius times `slack` (a check of a GPU answer). Unknown sets, and + * models whose drawn size the engine does not know yet, count as near. */ drill.near = (owner, instance, at, slack = 2) => { const found = movers().find(({ options }) => options.id === owner); - if (!found || found.options.kind === 'model') return true; - const { options, store } = found; + if (!found) return true; + const { options, store, pickRadiusPx } = found; + const modelRadius = + options.kind === 'model' ? (pickRadiusPx?.(instance) ?? null) : null; + if (options.kind === 'model' && modelRadius == null) return true; if (instance >= store.count) return false; geo.cartesian3.fromElements( store.positions[instance * 3], @@ -187,7 +210,13 @@ export function createCpuPicker({ projector, camera, movers, lines }) { if (!screen) return false; return ( Math.hypot(screen.x - at.x, screen.y - at.y) <= - moverRadius(options, store.scales[instance], depthOf(scratch)) * slack + moverRadius( + options, + store.scales[instance], + depthOf(scratch), + modelRadius, + ) * + slack ); }; return drill; diff --git a/src/globe/navara/movers/index.js b/src/globe/navara/movers/index.js index cd80e5a46..2e6128c4b 100644 --- a/src/globe/navara/movers/index.js +++ b/src/globe/navara/movers/index.js @@ -167,6 +167,7 @@ export function createNavaraMovers(view, { overlay }) { }); return { ready: mover.ready, + pickRadiusPx: mover.pickRadiusPx, commit() { mover.commit(store.dirty()); request(); @@ -183,9 +184,11 @@ export function createNavaraMovers(view, { overlay }) { resolvePick: (batchId) => pickIndex.resolve(batchId), /** The live sets' options and stores, for CPU picks. */ pickables: () => - [...registry.sets.values()].map(({ options, store }) => ({ + [...registry.sets.values()].map(({ options, store, part }) => ({ options, store, + // Models: the drawn radius the CPU checks a model hit against. + pickRadiusPx: part?.pickRadiusPx ?? null, })), destroy: () => registry.destroyAll(), }; diff --git a/src/globe/navara/movers/modelMover.js b/src/globe/navara/movers/modelMover.js index 4f029b424..ed946e861 100644 --- a/src/globe/navara/movers/modelMover.js +++ b/src/globe/navara/movers/modelMover.js @@ -109,6 +109,8 @@ export function createModelMover(options, store, { load, onLoad = () => {} }) { let disposed = false; const colorAttributes = []; const materials = []; + // Each instance's drawn diameter (px) at its last write (CPU picks). + const drawnPx = new Float32Array(capacity); // Translucent only while an instance is faded (opaque models keep the // opaque pass). const syncTransparency = () => { @@ -128,6 +130,7 @@ export function createModelMover(options, store, { load, onLoad = () => {} }) { const { positions, orientations, scales, shows, colors } = store; for (let index = from; index < to; index++) { let scale = shows[index] === 1 ? scales[index] : 0; + drawnPx[index] = 0; if (scale > 0 && cameraPosition && footprint > 0) { const dx = positions[index * 3] - cameraPosition.x; const dy = positions[index * 3 + 1] - cameraPosition.y; @@ -146,6 +149,7 @@ export function createModelMover(options, store, { load, onLoad = () => {} }) { if (pixels < model.minimumPixelSize) scale *= model.minimumPixelSize / pixels; scale = Math.min(scale, model.maximumScale); + drawnPx[index] = (pixels * scale) / scales[index]; } scratchTranslation.set( positions[index * 3] - origin.x, @@ -237,6 +241,15 @@ export function createModelMover(options, store, { load, onLoad = () => {} }) { uniforms, ready: () => loaded, origin: () => origin, + /** + * The drawn radius (px) of an instance at its last write, or null when + * not known yet (not loaded, no camera): CPU picks check a model hit + * against it. + */ + pickRadiusPx: (index) => + loaded && index < store.count && drawnPx[index] > 0 + ? drawnPx[index] / 2 + : null, commit(range) { if (!origin && store.count > 0) placeAt({ diff --git a/src/globe/navara/movers/modelMover.test.mjs b/src/globe/navara/movers/modelMover.test.mjs index f3aefbc45..034d650e5 100644 --- a/src/globe/navara/movers/modelMover.test.mjs +++ b/src/globe/navara/movers/modelMover.test.mjs @@ -85,7 +85,10 @@ test('a far model keeps its minimum pixel size', async () => { assert.ok( Math.abs(scale - expected) / expected < 1e-6, `${scale} vs ${expected}`, - ); + ); // CPU picks read the drawn radius: half the 40 px minimum (the pixel + // size at the pre-minimum distance, so within a radius's worth). + assert.ok(Math.abs(mover.pickRadiusPx(0) - 20) < 1e-3, mover.pickRadiusPx(0)); + assert.equal(mover.pickRadiusPx(1), null); }); test("the instance colour (sRGB, decoded) tints by the set's blend amount; its alpha is opacity, as Cesium's model colour", async () => { diff --git a/src/globe/navara/picking.js b/src/globe/navara/picking.js index 1607caa4a..9804e7037 100644 --- a/src/globe/navara/picking.js +++ b/src/globe/navara/picking.js @@ -63,8 +63,10 @@ export function createNavaraPicking({ // the CPU sees must be drawn under the click (its pick radius, no slack) // and be the one the CPU finds on top. A stale answer from the previous // gesture (the followed aircraft at the centre) lay within twice its - // radius of a neighbour's click and won (10b/11b flake). Models, which - // the CPU pick does not see, are still taken as answered. + // radius of a neighbour's click and won (10b/11b flake). A model hit is + // checked against its projected position within its drawn radius + // (pickRadiusPx); the CPU's top hit (drawn order first) is not asked of + // it, as a model is drawn depth-tested among the glyphs. const answerFrom = (hit, at) => { if (hit) { // The instance the CPU sees on top (lines are not in the GPU pick). diff --git a/src/globe/navara/picking.test.mjs b/src/globe/navara/picking.test.mjs index 50080690b..cd750edfc 100644 --- a/src/globe/navara/picking.test.mjs +++ b/src/globe/navara/picking.test.mjs @@ -330,3 +330,84 @@ test('a stale GPU answer for a neighbour of the click never wins over the instan } assert.equal(picking.clickStats().stale, 20); }); + +function modelSet(id, rows, radiusPx) { + const options = normalizeMoverSetOptions({ + id, + kind: 'model', + capacity: rows.length, + pickable: true, + model: { url: 'model.glb', minimumPixelSize: 24 }, + }); + const store = createMoverStore(options); + rows.forEach(([x, y, z, pickId], index) => + store.set(index, { position: { x, y, z }, pickId }), + ); + store.setCount(rows.length); + return { options, store, pickRadiusPx: () => radiusPx }; +} + +test('CPU hits rank as drawn: always-on-top sets, then set order, then depth', () => { + const ordered = (id, order, depthTest, z) => { + const options = normalizeMoverSetOptions({ + id, + kind: 'point', + capacity: 1, + pickable: true, + order, + depthTest, + point: { sizePx: 20 }, + }); + const store = createMoverStore(options); + store.set(0, { position: { x: 100, y: 100, z }, pickId: id }); + store.setCount(1); + return { options, store }; + }; + const sets = [ + ordered('local-adsb', -1, 'always', 5), // nearest, drawn under flights + ordered('fleet', 0, 'always', 50), + ordered('tracked', 2, 'always', 500), // farthest, drawn on top + ordered('depth-tested', 5, 'normal', 1), // nearest of all, depth-tested + ]; + const picking = createNavaraPicking({ + view: fakeView(), + projector, + camera, + movers: () => sets, + lines: () => [], + resolveBatch: () => null, + ...timers(), + }); + assert.deepEqual( + picking.drillPick({ x: 100, y: 100 }).map(({ id }) => id), + ['tracked', 'fleet', 'local-adsb', 'depth-tested'], + ); +}); + +test('a model hit is checked against its projected position within its drawn radius', async () => { + const view = fakeView(); + // A model drawn 40 px across (radius 20) at (100, 100). + const sets = [modelSet('models', [[100, 100, 50, 'jet']], 20)]; + const picking = createNavaraPicking({ + view, + projector, + camera, + movers: () => sets, + lines: () => [], + resolveBatch: () => ({ set: 'models', instance: 0, pickId: 'jet' }), + ...timers(), + }); + // Within its radius (+3 px): the GPU answer stands. + const inside = picking.clickPick({ x: 118, y: 100 }); + view.emit('featureClick', { batchId: 1 }); + assert.equal((await inside)?.id, 'jet'); + // A stale answer naming it from 60 px away: the CPU pick answers (nothing + // drawn there). + const outside = picking.clickPick({ x: 160, y: 100 }); + view.emit('featureClick', { batchId: 1 }); + assert.equal(await outside, undefined); + assert.equal(picking.clickStats().stale, 1); + // The CPU pick sees the model at its drawn radius, not the 24 px minimum. + assert.equal(picking.drillPick({ x: 121, y: 100 })[0]?.id, 'jet'); + assert.equal(picking.drillPick({ x: 125, y: 100 }).length, 0); +});