Enhance WebGL error handling and optimize drag preview performance - #29
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.
Feature/opti editor
There was a problem hiding this comment.
Code Review
This pull request introduces several performance optimizations and error-handling improvements to the 3D editor canvas. Key changes include caching the scene mesh list and reusing Three.js objects in useDragPreview to avoid per-frame allocations, introducing a DragPreviewGate to prevent idle frame loops, adding a WebGL availability check and error boundary, and refactoring ModelViewer to clone scenes only once per GLB load. The reviewer identified a critical memory leak in ModelViewer where newly created WebGL materials are never disposed, and provided a comprehensive code suggestion to clean up these resources upon unmount or scene changes.
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.
| // Clone the scene ONCE per GLB load. | ||
| // color and coloredTexture are intentionally NOT listed as deps here. | ||
| const clonedScene = useMemo(() => { | ||
| const clone = scene.clone(); | ||
| let currentTexture: THREE.Texture | null = null; | ||
|
|
||
| // Helper: create a custom shader material for recoloring | ||
| const createRecolorMaterial = (texture: THREE.Texture, tint: string) => { | ||
| return new THREE.ShaderMaterial({ | ||
| uniforms: { | ||
| map: { value: texture }, | ||
| tint: { value: new THREE.Color(tint) }, | ||
| }, | ||
| vertexShader: ` | ||
| varying vec2 vUv; | ||
| void main() { | ||
| vUv = uv; | ||
| gl_Position = projectionMatrix * modelViewMatrix * vec4(position, 1.0); | ||
| } | ||
| `, | ||
| fragmentShader: ` | ||
| uniform sampler2D map; | ||
| uniform vec3 tint; | ||
| varying vec2 vUv; | ||
|
|
||
| // Helper: RGB to HSV | ||
| vec3 rgb2hsv(vec3 c) { | ||
| vec4 K = vec4(0.0, -1.0/3.0, 2.0/3.0, -1.0); | ||
| vec4 p = mix(vec4(c.bg, K.wz), vec4(c.gb, K.xy), step(c.b, c.g)); | ||
| vec4 q = mix(vec4(p.xyw, c.r), vec4(c.r, p.yzx), step(p.x, c.r)); | ||
| float d = q.x - min(q.w, q.y); | ||
| float e = 1.0e-10; | ||
| return vec3(abs(q.z + (q.w - q.y) / (6.0 * d + e)), d / (q.x + e), q.x); | ||
| } | ||
| // Helper: HSV to RGB | ||
| vec3 hsv2rgb(vec3 c) { | ||
| vec4 K = vec4(1.0, 2.0/3.0, 1.0/3.0, 3.0); | ||
| vec3 p = abs(fract(c.xxx + K.xyz) * 6.0 - K.www); | ||
| return c.z * mix(K.xxx, clamp(p - K.xxx, 0.0, 1.0), c.y); | ||
| } | ||
| void main() { | ||
| vec4 texColor = texture2D(map, vUv); | ||
| vec3 texHSV = rgb2hsv(texColor.rgb); | ||
| vec3 tintHSV = rgb2hsv(tint); | ||
| // Replace hue with tint's hue, keep original s/v | ||
| texHSV.x = tintHSV.x; | ||
| vec3 recolored = hsv2rgb(texHSV); | ||
| gl_FragColor = vec4(recolored, texColor.a); | ||
| } | ||
| `, | ||
| }); | ||
| }; | ||
| materialsRef.current = []; | ||
|
|
||
| let lastTexture: THREE.Texture | null = null; | ||
|
|
||
| // Enable shadows and apply color if provided | ||
| clone.traverse((child) => { | ||
| if (child instanceof THREE.Mesh) { | ||
| // Only apply recolor shader for holds if coloredTexture and color are set | ||
| if (coloredTexture && color && child.material) { | ||
| // Try to extract the first texture from the material | ||
| let texture: THREE.Texture | null = null; | ||
| if (Array.isArray(child.material)) { | ||
| for (const mat of child.material) { | ||
| const stdMat = mat as THREE.MeshStandardMaterial; | ||
| if (stdMat.map) { | ||
| texture = stdMat.map; | ||
| break; | ||
| } | ||
| } | ||
| } else { | ||
| const stdMat = child.material as THREE.MeshStandardMaterial; | ||
| if (stdMat.map) { | ||
| texture = stdMat.map; | ||
| } | ||
| } | ||
|
|
||
| if (texture) { | ||
| currentTexture = texture; | ||
| child.material = createRecolorMaterial(texture, color); | ||
| } else if (currentTexture) { | ||
| child.material = createRecolorMaterial(currentTexture, color); | ||
| } else { | ||
| // Fallback: use MeshStandardMaterial with color | ||
| child.material = new THREE.MeshStandardMaterial({ | ||
| color: color, | ||
| roughness: 0.7, | ||
| metalness: 0.1, | ||
| }); | ||
| if (!(child instanceof THREE.Mesh) || !child.material) return; | ||
|
|
||
| const originalMat = child.material; | ||
| let texture: THREE.Texture | null = null; | ||
|
|
||
| if (Array.isArray(originalMat)) { | ||
| for (const mat of originalMat) { | ||
| const stdMat = mat as THREE.MeshStandardMaterial; | ||
| if (stdMat.map) { | ||
| texture = stdMat.map; | ||
| break; | ||
| } | ||
| } else if (color && child.material) { | ||
| // Apply color override if provided (for walls or if not using coloredTexture) | ||
| child.material = new THREE.MeshStandardMaterial({ | ||
| color: color, | ||
| roughness: 0.7, | ||
| metalness: 0.1, | ||
| side: THREE.DoubleSide, | ||
| }); | ||
| } | ||
| } else { | ||
| texture = (originalMat as THREE.MeshStandardMaterial).map ?? null; | ||
| } | ||
| // Fall back to a texture from a sibling mesh (same GLB can share textures) | ||
| if (!texture && lastTexture) texture = lastTexture; | ||
| if (texture) lastTexture = texture; | ||
|
|
||
| const shaderMat = texture | ||
| ? createRecolorMaterial(texture, colorRef.current ?? "#ffffff") | ||
| : null; | ||
| const plainMat = new THREE.MeshStandardMaterial({ | ||
| roughness: 0.7, | ||
| metalness: 0.1, | ||
| side: THREE.DoubleSide, | ||
| }); | ||
|
|
||
| materialsRef.current.push({ mesh: child, shaderMat, plainMat, originalMat }); | ||
| }); | ||
|
|
||
| // Apply the current color synchronously so the very first frame is correct | ||
| applyMaterials(materialsRef.current, colorRef.current, coloredTextureRef.current); | ||
|
|
||
| return clone; | ||
| }, [scene, color, coloredTexture]); | ||
| }, [scene]); // ← only re-runs when the GLB itself changes | ||
|
|
||
| // On subsequent color / mode changes: update uniforms only — no scene clone | ||
| useLayoutEffect(() => { | ||
| applyMaterials(materialsRef.current, color, coloredTexture); | ||
| }, [color, coloredTexture, clonedScene]); |
There was a problem hiding this comment.
Memory Leak: Newly created WebGL materials are never disposed
In Three.js, custom materials (ShaderMaterial and MeshStandardMaterial) allocate GPU resources that must be manually freed by calling .dispose().
Currently, when scene changes, clonedScene is re-evaluated, and materialsRef.current is overwritten with a fresh array, leaking the previously created materials in GPU memory. Additionally, when the component unmounts, the active materials are also leaked.
To fix this, we can return both the clonedScene and the list of materials from useMemo, and clean them up in a useEffect that runs whenever the active materials list changes or the component unmounts. This ensures proper garbage collection of WebGL resources.
// Clone the scene ONCE per GLB load.
// color and coloredTexture are intentionally NOT listed as deps here.
const { clonedScene, materials } = useMemo(() => {
const clone = scene.clone();
const materialsList: MaterialEntry[] = [];
let lastTexture: THREE.Texture | null = null;
clone.traverse((child) => {
if (!(child instanceof THREE.Mesh) || !child.material) return;
const originalMat = child.material;
let texture: THREE.Texture | null = null;
if (Array.isArray(originalMat)) {
for (const mat of originalMat) {
const stdMat = mat as THREE.MeshStandardMaterial;
if (stdMat.map) {
texture = stdMat.map;
break;
}
}
} else {
texture = (originalMat as THREE.MeshStandardMaterial).map ?? null;
}
// Fall back to a texture from a sibling mesh (same GLB can share textures)
if (!texture && lastTexture) texture = lastTexture;
if (texture) lastTexture = texture;
const shaderMat = texture
? createRecolorMaterial(texture, colorRef.current ?? "#ffffff")
: null;
const plainMat = new THREE.MeshStandardMaterial({
roughness: 0.7,
metalness: 0.1,
side: THREE.DoubleSide,
});
materialsList.push({ mesh: child, shaderMat, plainMat, originalMat });
});
// Apply the current color synchronously so the very first frame is correct
applyMaterials(materialsList, colorRef.current, coloredTextureRef.current);
return { clonedScene: clone, materials: materialsList };
}, [scene]); // ← only re-runs when the GLB itself changes
// Keep materialsRef in sync with the current active materials
materialsRef.current = materials;
// Clean up WebGL materials to prevent GPU memory leaks
useEffect(() => {
return () => {
for (const entry of materials) {
entry.shaderMat?.dispose();
entry.plainMat.dispose();
}
};
}, [materials]);
// On subsequent color / mode changes: update uniforms only — no scene clone
useLayoutEffect(() => {
applyMaterials(materialsRef.current, color, coloredTexture);
}, [color, coloredTexture, clonedScene]);
There was a problem hiding this comment.
Pull request overview
This PR improves the editor’s runtime robustness and interaction performance by adding WebGL availability/error handling around the main R3F canvas, and by reducing per-frame allocations/work in drag preview and GLB recoloring.
Changes:
- Optimizes drag preview raycasting by caching scene meshes per drag and reusing Three.js objects; reduces DOM layout work via cached canvas bounds.
- Refactors
ModelViewerto avoid cloning scenes on every color/mode change by caching per-mesh materials and updating via uniform/property mutation. - Adds a WebGL availability check + error boundary with a UI fallback to keep the rest of the editor usable when WebGL fails.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| frontend/src/features/editor/components/useDragPreview.ts | Reuses Raycaster/Vector2, caches raycast mesh list per drag, and caches canvas bounds for faster per-frame raycasting. |
| frontend/src/features/editor/components/ModelViewer.tsx | Avoids repeated scene.clone() on color changes by caching materials and updating uniforms/material props. |
| frontend/src/features/editor/components/MainCanvas.tsx | Adds WebGL detection/error boundary + fallback UI and gates drag preview mounting while dragging. |
| frontend/package-lock.json | Updates frontend dependency lockfile to newer versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <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> |
| 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> |
| /** | ||
| * Fix #3 – DragPreviewGate only mounts DragPreview (and its useFrame loop) | ||
| * while a sidebar drag is actually in progress. When idle, no useFrame | ||
| * callback is registered at all. | ||
| */ |
| window.addEventListener("scroll", update, { passive: true }); | ||
| return () => { | ||
| ro.disconnect(); | ||
| window.removeEventListener("scroll", update); | ||
| }; |
| // On subsequent color / mode changes: update uniforms only — no scene clone | ||
| useLayoutEffect(() => { | ||
| applyMaterials(materialsRef.current, color, coloredTexture); | ||
| }, [color, coloredTexture, clonedScene]); |
No description provided.