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

OutlinePass does not support InstancedMesh #18533

Closed
3 of 11 tasks
anthonysimone opened this issue Feb 2, 2020 · 7 comments
Closed
3 of 11 tasks

OutlinePass does not support InstancedMesh #18533

anthonysimone opened this issue Feb 2, 2020 · 7 comments

Comments

@anthonysimone
Copy link

Description of the problem

OutlinePass does not appear to properly support InstancedMesh. I've created an example here that uses the same setup of OutlinePass in the example in the docs. The sphere is a regular Mesh and the squares are the instances of InstancedMesh.
https://jsfiddle.net/anthonysimone/dL8s2qmv/

It seems like there's a couple things going on here.

  1. If you hover over the regular mesh first (sphere), then one of the instanced mesh instances, the base position of the InstancedMesh is outlined any time you’re hovering over any of the instances. In the above example all of the instances have been moved from the default position to demonstrate this clearly.
  2. If you first hover over any of the box instances, the entire collection of instances (though not the base position) of the InstancedMesh are outlined, but then hovering over a regular mesh (the sphere) causes an error.

I dug into OutlinePass a bit and it seems like it might just not be taking into account handling instanced meshes separately from regular meshes. I'm still pretty new to Three.js, so I don't have a potential fix to suggest at this point.

Three.js version
  • r113
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 3, 2020

The OutlinePass uses an internal material called prepareMaskMaterial which is used as a so called overrideMaterial. That means all objects in the scene are rendered with this material in order to produce the mask buffer. Depending on which objects are drawn first (sphere or instanced box), the material is configured differently (for instanced rendering or not). Which means it breaks when it is used with a different object type.

Fixing this is a bit hacky since the pass can't use a single override material for the scene anymore. It needs two different mask materials.

But even then the entire instanced mesh is rendered with an outline, which is probably not the expected behavior. The pass has to honor the instanceId property from the raycasting in some way which is probably more complicated than solving the first issue.

@gkjohnson
Copy link
Collaborator

Related to #14577. Adding that would address this rendering issue and quite a few of others with OutlinePass and other overrideMaterial-dependent effects.

@Mugen87

But even then the entire instanced mesh is rendered with an outline, which is probably not the expected behavior.

This is actually exactly what I would expect but it might not be the desired behavior in all cases. If I tell the outline pass to outline the instanced mesh it should the whole instanced mesh. I use mesh instancing to render voxels representing terrain so highlighting the whole object makes sense and if you're selecting something that's representative of a system like a particle effect then outlining the whole instanced mesh makes sense, too.

If the desired effect is for just one instance to be outlined then that's perfectly doable by creating a copy of one instance and outlining it. This could be done right now but the same issue is present with the depth pass that outline material uses, which you can see in the above fiddle when you highlight the sphere (notice the bottom left of the outline is obscured by the one cube from the instance mesh being rendered):

image

This would also be addressed by #14577.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 4, 2020

This is actually exactly what I would expect but it might not be the desired behavior in all cases. If I tell the outline pass to outline the instanced mesh it should the whole instanced mesh.

Yes, this sounds like a good default behavior for OutlinePass.

#14577 seems a bit vague to me. An override material can be anything, e.g. a simple material based on RawShaderMaterial written by the user. I don't think the engine should support the use case such that an override material can handle all original material settings. How would this look by the way? By modifying the user-providing shader code? That seems not like a proper approach to me. Instead, I would start with providing a more concrete option and introduce a depth pass, see #13858 and #8676.

@gkjohnson
Copy link
Collaborator

gkjohnson commented Feb 4, 2020

#14577 seems a bit vague to me.

I appreciate the feedback -- I've updated the original issue content to hopefully be more clear if you want to take another look.

I don't think the engine should support the use case such that an override material can handle all original material settings. How would this look by the way? By modifying the user-providing shader code? That seems not like a proper approach to me. Instead, I would start with providing a more concrete option and introduce a depth pass, see #13858 and #8676.

I don't think this is sustainable. Shadows already don't properly support the built in materials because displacement map isn't used and adding new hardcoded passes for everything is unrealistic -- proper normal passes with normal map support, diffuse color pass, etc would all be much easier to attain. It sounded like the deferred renderer was hard to maintain, buggy, and deprecated in part because of this. What I'm suggesting is something closer to Unity's Replacement Shader mechanism which makes these types of passes simple. If it's preferred that the previous behavior be retained then this could be an opt-in feature.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 4, 2020

I need to study Unity's Replacement Shader more closely. I've used Unity from time to time but never worked with that feature.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 25, 2021

Fixed via #20135, see https://jsfiddle.net/quf0p415/1/.

No runtime errors occur anymore. Point 1 of the original post is solved. Also the artifact mentioned in #18533 (comment). Point 2 is partially solved since all instanced meshes are highlighted when one is selected. But that is an acceptable behavior for now since some use cases require it anyway. A solution for individual highlighting is explained here: #18533 (comment)

@mrdoob I guess this is a good example of what you mean in #20975 (comment). By removing skinning, the same effect would be achieved and a single overrideMaterial can be used for normal, instanced and skinned meshes 🎉 .

@Mugen87 Mugen87 closed this as completed Mar 25, 2021
@mrdoob
Copy link
Owner

mrdoob commented Mar 25, 2021

@mrdoob I guess this is a good example of what you mean in #20975 (comment). By removing skinning, the same effect would be achieved and a single overrideMaterial can be used for normal, instanced and skinned meshes 🎉 .

Yes 😉

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

4 participants