From f8b61f1c7737bc4fd4e139bd060181f5b7e92e7e Mon Sep 17 00:00:00 2001 From: Ashley Anderson Date: Fri, 31 May 2024 21:53:50 -0400 Subject: [PATCH 1/5] Make hook dependencies errors --- .eslintrc.cjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 10be0d4d..6dd995f9 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -13,8 +13,7 @@ module.exports = { "react/react-in-jsx-scope": "off", "react/jsx-uses-react": "off", "react-hooks/rules-of-hooks": "error", // Enforce Rules of Hooks - // TODO: change exhaustive-deps to error - "react-hooks/exhaustive-deps": "warn", // Enforce effect dependencies + "react-hooks/exhaustive-deps": "error", // Enforce effect dependencies "camelcase": "error", "spaced-comment": "error", "semi": ["error", "always"], From 1c9431e34e4381baac813c481a8263df5a1fd964 Mon Sep 17 00:00:00 2001 From: Ashley Anderson Date: Fri, 31 May 2024 21:55:25 -0400 Subject: [PATCH 2/5] Move fetching logic into dispatcher, dispatch to set selection --- src/components/App.tsx | 50 +++++++++++++------------------------ src/hooks/usePointCanvas.ts | 45 +++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index d58fc9d4..d3fbf38b 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -147,44 +147,30 @@ export default function App() { console.debug("effect-selection"); const pointsID = canvas.points.id; const selectedPoints = canvas.selectedPoints; - if (!selectedPoints || !selectedPoints.has(pointsID)) return; - // keep track of which tracks we are adding to avoid duplicate fetching - const adding = new Set(); - - // this fetches the entire lineage for each track - const fetchAndAddTrack = async (pointID: number) => { - if (!trackManager) return; - const tracks = await trackManager.fetchTrackIDsForPoint(pointID); - // TODO: points actually only belong to one track, so can get rid of the outer loop - for (const t of tracks) { - const lineage = await trackManager.fetchLineageForTrack(t); - for (const l of lineage) { - if (adding.has(l) || canvas.tracks.has(l)) continue; - adding.add(l); - const [pos, ids] = await trackManager.fetchPointsForTrack(l); - // adding the track *in* the dispatcher creates issues with duplicate fetching - // but we refresh so the selected/loaded count is updated - canvas.addTrack(l, pos, ids); - dispatchCanvas({ type: ActionType.REFRESH }); - } - } - }; + if (!trackManager || !selectedPoints || !selectedPoints.has(pointsID)) { + setIsLoadingTracks(false); + return; + } dispatchCanvas({ type: ActionType.POINT_BRIGHTNESS, brightness: 0.8 }); - const selected = selectedPoints.get(pointsID) || []; dispatchCanvas({ type: ActionType.HIGHLIGHT_POINTS, points: selected }); - const maxPointsPerTimepoint = trackManager?.maxPointsPerTimepoint ?? 0; - + // keep track of which tracks we are adding to avoid duplicate fetching + const adding = new Set(); setIsLoadingTracks(true); - Promise.all(selected.map((p: number) => canvas.curTime * maxPointsPerTimepoint + p).map(fetchAndAddTrack)).then( - () => { - setIsLoadingTracks(false); - }, - ); - // TODO: add missing dependencies - }, [canvas.selectedPoints]); + selected.forEach((pointId) => { + dispatchCanvas({ + type: ActionType.ADD_TRACKS, + trackManager, + pointId, + adding, + // pass in the dispatcher to trigger a refresh, is this a hack? + dispatcher: dispatchCanvas, + }); + }); + dispatchCanvas({ type: ActionType.SELECTION, selection: new Map() }); + }, [canvas.points.id, canvas.selectedPoints, dispatchCanvas, trackManager]); // playback time points // TODO: this is basic and may drop frames diff --git a/src/hooks/usePointCanvas.ts b/src/hooks/usePointCanvas.ts index 093d02a9..0fc8a779 100644 --- a/src/hooks/usePointCanvas.ts +++ b/src/hooks/usePointCanvas.ts @@ -6,8 +6,10 @@ import { PointCanvas } from "@/lib/PointCanvas"; import { PointsCollection } from "@/lib/PointSelectionBox"; import { PointSelectionMode } from "@/lib/PointSelector"; import { ViewerState } from "@/lib/ViewerState"; +import { TrackManager } from "@/lib/TrackManager"; enum ActionType { + ADD_TRACKS = "ADD_TRACKS", AUTO_ROTATE = "AUTO_ROTATE", CAMERA_PROPERTIES = "CAMERA_PROPERTIES", CUR_TIME = "CUR_TIME", @@ -17,6 +19,7 @@ enum ActionType { POINTS_POSITIONS = "POINTS_POSITIONS", REFRESH = "REFRESH", REMOVE_ALL_TRACKS = "REMOVE_ALL_TRACKS", + SELECTION = "SELECTION", SELECTION_MODE = "SELECTION_MODE", SHOW_TRACKS = "SHOW_TRACKS", SHOW_TRACK_HIGHLIGHTS = "SHOW_TRACK_HIGHLIGHTS", @@ -24,6 +27,15 @@ enum ActionType { MIN_MAX_TIME = "MIN_MAX_TIME", } +interface AddTracks { + type: ActionType.ADD_TRACKS; + trackManager: TrackManager; + pointId: number; + adding: Set; + // callback to dispatch a refresh action from our async fetching + dispatcher: React.Dispatch; +} + interface AutoRotate { type: ActionType.AUTO_ROTATE; autoRotate: boolean; @@ -68,6 +80,11 @@ interface RemoveAllTracks { type: ActionType.REMOVE_ALL_TRACKS; } +interface Selection { + type: ActionType.SELECTION; + selection: PointsCollection; +} + interface SelectionMode { type: ActionType.SELECTION_MODE; selectionMode: PointSelectionMode; @@ -97,6 +114,7 @@ interface MinMaxTime { // setting up a tagged union for the actions type PointCanvasAction = + | AddTracks | AutoRotate | CameraProperties | CurTime @@ -106,6 +124,7 @@ type PointCanvasAction = | PointsPositions | Refresh | RemoveAllTracks + | Selection | SelectionMode | ShowTracks | ShowTrackHighlights @@ -118,6 +137,25 @@ function reducer(canvas: PointCanvas, action: PointCanvasAction): PointCanvas { switch (action.type) { case ActionType.REFRESH: break; + case ActionType.ADD_TRACKS: { + const { trackManager, pointId, adding, dispatcher } = action; + const fetchAndAddTrack = async (p: number) => { + const tracks = await trackManager.fetchTrackIDsForPoint(p); + // TODO: points actually only belong to one track, so can get rid of the outer loop + for (const t of tracks) { + const lineage = await trackManager.fetchLineageForTrack(t); + for (const l of lineage) { + if (adding.has(l) || canvas.tracks.has(l)) continue; + adding.add(l); + const [pos, ids] = await trackManager.fetchPointsForTrack(l); + canvas.addTrack(l, pos, ids); + dispatcher({ type: ActionType.REFRESH }); + } + } + }; + fetchAndAddTrack(canvas.curTime * trackManager.maxPointsPerTimepoint + pointId); + break; + } case ActionType.CAMERA_PROPERTIES: newCanvas.setCameraProperties(action.cameraPosition, action.cameraTarget); break; @@ -154,6 +192,9 @@ function reducer(canvas: PointCanvas, action: PointCanvasAction): PointCanvas { newCanvas.pointBrightness = 1.0; newCanvas.resetPointColors(); break; + case ActionType.SELECTION: + newCanvas.selector.selection = action.selection; + break; case ActionType.SELECTION_MODE: newCanvas.setSelectionMode(action.selectionMode); break; @@ -203,9 +244,9 @@ function usePointCanvas( // When the selection changes internally due to the user interacting with the canvas, // we need to trigger a react re-render. - canvas.selector.selectionChanged = useCallback((_selection: PointsCollection) => { + canvas.selector.selectionChanged = useCallback((selection: PointsCollection) => { console.debug("selectionChanged: refresh"); - dispatchCanvas({ type: ActionType.REFRESH }); + dispatchCanvas({ type: ActionType.SELECTION, selection: selection }); }, []); // set up the canvas when the div is available From ee85b2483685f3c48bbca3dd6b389c0458473b07 Mon Sep 17 00:00:00 2001 From: Ashley Anderson Date: Fri, 31 May 2024 22:09:15 -0400 Subject: [PATCH 3/5] Use newCanvas in reducer - not sure this makes a difference but feels more semantic --- src/hooks/usePointCanvas.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hooks/usePointCanvas.ts b/src/hooks/usePointCanvas.ts index 0fc8a779..8a647954 100644 --- a/src/hooks/usePointCanvas.ts +++ b/src/hooks/usePointCanvas.ts @@ -145,15 +145,15 @@ function reducer(canvas: PointCanvas, action: PointCanvasAction): PointCanvas { for (const t of tracks) { const lineage = await trackManager.fetchLineageForTrack(t); for (const l of lineage) { - if (adding.has(l) || canvas.tracks.has(l)) continue; + if (adding.has(l) || newCanvas.tracks.has(l)) continue; adding.add(l); const [pos, ids] = await trackManager.fetchPointsForTrack(l); - canvas.addTrack(l, pos, ids); + newCanvas.addTrack(l, pos, ids); dispatcher({ type: ActionType.REFRESH }); } } }; - fetchAndAddTrack(canvas.curTime * trackManager.maxPointsPerTimepoint + pointId); + fetchAndAddTrack(newCanvas.curTime * trackManager.maxPointsPerTimepoint + pointId); break; } case ActionType.CAMERA_PROPERTIES: From 1e5e457b1dd40e77b638f822a0e4e23d0fd2b4c0 Mon Sep 17 00:00:00 2001 From: Ashley Anderson Date: Mon, 3 Jun 2024 10:31:52 -0400 Subject: [PATCH 4/5] Trying to make track adding idempotent --- src/components/App.tsx | 21 +++++----- src/hooks/usePointCanvas.ts | 74 +++++++++++++++++++--------------- src/lib/BoxPointSelector.ts | 9 +++-- src/lib/PointCanvas.ts | 10 ++--- src/lib/PointSelectionBox.ts | 8 ++-- src/lib/PointSelector.ts | 20 ++++++--- src/lib/SpherePointSelector.ts | 8 ++-- 7 files changed, 82 insertions(+), 68 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index d3fbf38b..6400018d 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -32,7 +32,6 @@ export default function App() { const numTimes = trackManager?.numTimes ?? 0; // TODO: dataUrl can be stored in the TrackManager only const [dataUrl, setDataUrl] = useState(initialViewerState.dataUrl); - const [isLoadingTracks, setIsLoadingTracks] = useState(false); // PointCanvas is a Three.js canvas, updated via reducer const [canvas, dispatchCanvas, sceneDivRef] = usePointCanvas(initialViewerState); @@ -42,6 +41,7 @@ export default function App() { // this state is pure React const [playing, setPlaying] = useState(false); const [isLoadingPoints, setIsLoadingPoints] = useState(false); + const [numLoadingTracks, setNumLoadingTracks] = useState(0); // Manage shareable state that can persist across sessions. const copyShareableUrlToClipboard = () => { @@ -145,31 +145,28 @@ export default function App() { useEffect(() => { console.debug("effect-selection"); - const pointsID = canvas.points.id; const selectedPoints = canvas.selectedPoints; - if (!trackManager || !selectedPoints || !selectedPoints.has(pointsID)) { - setIsLoadingTracks(false); + if (!trackManager || !selectedPoints) { return; } dispatchCanvas({ type: ActionType.POINT_BRIGHTNESS, brightness: 0.8 }); - const selected = selectedPoints.get(pointsID) || []; - dispatchCanvas({ type: ActionType.HIGHLIGHT_POINTS, points: selected }); + const selected = Array.from(selectedPoints); + // TODO: HIGHLIGHT_POINTS still expects the point IDs within the current timeframe + // dispatchCanvas({ type: ActionType.HIGHLIGHT_POINTS, points: selected }); // keep track of which tracks we are adding to avoid duplicate fetching const adding = new Set(); - setIsLoadingTracks(true); selected.forEach((pointId) => { dispatchCanvas({ - type: ActionType.ADD_TRACKS, + type: ActionType.SYNC_TRACKS, trackManager, pointId, adding, - // pass in the dispatcher to trigger a refresh, is this a hack? - dispatcher: dispatchCanvas, + // pass in the setter to decrement the loading counter when we're done + setNumLoadingTracks, }); }); - dispatchCanvas({ type: ActionType.SELECTION, selection: new Map() }); }, [canvas.points.id, canvas.selectedPoints, dispatchCanvas, trackManager]); // playback time points @@ -290,7 +287,7 @@ export default function App() { > 0} initialCameraPosition={initialViewerState.cameraPosition} initialCameraTarget={initialViewerState.cameraTarget} /> diff --git a/src/hooks/usePointCanvas.ts b/src/hooks/usePointCanvas.ts index 8a647954..1260400d 100644 --- a/src/hooks/usePointCanvas.ts +++ b/src/hooks/usePointCanvas.ts @@ -1,15 +1,14 @@ -import { useCallback, useEffect, useReducer, useRef, Dispatch, RefObject } from "react"; +import { useCallback, useEffect, useReducer, useRef, Dispatch, RefObject, SetStateAction } from "react"; import { Vector3 } from "three"; import { PointCanvas } from "@/lib/PointCanvas"; -import { PointsCollection } from "@/lib/PointSelectionBox"; -import { PointSelectionMode } from "@/lib/PointSelector"; +import { PointSelection, PointSelectionMode } from "@/lib/PointSelector"; import { ViewerState } from "@/lib/ViewerState"; import { TrackManager } from "@/lib/TrackManager"; enum ActionType { - ADD_TRACKS = "ADD_TRACKS", + SYNC_TRACKS = "SYNC_TRACKS", AUTO_ROTATE = "AUTO_ROTATE", CAMERA_PROPERTIES = "CAMERA_PROPERTIES", CUR_TIME = "CUR_TIME", @@ -27,13 +26,13 @@ enum ActionType { MIN_MAX_TIME = "MIN_MAX_TIME", } -interface AddTracks { - type: ActionType.ADD_TRACKS; +interface SyncTracks { + type: ActionType.SYNC_TRACKS; trackManager: TrackManager; pointId: number; adding: Set; - // callback to dispatch a refresh action from our async fetching - dispatcher: React.Dispatch; + // callback to decrement the number of loading tracks + setNumLoadingTracks: Dispatch>; } interface AutoRotate { @@ -82,7 +81,7 @@ interface RemoveAllTracks { interface Selection { type: ActionType.SELECTION; - selection: PointsCollection; + selection: PointSelection; } interface SelectionMode { @@ -114,7 +113,7 @@ interface MinMaxTime { // setting up a tagged union for the actions type PointCanvasAction = - | AddTracks + | SyncTracks | AutoRotate | CameraProperties | CurTime @@ -137,25 +136,6 @@ function reducer(canvas: PointCanvas, action: PointCanvasAction): PointCanvas { switch (action.type) { case ActionType.REFRESH: break; - case ActionType.ADD_TRACKS: { - const { trackManager, pointId, adding, dispatcher } = action; - const fetchAndAddTrack = async (p: number) => { - const tracks = await trackManager.fetchTrackIDsForPoint(p); - // TODO: points actually only belong to one track, so can get rid of the outer loop - for (const t of tracks) { - const lineage = await trackManager.fetchLineageForTrack(t); - for (const l of lineage) { - if (adding.has(l) || newCanvas.tracks.has(l)) continue; - adding.add(l); - const [pos, ids] = await trackManager.fetchPointsForTrack(l); - newCanvas.addTrack(l, pos, ids); - dispatcher({ type: ActionType.REFRESH }); - } - } - }; - fetchAndAddTrack(newCanvas.curTime * trackManager.maxPointsPerTimepoint + pointId); - break; - } case ActionType.CAMERA_PROPERTIES: newCanvas.setCameraProperties(action.cameraPosition, action.cameraTarget); break; @@ -209,6 +189,30 @@ function reducer(canvas: PointCanvas, action: PointCanvasAction): PointCanvas { case ActionType.SIZE: newCanvas.setSize(action.width, action.height); break; + case ActionType.SYNC_TRACKS: { + const { trackManager, pointId, adding, setNumLoadingTracks } = action; + const fetchAndAddTrack = async (p: number) => { + setNumLoadingTracks((n) => n + 1); + const tracks = await trackManager.fetchTrackIDsForPoint(p); + // TODO: points actually only belong to one track, so can get rid of the outer loop + for (const t of tracks) { + // skip this if we already fetched the lineage for this track + if (newCanvas.rootTracks.has(t)) continue; + newCanvas.rootTracks.add(t); + const lineage = await trackManager.fetchLineageForTrack(t); + for (const l of lineage) { + if (adding.has(l) || newCanvas.tracks.has(l)) continue; + adding.add(l); + const [pos, ids] = await trackManager.fetchPointsForTrack(l); + newCanvas.addTrack(l, pos, ids); + } + } + }; + fetchAndAddTrack(pointId).then(() => { + setNumLoadingTracks((n) => n - 1); + }); + break; + } case ActionType.MIN_MAX_TIME: newCanvas.minTime = action.minTime; newCanvas.maxTime = action.maxTime; @@ -244,10 +248,14 @@ function usePointCanvas( // When the selection changes internally due to the user interacting with the canvas, // we need to trigger a react re-render. - canvas.selector.selectionChanged = useCallback((selection: PointsCollection) => { - console.debug("selectionChanged: refresh"); - dispatchCanvas({ type: ActionType.SELECTION, selection: selection }); - }, []); + canvas.selector.selectionChanged = useCallback( + (selection: PointSelection) => { + console.debug("selectionChanged:", selection); + const newSelection = new Set([...selection].map((p) => canvas.curTime * canvas.maxPointsPerTimepoint + p)); + dispatchCanvas({ type: ActionType.SELECTION, selection: newSelection }); + }, + [canvas.curTime, canvas.maxPointsPerTimepoint], + ); // set up the canvas when the div is available // this is an effect because: diff --git a/src/lib/BoxPointSelector.ts b/src/lib/BoxPointSelector.ts index 26cb6192..eb05dae5 100644 --- a/src/lib/BoxPointSelector.ts +++ b/src/lib/BoxPointSelector.ts @@ -1,8 +1,8 @@ -import { PerspectiveCamera, Scene, WebGLRenderer } from "three"; +import { PerspectiveCamera, Points, Scene, WebGLRenderer } from "three"; import { OrbitControls } from "three/addons/controls/OrbitControls.js"; import { SelectionHelper } from "three/addons/interactive/SelectionHelper.js"; -import { PointSelectionBox, PointsCollection } from "@/lib/PointSelectionBox"; +import { PointsCollection, PointSelectionBox } from "@/lib/PointSelectionBox"; import { SelectionChanged } from "@/lib/PointSelector"; // Selection with a 2D rectangle to make a 3D frustum. @@ -11,6 +11,7 @@ export class BoxPointSelector { readonly controls: OrbitControls; readonly box: PointSelectionBox; readonly helper: SelectionHelper; + readonly points: Points; readonly selectionChanged: SelectionChanged; // True if this should not perform selection, false otherwise. @@ -23,6 +24,7 @@ export class BoxPointSelector { renderer: WebGLRenderer, camera: PerspectiveCamera, controls: OrbitControls, + points: Points, selectionChanged: SelectionChanged, ) { this.renderer = renderer; @@ -30,6 +32,7 @@ export class BoxPointSelector { this.helper = new SelectionHelper(renderer, "selectBox"); this.helper.enabled = false; this.box = new PointSelectionBox(camera, scene); + this.points = points; this.selectionChanged = selectionChanged; } @@ -52,7 +55,7 @@ export class BoxPointSelector { setSelectedPoints(selectedPoints: PointsCollection) { console.debug("BoxPointSelector.setSelectedPoints: ", selectedPoints); this.box.collection = selectedPoints; - this.selectionChanged(selectedPoints); + this.selectionChanged(selectedPoints.get(this.points.id) ?? new Set()); } pointerUp(_event: MouseEvent) { diff --git a/src/lib/PointCanvas.ts b/src/lib/PointCanvas.ts index 6633eb67..8274bd74 100644 --- a/src/lib/PointCanvas.ts +++ b/src/lib/PointCanvas.ts @@ -22,8 +22,7 @@ import { OutputPass } from "three/addons/postprocessing/OutputPass.js"; import { UnrealBloomPass } from "three/addons/postprocessing/UnrealBloomPass.js"; import { Track } from "@/lib/three/Track"; -import { PointSelector, PointSelectionMode } from "@/lib/PointSelector"; -import { PointsCollection } from "@/lib/PointSelectionBox"; +import { PointSelection, PointSelector, PointSelectionMode } from "@/lib/PointSelector"; type Tracks = Map; @@ -38,6 +37,8 @@ export class PointCanvas { readonly selector: PointSelector; readonly tracks: Tracks = new Map(); + // set of track IDs that have had their lineage fetched + readonly rootTracks: Set = new Set(); showTracks = true; showTrackHighlights = true; @@ -48,8 +49,7 @@ export class PointCanvas { // this is used to initialize the points geometry, and kept to initialize the // tracks but could be pulled from the points geometry when adding tracks - // private here to consolidate external access via `TrackManager` instead - private maxPointsPerTimepoint = 0; + maxPointsPerTimepoint = 0; constructor(width: number, height: number) { this.scene = new Scene(); @@ -107,7 +107,7 @@ export class PointCanvas { return newCanvas as PointCanvas; } - get selectedPoints(): PointsCollection { + get selectedPoints(): PointSelection { return this.selector.selection; } diff --git a/src/lib/PointSelectionBox.ts b/src/lib/PointSelectionBox.ts index 56ffa988..ac44875f 100644 --- a/src/lib/PointSelectionBox.ts +++ b/src/lib/PointSelectionBox.ts @@ -24,7 +24,8 @@ const _vectemp1 = new Vector3(); const _vectemp2 = new Vector3(); const _vectemp3 = new Vector3(); -type PointsCollection = Map; +// map of id: [indices] +export type PointsCollection = Map>; class PointSelectionBox { camera: OrthographicCamera | PerspectiveCamera; @@ -163,9 +164,9 @@ class PointSelectionBox { if (frustum.containsPoint(_vec3)) { const objectCollection = this.collection.get(object.id); if (!objectCollection) { - this.collection.set(object.id, [i]); + this.collection.set(object.id, new Set([i])); } else { - objectCollection.push(i); + objectCollection.add(i); } } } @@ -192,5 +193,4 @@ function isPoints(obj: Object3D): obj is Points { return Boolean(obj && "isPoints" in obj && obj.isPoints); } -export type { PointsCollection }; export { PointSelectionBox }; diff --git a/src/lib/PointSelector.ts b/src/lib/PointSelector.ts index 4feb1715..5efc25d7 100644 --- a/src/lib/PointSelector.ts +++ b/src/lib/PointSelector.ts @@ -1,10 +1,11 @@ import { PerspectiveCamera, Points, Scene, WebGLRenderer } from "three"; import { OrbitControls } from "three/addons/controls/OrbitControls.js"; -import { PointsCollection } from "@/lib/PointSelectionBox"; import { BoxPointSelector } from "./BoxPointSelector"; import { SpherePointSelector } from "./SpherePointSelector"; +export type PointSelection = Set; + export enum PointSelectionMode { BOX = "BOX", SPHERICAL_CURSOR = "SPHERICAL_CURSOR", @@ -22,7 +23,7 @@ interface PointSelectorInterface { dispose(): void; } -export type SelectionChanged = (selection: PointsCollection) => void; +export type SelectionChanged = (selection: PointSelection) => void; // this is a separate class to keep the point selection logic separate from the rendering logic in // the PointCanvas class this fixes some issues with callbacks and event listeners binding to @@ -34,9 +35,9 @@ export class PointSelector { readonly sphereSelector: SpherePointSelector; selectionMode: PointSelectionMode = PointSelectionMode.BOX; - selection: PointsCollection = new Map(); + selection: PointSelection = new Set(); // To optionally notify external observers about changes to the current selection. - selectionChanged: SelectionChanged = (_selection: PointsCollection) => {}; + selectionChanged: SelectionChanged = (_selection: PointSelection) => {}; constructor( scene: Scene, @@ -45,7 +46,14 @@ export class PointSelector { controls: OrbitControls, points: Points, ) { - this.boxSelector = new BoxPointSelector(scene, renderer, camera, controls, this.setSelectedPoints.bind(this)); + this.boxSelector = new BoxPointSelector( + scene, + renderer, + camera, + controls, + points, + this.setSelectedPoints.bind(this), + ); this.sphereSelector = new SpherePointSelector( scene, renderer, @@ -84,7 +92,7 @@ export class PointSelector { return this.selectionMode === PointSelectionMode.BOX ? this.boxSelector : this.sphereSelector; } - setSelectedPoints(selection: PointsCollection) { + setSelectedPoints(selection: PointSelection) { console.debug("PointSelector.setSelectedPoints:", selection); this.selection = selection; this.selectionChanged(selection); diff --git a/src/lib/SpherePointSelector.ts b/src/lib/SpherePointSelector.ts index aad6e923..67c761d2 100644 --- a/src/lib/SpherePointSelector.ts +++ b/src/lib/SpherePointSelector.ts @@ -14,9 +14,7 @@ import { import { OrbitControls } from "three/addons/controls/OrbitControls.js"; import { TransformControls } from "three/examples/jsm/Addons.js"; -import { PointsCollection } from "@/lib/PointSelectionBox"; - -import { SelectionChanged } from "@/lib/PointSelector"; +import { PointSelection, SelectionChanged } from "@/lib/PointSelector"; // Selecting with a sphere, with optional transform controls. export class SpherePointSelector { @@ -175,8 +173,8 @@ export class SpherePointSelector { selected.push(i); } } - const points: PointsCollection = new Map(); - points.set(this.points.id, selected); + const points: PointSelection = new Set(); + selected.forEach(points.add, points); console.log("selected points:", selected); this.selectionChanged(points); } From f1aab725bf703b0834ced6614494be41c37ebd84 Mon Sep 17 00:00:00 2001 From: Ashley Anderson Date: Mon, 3 Jun 2024 14:01:05 -0400 Subject: [PATCH 5/5] Fix highlighting points on selection --- src/components/App.tsx | 3 +-- src/hooks/usePointCanvas.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 6400018d..3c0c97db 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -152,8 +152,7 @@ export default function App() { dispatchCanvas({ type: ActionType.POINT_BRIGHTNESS, brightness: 0.8 }); const selected = Array.from(selectedPoints); - // TODO: HIGHLIGHT_POINTS still expects the point IDs within the current timeframe - // dispatchCanvas({ type: ActionType.HIGHLIGHT_POINTS, points: selected }); + dispatchCanvas({ type: ActionType.HIGHLIGHT_POINTS, points: selected }); // keep track of which tracks we are adding to avoid duplicate fetching const adding = new Set(); diff --git a/src/hooks/usePointCanvas.ts b/src/hooks/usePointCanvas.ts index 1260400d..e41b7484 100644 --- a/src/hooks/usePointCanvas.ts +++ b/src/hooks/usePointCanvas.ts @@ -154,7 +154,7 @@ function reducer(canvas: PointCanvas, action: PointCanvasAction): PointCanvas { newCanvas.controls.autoRotate = action.autoRotate; break; case ActionType.HIGHLIGHT_POINTS: - newCanvas.highlightPoints(action.points); + newCanvas.highlightPoints([...action.points].map((p) => p % newCanvas.maxPointsPerTimepoint)); break; case ActionType.INIT_POINTS_GEOMETRY: newCanvas.initPointsGeometry(action.maxPointsPerTimepoint);