Feature/opti editor#26
Conversation
- Implemented a WebGLErrorBoundary to catch WebGL context creation failures. - Added a function to check WebGL availability and display a fallback UI if unavailable. - Updated MainCanvas to utilize the new error boundary and fallback mechanism for improved user experience.
…rove performance - Introduced DragPreviewGate to conditionally mount DragPreview only during active dragging, reducing unnecessary rendering. - Refactored useDragPreview to build a mesh cache once per drag start, enhancing performance by avoiding repeated traversals of the scene. - Updated ModelViewer to apply color changes more efficiently by mutating shader uniforms instead of cloning the scene on every update. - Improved overall drag-and-drop experience with performance optimizations and reduced resource usage.
There was a problem hiding this comment.
Code Review
This pull request optimizes the 3D editor's performance by caching scene meshes and reusing Three.js objects during drag previews, preventing redundant scene cloning on color changes in ModelViewer, and conditionally mounting the drag preview to avoid idle frame loops. It also adds a WebGL availability check and error boundary fallback. The reviewer feedback highlights critical improvements: disposing of custom materials in ModelViewer to prevent GPU memory leaks, moving the WebGL check to a useEffect hook to avoid SSR hydration mismatches, and configuring the canvas to prefer high-performance GPUs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| useLayoutEffect(() => { | ||
| applyMaterials(materialsRef.current, color, coloredTexture); | ||
| }, [color, coloredTexture, clonedScene]); |
There was a problem hiding this comment.
The custom materials (shaderMat and plainMat) created inside useMemo are never disposed of when the component unmounts or when clonedScene is re-created. In Three.js, materials must be manually disposed of to prevent GPU memory leaks, which can eventually crash the browser tab. Add a useEffect to handle the disposal of these materials on cleanup.
useLayoutEffect(() => {\n applyMaterials(materialsRef.current, color, coloredTexture);\n }, [color, coloredTexture, clonedScene]);\n\n useEffect(() => {\n const currentMaterials = materialsRef.current;\n return () => {\n for (const entry of currentMaterials) {\n if (entry.shaderMat) {\n entry.shaderMat.dispose();\n }\n entry.plainMat.dispose();\n }\n };\n }, [clonedScene]);
| } | ||
|
|
||
| const MainCanvas = ({ wallModels }: { wallModels: string[] }) => { | ||
| const [webGLAvailable] = useState<boolean>(() => isWebGLAvailable()); |
There was a problem hiding this comment.
Initializing webGLAvailable synchronously during render using isWebGLAvailable() can cause hydration mismatches if Server-Side Rendering (SSR) is used, as document is undefined on the server and will force the fallback UI to be rendered. To ensure SSR compatibility and avoid hydration mismatches, initialize the state to true and perform the WebGL availability check inside a useEffect hook after mount.
const [webGLAvailable, setWebGLAvailable] = useState<boolean>(true);\n\n useEffect(() => {\n setWebGLAvailable(isWebGLAvailable());\n }, []);
| gl={{ | ||
| failIfMajorPerformanceCaveat: false, | ||
| powerPreference: "default", | ||
| }} |
There was a problem hiding this comment.
For a 3D editor, it is highly recommended to set powerPreference to 'high-performance' in the WebGL context configuration. This hints to multi-GPU systems (like laptops with integrated and discrete GPUs) to use the high-performance discrete GPU, ensuring smoother frame rates and better responsiveness.
gl={{\n failIfMajorPerformanceCaveat: false,\n powerPreference: \"high-performance\",\n }}
There was a problem hiding this comment.
Pull request overview
This PR focuses on performance and resiliency improvements in the 3D editor (drag preview + model rendering) and updates the frontend lockfile accordingly.
Changes:
- Optimizes drag preview raycasting by reusing Three.js objects, caching the scene mesh list per drag-start, and caching the canvas rect.
- Optimizes
ModelViewerby cloning the GLTF scene only once per load and updating materials via uniform/property mutation (plusmemoto reduce rerenders). - Adds WebGL availability checks + an error boundary fallback so the rest of the UI remains usable when WebGL context creation fails.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| frontend/src/features/editor/components/useDragPreview.ts | Reduces per-frame allocations and avoids per-frame scene.traverse() while dragging. |
| frontend/src/features/editor/components/ModelViewer.tsx | Avoids repeated scene cloning/material recreation; introduces cached material entries + memoization. |
| frontend/src/features/editor/components/MainCanvas.tsx | Adds WebGL capability checks/error boundary + gates DragPreview mounting to active drags. |
| frontend/package-lock.json | Dependency/lockfile updates (React, R3F, Tailwind, PostHog, etc.). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const webGLFallback = ( | ||
| <div className="relative w-full h-full flex items-center justify-center bg-surface"> | ||
| <div className="text-center max-w-md px-8 py-10 rounded-2xl bg-surface-low shadow-[0_8px_32px_0_rgba(0,0,0,0.4)]"> | ||
| <span className="material-symbols-outlined text-5xl text-on-surface-variant mb-4 block">broken_image</span> | ||
| <h2 className="text-lg font-semibold text-on-surface mb-2">3D viewer unavailable</h2> | ||
| <p className="text-sm text-on-surface-variant mb-4"> | ||
| Your browser could not create a WebGL context, which is required to display the 3D wall editor. | ||
| This is usually caused by disabled hardware acceleration or an incompatible graphics driver. | ||
| </p> | ||
| <p className="text-xs text-on-surface-variant opacity-60 mb-6"> | ||
| Try enabling hardware acceleration in your browser settings | ||
| (<strong>Settings → System → Use hardware acceleration when available</strong>) | ||
| and reload the page. | ||
| </p> | ||
| <button | ||
| onClick={() => window.location.reload()} | ||
| className="px-4 py-2 rounded-lg bg-surface-high text-on-surface text-sm hover:opacity-80 transition-opacity" | ||
| > | ||
| Reload page | ||
| </button> |
| interface WebGLErrorBoundaryState { | ||
| hasError: boolean; | ||
| errorMessage: string; | ||
| } | ||
|
|
||
| /** | ||
| * Error boundary that catches WebGL context creation failures thrown by | ||
| * Three.js / React Three Fiber so the rest of the UI stays functional. | ||
| */ | ||
| class WebGLErrorBoundary extends Component< | ||
| { children: React.ReactNode; fallback: React.ReactNode }, | ||
| WebGLErrorBoundaryState | ||
| > { | ||
| constructor(props: { children: React.ReactNode; fallback: React.ReactNode }) { | ||
| super(props); | ||
| this.state = { hasError: false, errorMessage: "" }; | ||
| } | ||
|
|
||
| static getDerivedStateFromError(error: Error): WebGLErrorBoundaryState { | ||
| return { hasError: true, errorMessage: error.message }; | ||
| } |
| const ModelViewerInner = memo(function ModelViewerInner({ | ||
| url, | ||
| position = [0, 0, 0], | ||
| color, | ||
| coloredTexture, |
| return ( | ||
| <group position={position} {...props}> | ||
| <primitive object={clonedScene} /> | ||
| </group> | ||
| ); |
No description provided.