Skip to content
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

WebGLRenderer: Only compile renderable objects #29345

Merged
merged 1 commit into from
Sep 8, 2024

Conversation

vanruesc
Copy link
Contributor

@vanruesc vanruesc commented Sep 6, 2024

Description

The methods WebGLRenderer.compile() and WebGLRenderer.compileAsync() fail when trying to compile custom objects that define a material property. My use case involves objects that manage child meshes. The composite objects provide access to an inner, shared material via a getter for convenience.

To reproduce the error, uncomment line 47 or 48 in the following example: https://jsfiddle.net/7nvcjwk8/

Uncaught (in promise) TypeError: geometry is undefined
    getParameters WebGLPrograms.js:77
    getProgram WebGLRenderer.js:1631
    prepareMaterial WebGLRenderer.js:921
    compile WebGLRenderer.js:999
    traverse Object3D.js:536
    traverse Object3D.js:542
    compile WebGLRenderer.js:980
    compileAsync WebGLRenderer.js:1019
    init webgl_geometry_cube.html:65
    <anonymous> webgl_geometry_cube.html:37
WebGLPrograms.js:77

This PR changes the compile method to only process renderable objects, similar to how it's done in WebGLRenderer.projectObject().

Alternatively, Object3D could define isRenderable with a default value of false and the renderable sub classes could set this flag to true.

Copy link

github-actions bot commented Sep 6, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 685.18
169.62
685.24
169.64
+56 B
+14 B
WebGPU 825.96
221.44
825.96
221.44
+0 B
+0 B
WebGPU Nodes 825.54
221.34
825.54
221.34
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 461.96
111.46
462.02
111.48
+56 B
+15 B
WebGPU 525.27
141.52
525.27
141.52
+0 B
+0 B
WebGPU Nodes 481.93
131.34
481.93
131.34
+0 B
+0 B

@Mugen87 Mugen87 added this to the r169 milestone Sep 6, 2024
@Mugen87 Mugen87 merged commit 256b20e into mrdoob:dev Sep 8, 2024
12 checks passed
@vanruesc vanruesc deleted the fix-compile branch September 8, 2024 18:27
@mrdoob
Copy link
Owner

mrdoob commented Sep 12, 2024

The methods WebGLRenderer.compile() and WebGLRenderer.compileAsync() fail when trying to compile custom objects that define a material property.

Shouldn't you be using object.userData for that?

@vanruesc
Copy link
Contributor Author

vanruesc commented Sep 12, 2024

Shouldn't you be using object.userData for that?

In an end product (i.e. when working with instances of builtin classes) yes, but my use case is a custom class in a library. The class is a custom 3D object that manages an internal InstancedMesh which changes based on certain parameters and userData isn't really suited for defining its public API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants