Skip to content

Commit 731ce39

Browse files
PythonFZclaudepre-commit-ci[bot]
authored
refactor: extract usePathtracingMesh hook and optimize Zustand selectors (#865)
* refactor: extract usePathtracingMesh hook and optimize Zustand selectors - Create usePathtracingMesh hook to encapsulate pathtracing mesh conversion logic, eliminating ~40 lines of duplicate code from 6 geometry components - Refactor Particles, Bonds, Arrow, Box, Plane, Shape to use the new hook - Fix type error in Bonds.tsx (hoverMeshRef was Mesh instead of InstancedMesh) - Optimize Zustand store access by using individual selectors instead of object destructuring to prevent unnecessary re-renders - Improve convertInstancedMesh utility with better bounding box computation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * use larger array if needed * something finally working * fix: add missing effect dependencies and cleanup unused imports - Add updateMergedMesh to useEffect dependencies in all geometry components - Memoize fullData in Cell.tsx for consistency with other components - Remove unused BufferGeometryUtils import from convertInstancedMesh.ts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * address comments --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent f237fba commit 731ce39

24 files changed

Lines changed: 628 additions & 509 deletions

app/src/components/Canvas.tsx

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,27 @@ const PATHTRACING_GEOMETRY_COMPONENTS = {
5151
Box: Box,
5252
Plane: Plane,
5353
Shape: Shape,
54+
Curve: Curve,
5455
} as const;
5556

5657
/**
5758
* Components that accept geometryKey but NOT pathtracingEnabled.
5859
*/
5960
const SIMPLE_GEOMETRY_COMPONENTS = {
60-
Curve: Curve,
6161
Camera: Camera,
6262
} as const;
6363

6464
/**
65-
* Components that only accept data prop (no geometryKey).
65+
* Components that only accept data prop AND pathtracingEnabled.
6666
*/
67-
const DATA_ONLY_GEOMETRY_COMPONENTS = {
67+
const DATA_PATHTRACING_GEOMETRY_COMPONENTS = {
6868
Cell: Cell,
69+
} as const;
70+
71+
/**
72+
* Components that only accept data prop (no geometryKey, no pathtracingEnabled).
73+
*/
74+
const DATA_ONLY_GEOMETRY_COMPONENTS = {
6975
Floor: Floor,
7076
} as const;
7177

@@ -321,6 +327,22 @@ function MyScene() {
321327
);
322328
}
323329

330+
// Check data + pathtracing components (no geometryKey, but pathtracingEnabled)
331+
if (type in DATA_PATHTRACING_GEOMETRY_COMPONENTS) {
332+
const Component =
333+
DATA_PATHTRACING_GEOMETRY_COMPONENTS[
334+
type as keyof typeof DATA_PATHTRACING_GEOMETRY_COMPONENTS
335+
];
336+
return (
337+
<GeometryErrorBoundary key={name} geometryKey={name}>
338+
<Component
339+
data={config.data}
340+
pathtracingEnabled={pathtracingEnabled}
341+
/>
342+
</GeometryErrorBoundary>
343+
);
344+
}
345+
324346
// Check data-only components (no geometryKey)
325347
if (type in DATA_ONLY_GEOMETRY_COMPONENTS) {
326348
const Component =
Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import { useEffect, useRef } from "react";
22
import { usePathtracer } from "@react-three/gpu-pathtracer";
3+
import { useFrame } from "@react-three/fiber";
34
import { useAppStore } from "../store";
45
import type { PathTracing } from "../types/room-config";
56

67
/**
78
* Component that watches for pathtracing update requests and calls
89
* pathtracer.update() to rebuild the BVH acceleration structure.
910
*
10-
* This is needed when:
11-
* - Scene changes dynamically (e.g., particles switching from instanced to individual meshes)
12-
* - Current frame changes (particles move, arrows change, etc.)
13-
* - Pathtracing settings change
11+
* Uses useFrame to ensure update() is called after R3F has finished
12+
* processing all scene updates in the render loop.
1413
*/
1514
export function PathtracingUpdater({ settings }: { settings: PathTracing }) {
1615
const pathtracingNeedsUpdate = useAppStore(
@@ -25,38 +24,32 @@ export function PathtracingUpdater({ settings }: { settings: PathTracing }) {
2524

2625
// Track if this is the first render to avoid calling update on mount
2726
const isFirstRender = useRef(true);
27+
// Pending update flag - processed in useFrame to ensure scene is stable
28+
const pendingUpdate = useRef(false);
2829

29-
// Handle manual update requests (e.g., when switching to individual meshes)
30+
// Capture manual update requests
3031
useEffect(() => {
31-
if (pathtracingNeedsUpdate && update) {
32-
update();
32+
if (pathtracingNeedsUpdate) {
33+
pendingUpdate.current = true;
3334
clearPathtracingUpdate();
3435
}
35-
}, [pathtracingNeedsUpdate, update, clearPathtracingUpdate]);
36+
}, [pathtracingNeedsUpdate, clearPathtracingUpdate]);
3637

37-
// Handle automatic updates when frame changes
38+
// Capture frame change updates
3839
useEffect(() => {
39-
// Skip the first render
4040
if (isFirstRender.current) {
4141
isFirstRender.current = false;
4242
return;
4343
}
44+
pendingUpdate.current = true;
45+
}, [currentFrame]);
4446

45-
if (update) {
46-
update();
47-
}
48-
}, [currentFrame, update]);
49-
50-
// Handle automatic updates when pathtracing settings change
47+
// Capture settings change updates
5148
useEffect(() => {
52-
// Skip the first render
5349
if (isFirstRender.current) {
5450
return;
5551
}
56-
57-
if (update) {
58-
update();
59-
}
52+
pendingUpdate.current = true;
6053
}, [
6154
settings.samples,
6255
settings.min_samples,
@@ -66,9 +59,15 @@ export function PathtracingUpdater({ settings }: { settings: PathTracing }) {
6659
settings.environment_intensity,
6760
settings.environment_blur,
6861
settings.environment_background,
69-
update,
7062
]);
7163

72-
// This component doesn't render anything
64+
// Execute update in useFrame - guarantees R3F has processed scene updates
65+
useFrame(() => {
66+
if (pendingUpdate.current && update) {
67+
update();
68+
pendingUpdate.current = false;
69+
}
70+
});
71+
7372
return null;
7473
}

app/src/components/ProgressBar.tsx

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,27 @@ const FrameProgressBar = () => {
2727
const sliderContainerRef = useRef<HTMLDivElement>(null);
2828
const [sliderWidth, setSliderWidth] = useState(0);
2929

30-
const {
31-
currentFrame,
32-
setCurrentFrame,
33-
frameCount,
34-
isConnected,
35-
isLoading,
36-
skipFrames,
37-
setSkipFrames,
38-
frame_selection,
39-
bookmarks,
40-
addBookmark,
41-
deleteBookmark,
42-
frameSelectionEnabled,
43-
setFrameSelectionEnabled,
44-
synchronizedMode,
45-
setSynchronizedMode,
46-
getIsFetching,
47-
playing,
48-
setPlaying,
49-
} = useAppStore();
30+
const currentFrame = useAppStore((state) => state.currentFrame);
31+
const frameCount = useAppStore((state) => state.frameCount);
32+
const isConnected = useAppStore((state) => state.isConnected);
33+
const isLoading = useAppStore((state) => state.isLoading);
34+
const skipFrames = useAppStore((state) => state.skipFrames);
35+
const setSkipFrames = useAppStore((state) => state.setSkipFrames);
36+
const frame_selection = useAppStore((state) => state.frame_selection);
37+
const bookmarks = useAppStore((state) => state.bookmarks);
38+
const addBookmark = useAppStore((state) => state.addBookmark);
39+
const deleteBookmark = useAppStore((state) => state.deleteBookmark);
40+
const frameSelectionEnabled = useAppStore(
41+
(state) => state.frameSelectionEnabled,
42+
);
43+
const setFrameSelectionEnabled = useAppStore(
44+
(state) => state.setFrameSelectionEnabled,
45+
);
46+
const synchronizedMode = useAppStore((state) => state.synchronizedMode);
47+
const setSynchronizedMode = useAppStore((state) => state.setSynchronizedMode);
48+
const getIsFetching = useAppStore((state) => state.getIsFetching);
49+
const playing = useAppStore((state) => state.playing);
50+
const setPlaying = useAppStore((state) => state.setPlaying);
5051

5152
const { setStep, remoteLocked } = useStepControl();
5253

app/src/components/SelectionGroupsPanel.tsx

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,16 @@ interface SelectionGroupRow {
4141
}
4242

4343
export default function SelectionGroupsPanel() {
44-
const {
45-
roomId,
46-
selections,
47-
selectionGroups,
48-
activeSelectionGroup,
49-
updateSelectionForGeometry,
50-
showSnackbar,
51-
} = useAppStore();
44+
const roomId = useAppStore((state) => state.roomId);
45+
const selections = useAppStore((state) => state.selections);
46+
const selectionGroups = useAppStore((state) => state.selectionGroups);
47+
const activeSelectionGroup = useAppStore(
48+
(state) => state.activeSelectionGroup,
49+
);
50+
const updateSelectionForGeometry = useAppStore(
51+
(state) => state.updateSelectionForGeometry,
52+
);
53+
const showSnackbar = useAppStore((state) => state.showSnackbar);
5254

5355
// Local state
5456
const [currentGroupName, setCurrentGroupName] = useState("");

app/src/components/three/Arrow.tsx

Lines changed: 26 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ import {
3030
_matrix2,
3131
_color,
3232
} from "../../utils/threeObjectPools";
33-
import {
34-
convertInstancedMeshToMerged,
35-
disposeMesh,
36-
} from "../../utils/convertInstancedMesh";
33+
import { usePathtracingMesh } from "../../hooks/usePathtracingMesh";
3734
import { getGeometryWithDefaults } from "../../utils/geometryDefaults";
3835
import { useFrameKeys } from "../../hooks/useSchemas";
3936

@@ -105,10 +102,9 @@ export default function Arrow({
105102
const geometryDefaults = useAppStore((state) => state.geometryDefaults);
106103

107104
// Merge with defaults from Pydantic (single source of truth)
108-
const fullData = getGeometryWithDefaults<ArrowData>(
109-
data,
110-
"Arrow",
111-
geometryDefaults,
105+
const fullData = useMemo(
106+
() => getGeometryWithDefaults<ArrowData>(data, "Arrow", geometryDefaults),
107+
[data, geometryDefaults],
112108
);
113109
const {
114110
position: positionProp,
@@ -126,9 +122,17 @@ export default function Arrow({
126122
const mainMeshRef = useRef<THREE.InstancedMesh | null>(null);
127123
const selectionMeshRef = useRef<THREE.InstancedMesh | null>(null);
128124
const hoverMeshRef = useRef<THREE.Mesh | null>(null);
129-
const mergedMeshRef = useRef<THREE.Mesh | null>(null);
125+
const parentGroupRef = useRef<THREE.Group | null>(null);
130126
const [instanceCount, setInstanceCount] = useState(0);
131127

128+
// Pathtracing: convert instanced mesh to merged mesh
129+
// Uses refs and manual scene management for precise timing control
130+
const updateMergedMesh = usePathtracingMesh(
131+
parentGroupRef,
132+
mainMeshRef,
133+
pathtracingEnabled,
134+
);
135+
132136
// Use individual selectors to prevent unnecessary re-renders
133137
const currentFrame = useAppStore((state) => state.currentFrame);
134138
const frameCount = useAppStore((state) => state.frameCount);
@@ -151,9 +155,6 @@ export default function Arrow({
151155
const removeGeometryFetching = useAppStore(
152156
(state) => state.removeGeometryFetching,
153157
);
154-
const requestPathtracingUpdate = useAppStore(
155-
(state) => state.requestPathtracingUpdate,
156-
);
157158

158159
// Fetch frame keys to check if required data is available
159160
const { data: frameKeysData, isLoading: isLoadingKeys } = useFrameKeys(
@@ -443,6 +444,11 @@ export default function Arrow({
443444
mainMesh.computeBoundingBox();
444445
mainMesh.computeBoundingSphere();
445446

447+
// Update pathtracing mesh if enabled
448+
if (pathtracingEnabled) {
449+
updateMergedMesh(geometry);
450+
}
451+
446452
// --- Selection Mesh Update ---
447453
if (selecting.enabled && selectionMeshRef.current) {
448454
const selectionMesh = selectionMeshRef.current;
@@ -491,7 +497,6 @@ export default function Arrow({
491497
if (instanceCount !== 0) setInstanceCount(0);
492498
}
493499
}, [
494-
data, // Add data to dependencies to ensure updates trigger
495500
frameCount, // Watch frameCount to clear arrows when it becomes 0
496501
isFetching,
497502
positionData,
@@ -508,6 +513,12 @@ export default function Arrow({
508513
validSelectedIndices,
509514
selecting,
510515
geometryKey,
516+
pathtracingEnabled,
517+
resolution,
518+
material,
519+
opacity,
520+
data,
521+
updateMergedMesh,
511522
]);
512523

513524
// Separate effect for hover mesh updates - doesn't trigger data reprocessing
@@ -544,45 +555,6 @@ export default function Arrow({
544555
}
545556
}, [hoveredGeometryInstance, instanceCount, hovering, geometryKey]);
546557

547-
// Convert instanced mesh to merged mesh for path tracing
548-
useEffect(() => {
549-
if (!pathtracingEnabled) {
550-
// Clean up merged mesh when pathtracing disabled
551-
if (mergedMeshRef.current) {
552-
disposeMesh(mergedMeshRef.current);
553-
mergedMeshRef.current = null;
554-
}
555-
return;
556-
}
557-
558-
if (!mainMeshRef.current || instanceCount === 0) return;
559-
560-
// Dispose old merged mesh if it exists
561-
if (mergedMeshRef.current) {
562-
disposeMesh(mergedMeshRef.current);
563-
}
564-
565-
// Convert instanced mesh to single merged mesh with vertex colors
566-
const mergedMesh = convertInstancedMeshToMerged(mainMeshRef.current);
567-
mergedMeshRef.current = mergedMesh;
568-
569-
// Request pathtracing update
570-
requestPathtracingUpdate();
571-
572-
// Cleanup on unmount or when dependencies change
573-
return () => {
574-
if (mergedMeshRef.current) {
575-
disposeMesh(mergedMeshRef.current);
576-
mergedMeshRef.current = null;
577-
}
578-
};
579-
}, [
580-
pathtracingEnabled,
581-
instanceCount,
582-
geometryKey,
583-
requestPathtracingUpdate,
584-
]);
585-
586558
// Create the base geometry, recreate when resolution changes
587559
const geometry = useMemo(() => createArrowMesh(resolution), [resolution]);
588560

@@ -631,8 +603,9 @@ export default function Arrow({
631603
}
632604

633605
return (
634-
<group>
606+
<group ref={parentGroupRef}>
635607
{/* Main instanced mesh - visible when NOT pathtracing */}
608+
{/* Merged mesh for pathtracing is added to this group imperatively via usePathtracingMesh */}
636609
<instancedMesh
637610
key={instanceCount}
638611
ref={mainMeshRef}
@@ -688,11 +661,6 @@ export default function Arrow({
688661
/>
689662
</mesh>
690663
)}
691-
692-
{/* Merged mesh - visible when pathtracing */}
693-
{pathtracingEnabled && mergedMeshRef.current && (
694-
<primitive object={mergedMeshRef.current} />
695-
)}
696664
</group>
697665
);
698666
}

0 commit comments

Comments
 (0)