Skip to content

Commit eb138a3

Browse files
committed
Drop queued moves
1 parent 018b0ca commit eb138a3

11 files changed

Lines changed: 196 additions & 26 deletions

docs/src/pages/charts/SwimlaneChartPage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ ref.current.push({ lane: "Backend", task: "Auth", value: 2 })
411411
so borders always match the surrounding area.
412412
</p>
413413

414-
<div ref={null} style={{ background: "var(--surface-1)", borderRadius: 8, padding: 16, border: "1px solid var(--surface-3)", overflow: "hidden", marginBottom: 16 }}>
414+
<div style={{ background: "var(--surface-1)", borderRadius: 8, padding: 16, border: "1px solid var(--surface-3)", overflow: "hidden", marginBottom: 16 }}>
415415
<SwimlaneChart
416416
data={STATIC_DATA}
417417
categoryAccessor="lane"

src/components/stream/GeoCanvasHitTester.ts

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { GeoAreaSceneNode, GeoSceneNode } from "./geoTypes"
22
import type { PointSceneNode, LineSceneNode } from "./types"
33
import type { Quadtree } from "d3-quadtree"
44
import { getHitRadius } from "./hitTestUtils"
5+
import { findHitPointInQuadtree } from "./quadtreeHitTest"
56

67
export interface GeoHitResult {
78
node: GeoSceneNode
@@ -31,22 +32,11 @@ export function findNearestGeoNode(
3132
// ── 1. Point nodes ──────────────────────────────────────────────
3233

3334
if (pointQuadtree) {
34-
// Widen the query radius so the quadtree returns candidates that are
35-
// farther than maxDistance but still within their own hit radius.
36-
// `getHitRadius(r) = max(r + 5, 12, maxDistance)` so we widen to that.
37-
const queryRadius = Math.max(maxDistance, maxPointRadius + 5, 12)
38-
const candidate = pointQuadtree.find(mouseX, mouseY, queryRadius) ?? null
39-
if (candidate) {
40-
const dx = candidate.x - mouseX
41-
const dy = candidate.y - mouseY
42-
const dist = Math.sqrt(dx * dx + dy * dy)
43-
const hitRadius = getHitRadius(candidate.r, maxDistance)
44-
if (dist <= hitRadius) {
45-
return { node: candidate, distance: dist }
46-
}
47-
}
48-
// Quadtree result is authoritative when its query radius covers the
49-
// largest possible hit radius — no need for the linear fallback.
35+
// Visit every candidate within the widened search radius so variable-size
36+
// points don't miss hits (quadtree.find's nearest-only behavior would
37+
// shadow a farther, larger point that still contains the cursor).
38+
const hit = findHitPointInQuadtree(pointQuadtree, mouseX, mouseY, maxDistance, maxPointRadius)
39+
if (hit) return hit
5040
} else {
5141
// Linear scan when no spatial index is available
5242
let bestPoint: PointSceneNode | null = null

src/components/stream/OrdinalCanvasHitTester.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { OrdinalSceneNode, WedgeSceneNode, BoxplotSceneNode, ViolinSceneNod
22
import type { PointSceneNode, RectSceneNode, HoverData } from "./types"
33
import type { Quadtree } from "d3-quadtree"
44
import { hitTestRect as sharedHitTestRect, normalizeAngle, getHitRadius } from "./hitTestUtils"
5+
import { findHitPointInQuadtree } from "./quadtreeHitTest"
56

67
export interface OrdinalHitResult {
78
datum: any
@@ -24,15 +25,18 @@ export function findNearestOrdinalNode(
2425
let pointHitConfirmed = false
2526

2627
// Fast path: quadtree-accelerated point hit test for swarm plots.
28+
// Uses `visit()` rather than `find()` so variable-size points (where a
29+
// farther point with a larger r can still be a valid hit) aren't missed.
2730
if (pointQuadtree) {
28-
const queryRadius = Math.max(maxDistance, maxPointRadius + 5, 12)
29-
const candidate = pointQuadtree.find(px, py, queryRadius) ?? null
30-
if (candidate) {
31-
const result = hitTestPoint(candidate, px, py, maxDistance)
32-
if (result && result.distance < maxDistance) {
33-
best = result
34-
pointHitConfirmed = true
31+
const hit = findHitPointInQuadtree(pointQuadtree, px, py, maxDistance, maxPointRadius)
32+
if (hit) {
33+
best = {
34+
datum: hit.node.datum,
35+
x: hit.node.x,
36+
y: hit.node.y,
37+
distance: hit.distance
3538
}
39+
pointHitConfirmed = true
3640
}
3741
}
3842

src/components/stream/PipelineStore.cache.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,9 @@ describe("scene rebuilds after config changes", () => {
265265
{ x: 3, y: 30, group: "B", region: "north" },
266266
])
267267
store.computeScene([400, 300])
268-
const groupColorB = store.resolveGroupColor("B")
268+
// Warm the cache for group "B" so the subsequent colorAccessor swap has
269+
// something stale to invalidate. Return value intentionally unused.
270+
store.resolveGroupColor("B")
269271

270272
// Switch the colorAccessor — categories change from {A, B} to {north, south},
271273
// so the cached map is wrong. This must invalidate.

src/components/stream/StreamGeoFrame.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,12 @@ const StreamGeoFrame = forwardRef<StreamGeoFrameHandle, StreamGeoFrameProps>(
862862
return () => {
863863
if (rafRef.current) { cancelAnimationFrame(rafRef.current); rafRef.current = 0 }
864864
tileCacheRef.current?.clear()
865+
// Drop any queued pointermove so flushPendingMove can't fire on unmount.
866+
pendingMoveCoordsRef.current = null
867+
if (moveRafRef.current !== 0) {
868+
cancelAnimationFrame(moveRafRef.current)
869+
moveRafRef.current = 0
870+
}
865871
}
866872
}, [scheduleRender])
867873

src/components/stream/StreamNetworkFrame.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,12 @@ const StreamNetworkFrame = forwardRef<
11831183
scheduleRender()
11841184
return () => {
11851185
if (rafRef.current) { cancelAnimationFrame(rafRef.current); rafRef.current = 0 }
1186+
// Drop any queued pointermove so flushPendingMove can't fire on unmount.
1187+
pendingMoveCoordsRef.current = null
1188+
if (moveRafRef.current !== 0) {
1189+
cancelAnimationFrame(moveRafRef.current)
1190+
moveRafRef.current = 0
1191+
}
11861192
}
11871193
}, [scheduleRender])
11881194

src/components/stream/StreamOrdinalFrame.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,12 @@ const StreamOrdinalFrame = forwardRef<StreamOrdinalFrameHandle, StreamOrdinalFra
840840
scheduleRender()
841841
return () => {
842842
if (rafRef.current) { cancelAnimationFrame(rafRef.current); rafRef.current = 0 }
843+
// Drop any queued pointermove so flushPendingMove can't fire on unmount.
844+
pendingMoveCoordsRef.current = null
845+
if (moveRafRef.current !== 0) {
846+
cancelAnimationFrame(moveRafRef.current)
847+
moveRafRef.current = 0
848+
}
843849
}
844850
}, [scheduleRender])
845851

src/components/stream/StreamXYFrame.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,12 @@ const StreamXYFrame = forwardRef<StreamXYFrameHandle, StreamXYFrameProps>(
10981098
scheduleRender()
10991099
return () => {
11001100
if (rafRef.current) { cancelAnimationFrame(rafRef.current); rafRef.current = 0 }
1101+
// Drop any queued pointermove so flushPendingMove can't fire on unmount.
1102+
pendingMoveCoordsRef.current = null
1103+
if (moveRafRef.current !== 0) {
1104+
cancelAnimationFrame(moveRafRef.current)
1105+
moveRafRef.current = 0
1106+
}
11011107
}
11021108
}, [scheduleRender])
11031109

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { describe, it, expect } from "vitest"
2+
import { quadtree } from "d3-quadtree"
3+
import { findHitPointInQuadtree } from "./quadtreeHitTest"
4+
5+
interface TestPoint { x: number; y: number; r: number; id: string }
6+
7+
function makeQt(points: TestPoint[]) {
8+
return quadtree<TestPoint>().x(p => p.x).y(p => p.y).addAll(points)
9+
}
10+
11+
describe("findHitPointInQuadtree", () => {
12+
it("returns the point under the cursor for a simple uniform-radius case", () => {
13+
const points: TestPoint[] = [
14+
{ x: 10, y: 10, r: 5, id: "a" },
15+
{ x: 50, y: 50, r: 5, id: "b" },
16+
{ x: 90, y: 90, r: 5, id: "c" }
17+
]
18+
const qt = makeQt(points)
19+
const hit = findHitPointInQuadtree(qt, 51, 51, 30, 5)
20+
expect(hit?.node.id).toBe("b")
21+
})
22+
23+
it("returns null when no point is within its own hit radius", () => {
24+
const points: TestPoint[] = [{ x: 10, y: 10, r: 5, id: "a" }]
25+
const qt = makeQt(points)
26+
// Cursor is 200px away — far outside any possible hit radius
27+
const hit = findHitPointInQuadtree(qt, 200, 200, 30, 5)
28+
expect(hit).toBeNull()
29+
})
30+
31+
it("prefers a farther large-radius point over a nearer small-radius miss (the core bug)", () => {
32+
// Without the visit-all-candidates approach, quadtree.find would return
33+
// the nearest point (`small`), reject it for distance > hitRadius, and
34+
// miss the bigger point behind it that *does* contain the cursor.
35+
// getHitRadius(r, 30) = max(r + 5, 12, 30).
36+
const points: TestPoint[] = [
37+
{ x: 10, y: 0, r: 1, id: "small" }, // dist 40, hitRadius 30 → MISS
38+
{ x: 110, y: 0, r: 80, id: "big" } // dist 60, hitRadius 85 → HIT
39+
]
40+
const qt = makeQt(points)
41+
const hit = findHitPointInQuadtree(qt, 50, 0, 30, 80)
42+
expect(hit?.node.id).toBe("big")
43+
})
44+
45+
it("among multiple hits returns the closest by distance", () => {
46+
const points: TestPoint[] = [
47+
{ x: 100, y: 100, r: 30, id: "far" }, // 30px away, hitRadius ≥ 35
48+
{ x: 108, y: 100, r: 20, id: "near" } // 22px away, hitRadius ≥ 25
49+
]
50+
const qt = makeQt(points)
51+
const hit = findHitPointInQuadtree(qt, 130, 100, 30, 30)
52+
expect(hit?.node.id).toBe("near")
53+
})
54+
55+
it("prunes subtrees outside the search radius (perf smoke — no bound violation)", () => {
56+
// Build a large scatter with only one point near the cursor. If pruning
57+
// works, we're still correct; if it doesn't, we're also correct but slow.
58+
// This test just confirms correctness at scale.
59+
const points: TestPoint[] = []
60+
for (let i = 0; i < 10000; i++) {
61+
points.push({ x: 1000 + (i % 100), y: 1000 + Math.floor(i / 100), r: 2, id: `bg${i}` })
62+
}
63+
points.push({ x: 50, y: 50, r: 5, id: "target" })
64+
const qt = makeQt(points)
65+
const hit = findHitPointInQuadtree(qt, 50, 50, 30, 5)
66+
expect(hit?.node.id).toBe("target")
67+
})
68+
69+
it("handles co-located points via the leaf linked list", () => {
70+
// d3-quadtree stores points at identical coordinates as a `.next` list.
71+
const points: TestPoint[] = [
72+
{ x: 50, y: 50, r: 5, id: "a" },
73+
{ x: 50, y: 50, r: 5, id: "b" },
74+
{ x: 50, y: 50, r: 5, id: "c" }
75+
]
76+
const qt = makeQt(points)
77+
const hit = findHitPointInQuadtree(qt, 50, 50, 30, 5)
78+
// Any of the three is an acceptable hit — they're all at dist=0.
79+
expect(hit?.node.id).toMatch(/^[abc]$/)
80+
expect(hit?.distance).toBe(0)
81+
})
82+
})
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import type { Quadtree } from "d3-quadtree"
2+
import { getHitRadius } from "./hitTestUtils"
3+
4+
export interface QuadtreeHit<T> {
5+
node: T
6+
distance: number
7+
}
8+
9+
/**
10+
* Find the closest point whose own hit radius contains the cursor.
11+
*
12+
* Uses `quadtree.visit()` to enumerate every candidate within the widened
13+
* search radius (maxDistance extended by the largest point radius in the
14+
* scene). `quadtree.find()` alone is insufficient because it returns the
15+
* nearest candidate by center-to-center distance — a farther point with a
16+
* much larger visual radius can still be a valid hit that `find()` would
17+
* hide.
18+
*
19+
* `T` must expose `x`, `y`, and `r` — the standard `PointSceneNode` shape.
20+
*/
21+
export function findHitPointInQuadtree<T extends { x: number; y: number; r: number }>(
22+
qt: Quadtree<T>,
23+
px: number,
24+
py: number,
25+
maxDistance: number,
26+
maxPointRadius: number
27+
): QuadtreeHit<T> | null {
28+
const searchRadius = Math.max(maxDistance, maxPointRadius + 5, 12)
29+
const xMin = px - searchRadius
30+
const xMax = px + searchRadius
31+
const yMin = py - searchRadius
32+
const yMax = py + searchRadius
33+
34+
let best: T | null = null
35+
let bestDist = Infinity
36+
37+
qt.visit((rawNode, x0, y0, x1, y1) => {
38+
// Prune subtrees whose bounding box is outside the search region.
39+
if (x0 > xMax || x1 < xMin || y0 > yMax || y1 < yMin) return true
40+
41+
// Leaf nodes in d3-quadtree are objects with `.data` and optional `.next`
42+
// (linked list for co-located points); internal nodes are array-like with
43+
// `.length === 4`. The absence of a numeric `length` distinguishes leaves.
44+
const node = rawNode as any
45+
if (!node.length) {
46+
let leaf: any = node
47+
do {
48+
const point = leaf.data as T
49+
const dx = point.x - px
50+
const dy = point.y - py
51+
const dist = Math.sqrt(dx * dx + dy * dy)
52+
const hitR = getHitRadius(point.r, maxDistance)
53+
if (dist <= hitR && dist < bestDist) {
54+
best = point
55+
bestDist = dist
56+
}
57+
leaf = leaf.next
58+
} while (leaf)
59+
}
60+
return false
61+
})
62+
63+
return best ? { node: best, distance: bestDist } : null
64+
}

0 commit comments

Comments
 (0)