Emit LOD visibility event after GLB render#15
Conversation
WalkthroughAdds a requestAnimationFrame after GLTF load in viewer3d Model to timestamp when LOD0 becomes visible and dispatch a 'lod0:visible' CustomEvent. Introduces a global Window augmentation for __lod0VisibleAt, guards for window and gltf.scene presence, and cleans up scheduled RAF on unmount or dependency change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant React as React Component (Model)
participant Loader as GLTF Loader
participant RAF as requestAnimationFrame
participant Win as window
participant DOM as Document
User->>React: Mount Model
React->>Loader: Load GLTF(src)
Loader-->>React: onLoad(gltf)
Note over React: Verify window and gltf.scene
React->>RAF: schedule callback
RAF-->>React: callback
React->>Win: set window.__lod0VisibleAt = now
React->>DOM: dispatch CustomEvent('lod0:visible', { detail: now })
rect rgba(230,250,230,0.6)
Note right of DOM: Event listeners may react to LOD0 visibility
end
User-->>React: Unmount / deps change
React->>RAF: cancel via flag (skip callback path)
Note over React: Cleanup completed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/components/viewer3d.tsx(2 hunks)
🔇 Additional comments (2)
frontend/src/app/components/viewer3d.tsx (2)
3-3: LGTM!The
useEffectimport is necessary for the new effect hook and is correctly added.
53-53: Ignore dependency-array suggestion: the early-return guard (if !gltf?.scene return) prevents any dispatch onsrcchanges, so only one event fires when the new model loads. Removingsrcisn’t needed to avoid double or premature dispatch.Likely an incorrect or invalid review comment.
| declare global { | ||
| interface Window { | ||
| __lod0VisibleAt?: number; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider moving global augmentation to a dedicated types file.
While the Window interface augmentation is functionally correct, global type declarations are conventionally placed in dedicated type definition files (e.g., globals.d.ts or window.d.ts) rather than component files. This improves maintainability and separates type definitions from component logic.
🤖 Prompt for AI Agents
In frontend/src/app/components/viewer3d.tsx around lines 11 to 15, the global
Window augmentation is declared inline in the component file; move this
declaration to a dedicated type definition file (e.g., src/types/globals.d.ts or
src/types/window.d.ts). Create the .d.ts file, copy the declare global {
interface Window { __lod0VisibleAt?: number; } } block there, ensure the new
file is included in tsconfig "include" (or placed under an already included
folder), and remove the declaration from viewer3d.tsx so component code contains
no global type augmentations.
| const rafId = window.requestAnimationFrame(() => { | ||
| if (cancelled) { | ||
| return; | ||
| } | ||
|
|
||
| const ts = performance.now(); | ||
| window.__lod0VisibleAt = ts; | ||
| window.dispatchEvent(new CustomEvent('lod0:visible', { detail: { ts } })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify: requestAnimationFrame executes before paint, not after visible render.
The RAF callback runs before the next paint, not after the frame is painted to the screen. This means window.__lod0VisibleAt and the 'lod0:visible' event are set/dispatched before the GLB model is actually rendered and visible to the user.
If the goal is to track when LOD0 is truly visible (painted), consider these options:
- Use a second RAF nested inside the first to ensure at least one paint cycle completes
- Use the
requestIdleCallbackAPI if visual confirmation is needed - Document that this timestamp represents "scheduled for render" rather than "visible"
To confirm the timing behavior, you can test with this approach:
// Nested RAF ensures one paint cycle
const rafId = window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
if (cancelled) return;
const ts = performance.now();
window.__lod0VisibleAt = ts;
window.dispatchEvent(new CustomEvent('lod0:visible', { detail: { ts } }));
});
});🏁 Script executed:
#!/bin/bash
# Display the first 200 lines of viewer3d.tsx to inspect the useEffect implementation
sed -n '1,200p' frontend/src/app/components/viewer3d.tsxLength of output: 1767
Ensure visibility timestamp after paint
requestAnimationFrame callbacks run before the next repaint, so the current code dispatches lod0:visible before the model is actually rendered. To capture the post-paint frame, nest a second RAF or use requestIdleCallback, for example:
const rafId = window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
if (cancelled) return;
const ts = performance.now();
window.__lod0VisibleAt = ts;
window.dispatchEvent(new CustomEvent('lod0:visible', { detail: { ts } }));
});
});🤖 Prompt for AI Agents
In frontend/src/app/components/viewer3d.tsx around lines 39 to 46, the current
requestAnimationFrame handler fires before the browser paint so the lod0:visible
event is emitted too early; update the logic to schedule a nested
requestAnimationFrame (or requestIdleCallback) inside the first RAF and perform
the cancelled check and timestamp/dispatch in that second callback so the
timestamp reflects after-paint rendering.
| const ts = performance.now(); | ||
| window.__lod0VisibleAt = ts; | ||
| window.dispatchEvent(new CustomEvent('lod0:visible', { detail: { ts } })); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Multiple Model instances will overwrite window.__lod0VisibleAt.
If multiple Model components are rendered simultaneously, each will overwrite window.__lod0VisibleAt and dispatch its own 'lod0:visible' event. This may cause race conditions or confusion about which model's timestamp is recorded.
Consider one of these approaches:
- Document this single-instance assumption
- Use a Map keyed by
srcto track multiple models:window.__lod0VisibleAt = new Map<string, number>() - Include the model URL in the event detail for disambiguation
🏁 Script executed:
#!/bin/bash
echo "Occurrences of __lod0VisibleAt:"
rg -n "__lod0VisibleAt"
echo -e "\nOccurrences of 'lod0:visible':"
rg -n "lod0:visible"Length of output: 341
🏁 Script executed:
sed -n '1,30p' frontend/src/app/components/viewer3d.tsxLength of output: 758
Avoid overwriting a single global timestamp across multiple Model instances.
If more than one <Model> renders, each will reset window.__lod0VisibleAt and fire 'lod0:visible' without distinguishing which model triggered it. Consider one of the following:
- Document that only one model may be mounted at a time
- Switch to
window.__lod0VisibleAt = new Map<string, number>()keyed bysrc - Include
src(model URL) in the event detail:
new CustomEvent('lod0:visible', { detail: { src, ts } })
🤖 Prompt for AI Agents
In frontend/src/app/components/viewer3d.tsx around lines 44-46, the code writes
a single global timestamp to window.__lod0VisibleAt and emits 'lod0:visible'
without identifying which Model instance caused it; change this to use a Map
keyed by the model's src and include src in the event detail: create or reuse
window.__lod0VisibleAt as a Map<string, number>, set
window.__lod0VisibleAt.set(src, ts) instead of overwriting a scalar, and
dispatch new CustomEvent('lod0:visible', { detail: { src, ts } }) so listeners
can distinguish between different models.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @shayancoin. * #15 (comment) The following files were modified: * `frontend/src/app/components/viewer3d.tsx`
Docstrings generation was requested by @shayancoin. * #15 (comment) The following files were modified: * `frontend/src/app/components/viewer3d.tsx` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68eefda155e4833089be4c18982822d4
Summary by CodeRabbit