mrdoob
Line2: Fix error when raycasting line object with materials array
Related issue: --
Description
Line2.raycast would throw an error when the Line2 instance had an array of materials because it relies on material.resolution and material.linewidth. Now if an array of materials is present the resolution and linewidth from the first material in the array are used.
cc @WestLangley
Line2.raycast would throw an error when the Line2 instance had an array of materials...
Does Line2 support materials array?
Does
Line2support materials array?
Yeah I'm using it my project. You just have to add groups to your geometry in order to use multiple materials:
const line = new Line2();
line.geometry.addGroup(0, Infinity, 0);
line.geometry.addGroup(0, Infinity, 1);
line.material = [ lineMat1, lineMat2 ];
line.setPositions( ... );
In my case I'm drawing the geometry twice -- once with full opacity and once with partial opacity and inverted depth test for an xray effect.
But the conventional use of groups is to render part of the geometry with one material and another part with another material. In which case, you should raycast using the proper material. No?
But the conventional use of groups is to render part of the geometry with one material and another part with another material. In which case, you should raycast using the proper material. No?
You're right. After a bit more testing it looks like using groups with instanced geometry like this still specify the vertex indices to draw with a mterial rather than the which instances to draw (which might be more intuitive). Here's what the fat lines example looks like when setting calling .addGroup( 0, 10, 0 ) on the geometry with an array of a single material.
Given that geometry groups that specify a subrange seem like they'd be an uncommon use case so I'm not sure if it makes sense to iterate over all materials / groups for raycasting. But it would be nice if the raycasting did not throw an error when using it the way I've specified.
What is actually your use case for using groups with instanced lines?
Fixing a potential error only makes sense to me if (and only if) the setup producing the error is valid.
@Mugen87
What is actually your use case for using groups with instanced lines?
It's what I mentioned in https://github.com/mrdoob/three.js/pull/20861#issuecomment-741263216. I add two groups and materials to the Line2 Geometry so I can draw it twice in the same spot to get a "draw through" or xray effect. I do the same with other non line geometries, as well. Here's a fiddle that shows the effect:
https://jsfiddle.net/h6wg4x5f/2/
I'm not using groups in any special or undocumented way here.
@Mugen87 I've updated the PR so the material with the largest line width is used for raycasting if there are multiple materials. The code I've presented otherwise runs and works correctly -- raycasting is the only place it breaks so I'd hope that can be fixed.
I agree with @WestLangley. Groups were not meant to be used to draw a geometry multiple times...
But it would be nice if the raycasting did not throw an error when using it the way I've specified.
How about just logging a warning if Array.isArray( material ) === true instead? 🤔
Groups were not meant to be used to draw a geometry multiple times...
Is there are reason you don't want to support that? It works as expected right now and there's nothing in the docs that specify that group indices can't overlap. I think it's a useful mechanism. For reference Unity supports rendering geometry subgroups multiple times with its Renderer.materials array.
Should this PR be closed now when https://github.com/mrdoob/three.js/pull/23740 is merged?
I would still like to understand why rendering a single geometry with multiple materials isn't something that should be supported. It's a useful feature for a few reasons and is possible in other engines. Otherwise users are stuck creating multiple copies of the same mesh, ensuring they're both managed, updated, and transformed synchronously (which is a pain with user interaction), and raycasting performance suffers because now there are extra number of objects in the scene that are being cast against.
I'd also like to push back against the idea that if multiple materials are supported then multiple geometries per mesh would be expected to be supported from https://github.com/mrdoob/three.js/issues/12135:
If we follow this style, it would also be nice to be able to do this:
var mesh = new THREE.Mesh( [ geometry1, geometry2 ], material );
There are already plenty of ways to manage multiple separate geometries in a single mesh with a common transform. The geometry can be merged (and identified via group if needed). A skinned mesh can be used as I've demonstrated here. And coming up the BatchedMesh proposal will afford this extremely easily (#22376).
But there's still no easy way to render a single geometry with two materials twice (or more). I don't think it's right to say these are equivalent requests or problems.