Three.js: InstancedMesh.raycast should return array of instanceIds instead of a single instanceId

Created on 17 Apr 2020  路  12Comments  路  Source: mrdoob/three.js

Description of the problem

Currently there's no way to retrieve all the intersected instances when we raycast against an InstancedMesh, because only the id of the first intersected instance is returned, the rest are ignored.

I'm planning to create a PR to solve this, but first I wanted to create this ticket to see if there's any reason why we shouldn't do it.

Three.js version
  • [x] r115
Suggestion

Most helpful comment

I think it makes sense to return all intersections for the sake of consistency. I think these lines

https://github.com/mrdoob/three.js/blob/19beb8ecc83b8f52de1e00dcfca59fc2ce55078f/src/objects/InstancedMesh.js#L65-L74

should to be changed to something like this

for ( var i = 0, l = _instanceIntersects.length; i < l; i ++ ) {

    var intersect = _instanceIntersects[ i ];
    intersect.instanceId = instanceId;
    intersect.object = this;
    intersects.push( intersect );

}

_instanceIntersects.length = 0;

All 12 comments

That shouldn't be the case. At most a single hit result is added to the intersects array for every instance hit.

But it actually looks like this is incorrect in other ways. The Mesh.raycast function returns a result for every intersection in the model, while the InstancedMesh one only returns one hit per instance. And that one hit may not even be the closest because the results from _mesh.raycast are not sorted so it's really just returning a random intersection of all instances that it hit.

I'd think it should just add all results to the intersections array so that Raycaster can sort the results correctly.

And that one hit may not even be the closest because the results from _mesh.raycast are not sorted so it's really just returning a random intersection of all instances that it hit.

That can easily be fixed by applying the same sort in InstacedMesh.raycast() like Raycaster does. I'm not sure I would return all intersections of an instance. The closest one seems sufficient to me.

@webglzhang What do you think about this topic?

I'm not sure I would return all intersections of an instance. The closest one seems sufficient to me.

@Mugen87
Most of the time it's sufficient, but there are certainly use-cases where all the intersections are needed. I work on a project where they're all needed, that's why I opened this issue in the first place.

Well, it probably won't hurt to add all of them. At least for now I can't think of a downside doing this.

And that one hit may not even be the closest because the results from _mesh.raycast are not sorted so it's really just returning a random intersection of all instances that it hit.

@gkjohnson
Seems like somehow it does return the first hit, though, at least if I play around with this example, it works just fine, as expected:
https://threejs.org/examples/webgl_instancing_raycast.html

So at this point, I feel a little bit lost. I know I wrote in the original post, that I was planning to create a PR for this myself, but I'd certainly need some help from someone who knows what's going on. 馃檹

Seems like somehow it does return the first hit, though, at least if I play around with this example, it works just fine, as expected:

Try to set the side property of the material to THREE.DoubleSide. This will ensure you get two intersections per sphere instance.

Oh, I misunderstood it seems. So as of now, it returns a random intersection / given instance, but it does return the closest instance from the instancedmesh, right?

In the official example it works as expected. However, there might be use cases with overlapping instances where the result is not correct because the first (unsorted) intersection for an instance is returned.

I think it makes sense to return all intersections for the sake of consistency. I think these lines

https://github.com/mrdoob/three.js/blob/19beb8ecc83b8f52de1e00dcfca59fc2ce55078f/src/objects/InstancedMesh.js#L65-L74

should to be changed to something like this

for ( var i = 0, l = _instanceIntersects.length; i < l; i ++ ) {

    var intersect = _instanceIntersects[ i ];
    intersect.instanceId = instanceId;
    intersect.object = this;
    intersects.push( intersect );

}

_instanceIntersects.length = 0;

That looks good to me.

@gkjohnson Great, do you want me to create a PR with this, or do you want to do it yourself? I have no intention to take credit for your work, that's why I'm asking

Great, do you want me to create a PR with this

Go for it!

Was this page helpful?
0 / 5 - 0 ratings