Skip to content

Emit LOD visibility event after GLB render #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shayancoin merged 1 commit into from
Oct 15, 2025
Merged

Emit LOD visibility event after GLB render #15

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion frontend/src/app/components/viewer3d.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
'use client';

import React, { Suspense, useMemo } from 'react';
import React, { Suspense, useEffect, useMemo } from 'react';
import { Canvas } from '@react-three/fiber';
import { Environment, OrbitControls, useGLTF } from '@react-three/drei';

type Viewer3DProps = {
modelUrl: string;
};

declare global {
interface Window {
__lod0VisibleAt?: number;
}
}
Comment on lines +11 to +15
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.


function Model({ modelUrl }: { modelUrl: string }) {
// Resolve absolute URL to avoid base path issues
const src = useMemo(() => {
Expand All @@ -19,6 +25,33 @@ function Model({ modelUrl }: { modelUrl: string }) {

// Load GLB (no extra params to avoid signature mismatches)
const gltf = useGLTF(src);

useEffect(() => {
if (typeof window === 'undefined') {
return undefined;
}

if (!gltf?.scene) {
return undefined;
}

let cancelled = false;
const rafId = window.requestAnimationFrame(() => {
if (cancelled) {
return;
}

const ts = performance.now();
window.__lod0VisibleAt = ts;
window.dispatchEvent(new CustomEvent('lod0:visible', { detail: { ts } }));
Comment on lines +39 to +46
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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 requestIdleCallback API 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.tsx

Length 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.

Comment on lines +44 to +46
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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 src to 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.tsx

Length 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 by src
  • 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.

});

return () => {
cancelled = true;
window.cancelAnimationFrame(rafId);
};
}, [gltf?.scene, src]);

return <primitive object={gltf.scene} />;
}

Expand Down
Loading