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

InstancedMesh's setMatrixAt fails when another mesh in the scene has the same material #17701

Closed
5 of 13 tasks
GuilhermeRossato opened this issue Oct 9, 2019 · 10 comments · Fixed by #20135
Closed
5 of 13 tasks
Labels

Comments

@GuilhermeRossato
Copy link

GuilhermeRossato commented Oct 9, 2019

Description of the problem

I'm having a problem where the InstancedMesh method setMatrixAt seems to fail (the matrix positions doesn't change visually) whenever I add another object with the same material object (not a clone) in the scene. No log/warning/errors are emitted.

The following briefly illustrates:

    var geometry = new THREE.BoxBufferGeometry();
    var material = new THREE.MeshStandardMaterial( { roughness: 0, envMap: texture });
    var mesh1 = new THREE.Mesh( geometry, material );
    mesh1.position.set(0, 1, 0);
    scene.add( mesh1 ); // removing this line makes the instanced mesh work as intended
    mesh = new THREE.InstancedMesh( geometry, material, 9 ); // same material usage
    scene.add( mesh ); // This works and I can see both meshes in the scene.
    
    var dummy = new THREE.Object3D();
    dummy.position.set( 1, 5, 1 );
    dummy.updateMatrix();
    mesh.setMatrixAt( 0, dummy.matrix ); // Nope, doesn't work, it just stays where it's at.
    mesh.setMatrixAt( 1, dummy.matrix ); // Rotation seems to be applied, but position doesn't.
    // It looks like every instance is at the same place, but I don't know.

Note that if I use material.clone() at the instanced mesh declaration, the problem also goes away / is fixed, but now I have two materials, and I intend to have one for performance reasons.

A live example is available here:

  • jsfiddle (The problem is that you should see multiple boxes, but only 1 or 2 are visible, remove line 41 to fix)
Three.js version
  • Dev
  • r109
  • r108
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, ...)

(Not applicable, as far as I know)

@GuilhermeRossato
Copy link
Author

I have not checked whether other OS'es or Browsers have this issue, I just confirmed it on Windows, both Chrome and Edge.

@mrdoob
Copy link
Owner

mrdoob commented Oct 9, 2019

Yeah, known issue. Haven't made that work yet.

@mrdoob mrdoob added this to the r110 milestone Oct 9, 2019
@mrdoob mrdoob modified the milestones: r110, r111 Oct 30, 2019
@s-ol
Copy link

s-ol commented Nov 9, 2019

Not sure if that is a related issue or intended behaviour, but I also have problems re-usinng the BufferGeometry passed into InstancedMesh. For example when using one geometry for two InstancedMeshes, neither renders, and there is also no warnings or errors.

@mrdoob
Copy link
Owner

mrdoob commented Nov 10, 2019

@s-ol Interesting. Do you mind putting together a jsfiddle?

@s-ol
Copy link

s-ol commented Nov 10, 2019

@mrdoob sure, here you go: http://jsfiddle.net/x7Lzvqte/

you can toggle the red & green batches on and off individually with the bools at the top.
When either one is on it works as expected (red on the left or green on the right), but when both are on only the green one renders, and it renders on the left side.

@mrdoob mrdoob modified the milestones: r111, r112 Nov 27, 2019
@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 15, 2019

I've debugged the issue a bit and found out that the bug refers to the same root cause mentioned here: #17959 (comment).

The renderer needs the ability to manage multiple shader programs per material, see #15047. Currently there is always a 1:1 relationship (which is somewhat wrong since material properties are not the only factors which determine the correct shader program). So the renderer assumes it can reuse the same shader for both the mesh and instanced mesh.

@donmccurdy
Copy link
Collaborator

The renderer needs the ability to manage multiple shader programs per material. Currently there is always a 1:1 relationship...

Or alternatively, a property on the material that enables instancing, similar to the relationship between .skinning=true and THREE.SkinnedMesh. I’m not sure which solution is better but would prefer consistency. 🤔

@donmccurdy
Copy link
Collaborator

but now I have two materials, and I intend to have one for performance reasons.

Note that in any outcome here, you really do need two shader programs for this situation. You might find that using an InstancedMesh with count=1 for the second object avoids this, not sure if there should be any side effects of that...

@mrdoob mrdoob modified the milestones: r112, r113 Dec 23, 2019
@mrdoob mrdoob modified the milestones: r113, r114 Jan 29, 2020
@mrdoob mrdoob modified the milestones: r114, r115 Feb 29, 2020
@mracette
Copy link

mracette commented Mar 1, 2020

I just got bit by this as well. A note in the docs while the fix is underway would be great.
https://threejs.org/docs/#api/en/objects/InstancedMesh

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 2, 2020

@mracette Good point. Let me update the docs.

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

Successfully merging a pull request may close this issue.

6 participants