Raycaster.setFromCamera: Position the ray origin such that it is on the camera near plane

[gkjohnson]

Fix #28026

Description

Adjust Raycaster.setFromCamera so the origin is placed on the camera near plane.

[github-actions[bot]]

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev	Filesize PR	Diff
671.2 kB (166.4 kB)	671.2 kB (166.4 kB)	-35 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev	Filesize PR	Diff
451.1 kB (109 kB)	451.1 kB (109 kB)	+0 B

[sciecode]

Although the reason for the PR is sound, this is not how we should address it.

We don't make changes to the ray origin of the raycaster, we change the places where we are erroneously projecting the raycaster.ray to near & far plane.

e.g https://github.com/mrdoob/three.js/blob/2ff77e4b335e31c108aac839a07401664998c730/src/objects/Mesh.js#L167-L175

As you can see, we are already trying to reproject _ray towards the near plane, this is just done incorrectly. Every place where we are referencing raycaster.near raycaster.far, we are treating those as distance, where they should be treated as depth. And even if we solve the wrong near recast, the far plane is also "rounded", we also need to recast the distance to the far plane to make the following computations.

[sciecode]

The easiest way of fixing this without major reworks, is to keep raycaster.near & raycaster.far as being distances. Raycaster docs already states that.

But when we use raycaster.setFromCamera we also modify the near/far to match the correct distance to both clipping planes for that specific ray direction, this will make everything else fall into place.

I can't say whether or not changing Raycaster's near/far is considered good practice in the context of the API. Others can comment on that.

[sciecode]

Visual diagram - in case my explanation isn't clear:

Apologies for my horrible digital art attempt :)

[gkjohnson]

As you can see, we are already trying to reproject _ray towards the near plane, this is just done incorrectly.

There's nothing erroneous about the current Raycaster near/far implementation - it's working as intended. Raycaster has many other use cases beyond being used with a camera projection. The near and far value are designed to specify the distance along the ray, useful for things like only intersecting things nearby a VR controller for interaction, for example.

Setting the near / far to projected distances in setFromCamera is something I've considered but it will not work for negative orthographic camera near values which I'm feeling should be valid.

[sciecode]

I'm fairly certain I wasn't clear, I apologize. I'll try to explain the 2 different problems here.

The first one is that setting the raycaster origin projected to the near plane on setFromCamera would break the assumptions we are making in other parts of the code. As is shown in the snippet:

https://github.com/mrdoob/three.js/blob/2ff77e4b335e31c108aac839a07401664998c730/src/objects/Mesh.js#L167-L170 We are already attempting to do a similar thing inside some raycast functions, which means that we would first project the origin towards the near plane, and afterwards we would move along the ray direction yet again. Now, the confusion might be here, with the default raycaster.near value of 0, this doesn't make a difference. With other values - it breaks the logic. All I'm saying is that this PR isn't compatible with other functions reliant on Raycaster.

Aside from that, there is arguably a different problem, one might expect that after calling setFromCamera the raycaster would be confined to detecting only things within that camera's bounds, but that is incorrect, as we don't make any changes to near/far to guarantee that assumption. And, if the user simply tries to copy the camera's near/far, he would run into the problem shown in the diagram above. Which can be validated in this example.

Since it might not be obvious to users how to guarantee frustum confinement, we can either implement it by default on setFromCamera or provide as an optional boolean. Eitherway, this doesn't remove the ability for the user to change it to other distances or use the raycaster for other uses. It only accomodates a reasonable assumption about having the proper bounds clipping when calling setFromCamera.

[gkjohnson]

We are already attempting to do a similar thing inside some raycast functions, which means that we would first project the origin towards the near plane, and afterwards we would move along the ray direction yet again. Now, the confusion might be here, with the default raycaster.near value of 0, this doesn't make a difference. With other values - it breaks the logic.

This feels like manufacturing a problem. Near / far raycaster values were not added with the intent of solving this camera bounds issue (as in setting them directly doesn't work). I guess what you're saying is that peoples existing projects may incorrectly set the near value to fix this and now this will double the distance but I'm not convinced this is happening in real cases. Setting the near value in the setFromCamera also newly overrides any user settings. Either way in some cases this is a breaking change users may need to be aware of. Generally I don't agree the approach in this PR is incompatible with these other settings, though. It's just placing the ray origin at the near plane which is what I expected when using this. You're free to disagree but there's no objectively right answer here.

_ray.copy( raycaster.ray ).recast( raycaster.near ); 

~I see now that negative near values will work and~ I agree that clamping the intersections to far values is a good addition. I'm happy to change this PR to set the near and far values to ensure only visible items are intersected including the far plane but I'll wait for other feedback before putting more work in.

cc @Mugen87 @mrdoob

[gkjohnson]

_ray.copy( raycaster.ray ).recast( raycaster.near );

~This recast logic literally only happens for Meshes, though. Every other object will potentially fail to find intersections if a negative near value is used...~

Negative values are not supported for raycaster.near

[sciecode]

Ok, I decided to review what's happening after a night of sleep, and I did misinterpret the original purpose for the PR. My bad, that's probably why we had apparently conflicting views about what's going on. I really wasn't trying to "manufacture a problem" haha.

I interpreted the change of raycaster.ray.origin as a way to simply account for the camera frustum bounds. But you are proposing a change in the API itself, making so the meaning of raycaster.near/far after setFromCamera would be changed after the PR, this is the part that confused me. With that mindset, then yes. There is no underlying conflict with the implementation, just a breaking change in that near/far are to be considered from the near plane.

With that being said, If I understand correctly, the distance for intersects would no longer be relative to the camera, which might be a bigger problem than the near/far changes itself.

Eitherway it is subjective, whether or not my proposal is better or not than yours. As both introduce some form of breaking change.

~The only thing I didn't understand is this statement:~

Near / far raycaster values were not added with the intent of solving this camera bounds issue (as in setting them directly doesn't work)

~Why setting them directly doesn't work? I have used this before in many cases and even used in the jsFiddle example I linked, and everything still worked as expected.~

edit: Ok, you probably mean copying the camera's near/far straight into raycaster's near/far, which is exactly what I was trying to demonstrate. Yes, that doesn't work. The thing is, the same problem will happen if someone wants to get the original distance to camera after this PR. Users might naively try to add the camera's near depth to account for the offset, which would be incorrect.

Personally, I think introducing a boolean parameter to setFromCamera and adjusting raycaster near/far for the correct frustum bounds would be the path with least complications for the users. But I'll stop commenting now, others can decide on the best path forward.

Sorry for the confusion.