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

SSRPass: throws err for a simple scene. #21379

Closed
ycw opened this issue Feb 28, 2021 · 11 comments
Closed

SSRPass: throws err for a simple scene. #21379

ycw opened this issue Feb 28, 2021 · 11 comments

Comments

@ycw
Copy link
Contributor

ycw commented Feb 28, 2021

Describe the bug

  • An err is thrown when i call render() on EffectComposer{} which is added a SSRPass.
  • The err only occurs when my scene contains both Meshs and InstancedMeshs.

To Reproduce

Steps to reproduce the behavior:

  1. add 1 instanced mesh and 1 regular mesh to then scene
  2. make an effect composer; add a RenderPass .. followed by a SSRPass.
  3. call composer.render() in animation loop.
  4. bring up devtools, it shows

Uncaught TypeError: can't access property "isInterleavedBufferAttribute", attribute is undefined

stacktrace:

    get https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:12445
    setupVertexAttributes https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:14100
    setup https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:13770
    renderBufferDirect https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:23733
    renderObject https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:24269
    renderObjects https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:24242
    render https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:24025
    renderOverride https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/examples/jsm/postprocessing/SSRPass.js:577
    render https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/examples/jsm/postprocessing/SSRPass.js:396
    render https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/examples/jsm/postprocessing/EffectComposer.js:150
    <anonymous> https://fiddle.jshell.net/_display/?editor_console=true:196
    onAnimationFrame https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:23897
    onAnimationFrame https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:12286
    start https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:12299
    setAnimationLoop https://cdn.jsdelivr.net/gh/mrdoob/three.js@dev/build/three.module.js:23911
    <anonymous> https://fiddle.jshell.net/_display/?editor_console=true:195

Live example

  • jsfiddle (see devtools console for the err)

Expected behavior

it should render a box and two spheres .. w/ reflection on their surfaces.

Platform:

  • Device: [Desktop]
  • OS: [Windows]
  • Browser: [Firefox]
  • Three.js version: [dev]

Thanks.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

/ping @gonnavis

@gonnavis
Copy link
Contributor

Received, I will try to find out what the problem is.

@gonnavis
Copy link
Contributor

gonnavis commented Feb 28, 2021

@ycw Thanks for your convient demo code!

@Mugen87 After some testing, I found it may caused by the MeshNormalMaterial overrideMaterial combined with InstancedMesh (SSRPass use MeshNormalMaterial as overrideMaterial internally).

Because after I removed all composer related codes, and use renderer.render(scene, camera) instead of compoer.render(), then add scene.overrideMaterial = new THREE.MeshNormalMaterial(), the error still occur. jsfiddle

I'll continue to research the problem and try to solve it if I can.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

Probably a duplicate of #18533.

@ycw
Copy link
Contributor Author

ycw commented Feb 28, 2021

@gonnavis @Mugen87 I don't know why it works after swapping the order of meshes creation. ( regular mesh first, followed by instanced mesh ) see https://jsfiddle.net/trch4517/

@gonnavis
Copy link
Contributor

gonnavis commented Feb 28, 2021

Yeah, I also found in this non-composer scene https://jsfiddle.net/gonnavis/hjgdrfv5/2/ , the instancedMeshes (two spheres) which probably caused this problem are rendered, but the ordinary mesh (the cube) not rendered. So it should be the error throwed in instancedMesh blocked the subsequent ordinary mesh render.

After swap the creation in this non-composer scene https://jsfiddle.net/gonnavis/hjgdrfv5/5/ , I see one ordinary mesh and one instance rendered ( should be the first instance rendered ). So the normal infos still not all correct, this is why in https://jsfiddle.net/trch4517/ just one sphere has reflection.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

Okay, in this case I mark this issue as a duplicate of #18533 since it's the same cause (and the same generic solution is required).

@ycw
Copy link
Contributor Author

ycw commented Feb 28, 2021

thanks

@gonnavis
Copy link
Contributor

gonnavis commented Feb 28, 2021

@ycw @Mugen87 The main problem may because of instancedMesh and regular mesh can't share same material now.

One obvious low-performance workaround is, at here

insert

	this.scene.traverseVisible( child => {

		child._normalMaterial = new MeshNormalMaterial();

	});

and replace these codes

this.scene.overrideMaterial = overrideMaterial;
renderer.render( this.scene, this.camera );
this.scene.overrideMaterial = null;
with

		this.scene.traverseVisible( child => {

			child._SSRPassMaterialBack = child.material;

			child.material = child._normalMaterial;

		} );
		renderer.render( this.scene, this.camera );
		this.scene.traverseVisible( child => {

			child.material = child._SSRPassMaterialBack;

		} );

then it works well. If do some type check, just create two MeshNormalMaterials for instanced or regular mesh seperately, would be better.
image

But the fundamental problem seems too in-depth for me to solve~~

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

If it's only about material sharing, #20135 should fix it.

In any event, we should not start to add fixes per pass. The renderer should handle this internally.

@ycw
Copy link
Contributor Author

ycw commented Mar 1, 2021

@gonnavis @Mugen87 @mrdoob i tried to merge #20135 ( w/ few bugfixes ) to r126 dev and .. it works!

https://jsfiddle.net/m7bcex6s/1/

thanks.

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

No branches or pull requests

3 participants