Lines/Points: Raycast ignores BufferGeometry drawRange and groups

[rock7-dan]

It would appear that the raycast function for Points/Line evaluates all vertices in the geometry, even when only a subset is visible (i.e. backed by BufferGeometry with a drawRange set) - resulting in erroneous hits against non-visible geometry.

Solution would be to clamp the raycast position iterators to the drawRange if there's one specified.

Would you like me to do a PR?

Three.js version

• Dev
• r75
• ...

Browser

• All of them
• Chrome
• Firefox
• Internet Explorer

OS

• All of them
• Windows
• Linux
• Android
• IOS

Hardware Requirements (graphics card, VR Device, ...)

[tschw]

Would you like me to do a PR?

👍 I'd like it. From my experience as a contributor, additions of this kind are usually welcome. The challenge (as always) is to keep it as short and simple as possible, while reasonably fast and complete.

In this particular case, there can be different kinds of geometry (are meshes handled correctly?) and an index buffer and draw ranges present independent from one another - pretty much begs for automated testing, which is rather basic right now. It'd be super awesome if you would extend it in your PR, so we can detect breakage under future changes.

[jimmyland]

Just hit this for meshes; the bug is there too.

[mrdoob]

If we add support for drawRange then we will want to add support for groups too. And that means that Raycaster will have to also check whether the material has flipped normals or not...

Or maybe not, maybe people will be happy with just drawRange...?

[jimmyland]

I would be happy with just drawRange, fwiw. (Intuitively, I would have expected raycaster not to hit faces that are not in the draw range, but I wouldn't necessarily expect it to consider flipped normals or any material properties.)

[mrdoob]

I see. Well, I guess it's part of the geometry after all, and it should be pretty short code. So... sounds good to me!

[tschw]

@mrdoob

If we add support for drawRange then we will want to add support for groups too.

It's actually good when stuff like this comes up. Maybe it's possible to already consider that we may want to add it later while not (yet) implementing it?

As usual, step by step towards completeness...

[mrdoob]

Oh, I'm saying that because RayCaster used to support multi materials. It made the code too complicated so we simplified to geometry only.