-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(viewer): guard TextureNode updates against null textures #527
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import type * as THREE from 'three/webgpu' | ||
| import { DataTexture, TextureNode } from 'three/webgpu' | ||
|
|
||
| let installed = false | ||
| let reported = 0 | ||
| let fallbackTexture: THREE.Texture | null = null | ||
|
|
||
| function getFallbackTexture(): THREE.Texture { | ||
| if (!fallbackTexture) { | ||
| fallbackTexture = new DataTexture(new Uint8Array([0, 0, 0, 255]), 1, 1) | ||
| fallbackTexture.needsUpdate = true | ||
| } | ||
| return fallbackTexture | ||
| } | ||
|
|
||
| /** | ||
| * three's node system pulls texture uniforms from materials each frame via | ||
| * reference nodes, and several override-material passes (shadow, depth/normal | ||
| * prepasses) copy per-object texture slots onto shared materials whose cached | ||
| * per-mesh node graphs can disagree about a slot's presence. When they do, | ||
| * `TextureNode.update` dereferences a null texture and the exception kills the | ||
| * whole render pass — the scene goes black. Substitute a 1×1 black fallback | ||
| * instead (skipping is not enough: the null would still reach the backend's | ||
| * texture-binding WeakMap). The slot renders black for a frame and recovers | ||
| * as soon as the reference pulls a real value again. | ||
| */ | ||
| export function installTextureNodeNullGuard(): void { | ||
| if (installed) return | ||
| installed = true | ||
|
|
||
| const prototype = TextureNode.prototype as { update: () => void } | ||
| const originalUpdate = prototype.update | ||
|
|
||
| prototype.update = function update(this: { value: unknown; uuid: string }) { | ||
| if (this.value == null) { | ||
| if (reported < 5) { | ||
| reported += 1 | ||
| console.warn(`[viewer] TextureNode ${this.uuid} has no texture — using fallback`) | ||
| } | ||
| this.value = getFallbackTexture() | ||
| } | ||
| originalUpdate.call(this) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update patch drops frame argumentMedium Severity The Reviewed by Cursor Bugbot for commit 01acd19. Configure here. |
||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback assignment can stick on node
Medium Severity
When
valueis null, the guard writes the shared fallback ontothis.valueand leaves it there. Recovery assumes a reference will overwrite it later; if that pull is skipped or only runs on material changes, the node keeps sampling the 1×1 black texture after a real map is available, or keeps a fallback where the slot should stay empty.Reviewed by Cursor Bugbot for commit 01acd19. Configure here.