diff --git a/src/globe/cpuPicks.js b/src/globe/cpuPicks.js index fed3ced4f..a37b9f2b1 100644 --- a/src/globe/cpuPicks.js +++ b/src/globe/cpuPicks.js @@ -156,10 +156,14 @@ export function createCpuPicker({ projector, camera, movers, lines }) { } } - const drill = (at, limit = Infinity, { includeMovers = true } = {}) => { + const drill = ( + at, + limit = Infinity, + { includeMovers = true, includeLines = true } = {}, + ) => { const hits = []; if (includeMovers) moverHits(at, hits); - lineHits(at, hits); + if (includeLines) lineHits(at, hits); hits.sort((a, b) => a.depth - b.depth); return hits.slice(0, limit).map(({ result }) => result); }; diff --git a/src/globe/navara/picking.js b/src/globe/navara/picking.js index f84aa0d36..1607caa4a 100644 --- a/src/globe/navara/picking.js +++ b/src/globe/navara/picking.js @@ -38,6 +38,10 @@ export function createNavaraPicking({ now = () => performance.now(), }) { const drillPick = createCpuPicker({ projector, camera, movers, lines }); + const isModelSet = (id) => + movers().some( + ({ options }) => options.id === id && options.kind === 'model', + ); // Clicks waiting for Navara's GPU answer, oldest first. const waiting = []; @@ -55,10 +59,25 @@ export function createNavaraPicking({ // frame): a set that changes its instances after it still names the // instance that was under the pointer. const resolveInfo = (info) => (info ? resolveBatch(info.batchId) : null); + // A GPU answer is checked against the CPU pick at the click: an instance + // 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. const answerFrom = (hit, at) => { - if (hit && !drillPick.near(hit.set, hit.instance, at)) { - stats.stale++; - return drillPick(at, 1)[0]; + if (hit) { + // The instance the CPU sees on top (lines are not in the GPU pick). + const top = drillPick(at, 1, { includeLines: false })[0]; + const stale = + !drillPick.near(hit.set, hit.instance, at, 1) || + (top !== undefined && + !(top.owner === hit.set && top.instance === hit.instance) && + !isModelSet(hit.set)); + if (stale) { + stats.stale++; + return drillPick(at, 1)[0]; + } } return hit ? { id: hit.pickId, owner: hit.set, instance: hit.instance } diff --git a/src/globe/navara/picking.test.mjs b/src/globe/navara/picking.test.mjs index b84f8c119..50080690b 100644 --- a/src/globe/navara/picking.test.mjs +++ b/src/globe/navara/picking.test.mjs @@ -296,3 +296,37 @@ test('the pick id is the one the instance had when Navara answered', async () => time += 1; assert.equal((await picking.clickPick({ x: 10, y: 10 })).id, 'before'); }); + +test('a stale GPU answer for a neighbour of the click never wins over the instance under it (20 gestures)', async () => { + // The followed aircraft A sits at the centre; B is drawn 15 px from it. + // Navara answers B's click with the previous gesture's pick (A): within + // twice A's pick radius, but the instance under the click is B. + const view = fakeView(); + const sets = [ + moverSet('flights', [ + [100, 100, 5, 'a'], + [115, 100, 5, 'b'], + ]), + ]; + const picking = createNavaraPicking({ + view, + projector, + camera, + movers: () => sets, + lines: () => [], + resolveBatch: (batchId) => + batchId === 1 + ? { set: 'flights', instance: 0, pickId: 'a' } + : { set: 'flights', instance: 1, pickId: 'b' }, + ...timers(), + }); + for (let gesture = 0; gesture < 20; gesture++) { + const onA = picking.clickPick({ x: 100, y: 100 }); + view.emit('featureClick', { batchId: 1 }); + assert.equal((await onA).id, 'a', `gesture ${gesture}: a`); + const onB = picking.clickPick({ x: 115, y: 100 }); + view.emit('featureClick', { batchId: 1 }); // stale: A again + assert.equal((await onB).id, 'b', `gesture ${gesture}: b, not the stale a`); + } + assert.equal(picking.clickStats().stale, 20); +});