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

Add near and far check in intersectRay #658

Merged
merged 9 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -370,10 +370,10 @@ Helper function for use when `indirect` is set to true. This function takes a tr
### .raycast

```js
raycast( ray : Ray, side : FrontSide | BackSide | DoubleSide = FrontSide ) : Array<RaycastHit>
raycast( ray : Ray, side : FrontSide | BackSide | DoubleSide = FrontSide, near : Number = 0, far : Number = Infinity ) : Array<RaycastHit>
```
```js
raycast( ray : Ray, material : Array<Material> | Material ) : Array<RaycastHit>
raycast( ray : Ray, material? : Array<Material> | Material, near : Number = 0, far : Number = Infinity ) : Array<RaycastHit>
```

Returns all raycast triangle hits in unsorted order. It is expected that `ray` is in the frame of the BVH already. Likewise the returned results are also provided in the local frame of the BVH. The `side` identifier is used to determine the side to check when raycasting or a material with the given side field can be passed. If an array of materials is provided then it is expected that the geometry has groups and the appropriate material side is used per group.
Expand All @@ -383,10 +383,10 @@ Note that unlike three.js' Raycaster results the points and distances in the int
### .raycastFirst

```js
raycastFirst( ray : Ray, side : FrontSide | BackSide | DoubleSide = FrontSide ) : RaycastHit
raycastFirst( ray : Ray, side : FrontSide | BackSide | DoubleSide = FrontSide, near : Number = 0, far : Number = Infinity ) : RaycastHit
```
```js
raycastFirst( ray : Ray, material : Array<Material> | Material ) : RaycastHit
raycastFirst( ray : Ray, material : Array<Material> | Material, near : Number = 0, far : Number = Infinity ) : RaycastHit
```

Returns the first raycast hit in the model. This is typically much faster than returning all hits. See [raycast](#raycast) for information on the side and material options as well as the frame of the returned intersections.
Expand Down
21 changes: 17 additions & 4 deletions example/raycast.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
const params = {
raycasters: {
count: 150,
speed: 1
speed: 1,
near: 0,
far: pointDist
},

mesh: {
Expand Down Expand Up @@ -96,6 +98,8 @@
const rcFolder = gui.addFolder( 'Raycasters' );
rcFolder.add( params.raycasters, 'count' ).min( 1 ).max( 1000 ).step( 1 ).onChange( () => updateFromOptions() );
rcFolder.add( params.raycasters, 'speed' ).min( 0 ).max( 20 );
rcFolder.add( params.raycasters, 'near' ).min( 0 ).max( pointDist ).onChange( () => updateFromOptions() );
rcFolder.add( params.raycasters, 'far' ).min( 0 ).max( pointDist ).onChange( () => updateFromOptions() );
rcFolder.open();

const meshFolder = gui.addFolder( 'Mesh' );
Expand Down Expand Up @@ -177,11 +181,16 @@

hitMesh.position.set( pointDist - length, 0, 0 );

cylinderMesh.position.set( pointDist - ( length / 2 ), 0, 0 );
cylinderMesh.scale.set( 1, length, 1 );
const lineLength = res.length ? length - raycaster.near : length - raycaster.near - ( pointDist - raycaster.far );

cylinderMesh.position.set( pointDist - raycaster.near - ( lineLength / 2 ), 0, 0 );
cylinderMesh.scale.set( 1, lineLength, 1 );

cylinderMesh.rotation.z = Math.PI / 2;

hitMesh.visible = res.length > 0;
origMesh.visible = res.length > 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep origMesh visible regardless of whether or not we see a hit. It's a little confusing to see those sphere flicker, I think


},

remove: () => {
Expand All @@ -195,6 +204,9 @@

function updateFromOptions() {

raycaster.near = params.raycasters.near;
raycaster.far = params.raycasters.far;

// Update raycaster count
while ( rayCasterObjects.length > params.raycasters.count ) {

Expand Down Expand Up @@ -261,11 +273,12 @@
const lerp = ( a, b, t ) => a + ( b - a ) * t;
const lerpAmt = ( knots.length - 1 ) / ( 300 - 1 );
const dist = lerp( 0, 2, lerpAmt );
const scale = lerp( 1, 0.2, lerpAmt );

Check warning on line 276 in example/raycast.js

View workflow job for this annotation

GitHub Actions / build (16.x)

'scale' is assigned a value but never used

knots.forEach( c => {

c.scale.set( 1, 1, 1 ).multiplyScalar( scale );
// c.scale.set( 1, 1, 1 ).multiplyScalar( scale );
c.scale.set( 1, 0.1, 1 );

const vec3 = new THREE.Vector3( 0, 1, 0 );
vec3.applyAxisAngle( new THREE.Vector3( 1, 0, 0 ), Math.PI * Math.random() );
Expand Down
8 changes: 4 additions & 4 deletions src/core/MeshBVH.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export class MeshBVH {
}

/* Core Cast Functions */
raycast( ray, materialOrSide = FrontSide ) {
raycast( ray, materialOrSide = FrontSide, near = 0, far = Infinity ) {
agargaro marked this conversation as resolved.
Show resolved Hide resolved

const roots = this._roots;
const geometry = this.geometry;
Expand All @@ -227,7 +227,7 @@ export class MeshBVH {
const materialSide = isArrayMaterial ? materialOrSide[ groups[ i ].materialIndex ].side : side;
const startCount = intersects.length;

raycastFunc( this, i, materialSide, ray, intersects );
raycastFunc( this, i, materialSide, ray, intersects, near, far );

if ( isArrayMaterial ) {

Expand All @@ -246,7 +246,7 @@ export class MeshBVH {

}

raycastFirst( ray, materialOrSide = FrontSide ) {
raycastFirst( ray, materialOrSide = FrontSide, near = 0, far = Infinity ) {

const roots = this._roots;
const geometry = this.geometry;
Expand All @@ -261,7 +261,7 @@ export class MeshBVH {
for ( let i = 0, l = roots.length; i < l; i ++ ) {

const materialSide = isArrayMaterial ? materialOrSide[ groups[ i ].materialIndex ].side : side;
const result = raycastFirstFunc( this, i, materialSide, ray );
const result = raycastFirstFunc( this, i, materialSide, ray, near, far );
if ( result != null && ( closestResult == null || result.distance < closestResult.distance ) ) {

closestResult = result;
Expand Down
20 changes: 11 additions & 9 deletions src/core/cast/raycast.template.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ import { BufferStack } from '../utils/BufferStack.js';
import { intersectTris } from '../utils/iterationUtils.generated.js';
import { intersectTris_indirect } from '../utils/iterationUtils_indirect.generated.js';

export function raycast/* @echo INDIRECT_STRING */( bvh, root, side, ray, intersects ) {
export function raycast/* @echo INDIRECT_STRING */( bvh, root, side, ray, intersects, near, far ) {

BufferStack.setBuffer( bvh._roots[ root ] );
_raycast( 0, bvh, side, ray, intersects );
_raycast( 0, bvh, side, ray, intersects, near, far );
BufferStack.clearBuffer();

}

function _raycast( nodeIndex32, bvh, side, ray, intersects ) {
function _raycast( nodeIndex32, bvh, side, ray, intersects, near, far ) {

const { float32Array, uint16Array, uint32Array } = BufferStack;
const nodeIndex16 = nodeIndex32 * 2;
Expand All @@ -24,27 +24,29 @@ function _raycast( nodeIndex32, bvh, side, ray, intersects ) {

/* @if INDIRECT */

intersectTris_indirect( bvh, side, ray, offset, count, intersects );
// It might happen that a triangle less distant than 'near' is selected here. Should we add the near check here too? is it worth it? It might slow down to handle really rare cases
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might happen that a triangle less distant than 'near' is selected here. Should we add the near check here too? is it worth it? It might slow down to handle really rare cases

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The triangle hits will also need to be filtered so that the results returned by the raycast function are accurate. Otherwise its the case that we may have 10 triangles in a large leaf node such that the near hit point falls within the ray range but the triangles within do not.

We could filter them afterward but it probably makes sense to just not add them to the intersections array here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like we can remove this line now, too?


intersectTris_indirect( bvh, side, ray, offset, count, intersects, near, far );

/* @else */

intersectTris( bvh, side, ray, offset, count, intersects );
intersectTris( bvh, side, ray, offset, count, intersects, near, far );

/* @endif */

} else {

const leftIndex = LEFT_NODE( nodeIndex32 );
if ( intersectRay( leftIndex, float32Array, ray ) ) {
if ( intersectRay( leftIndex, float32Array, ray, near, far ) ) {

_raycast( leftIndex, bvh, side, ray, intersects );
_raycast( leftIndex, bvh, side, ray, intersects, near, far );

}

const rightIndex = RIGHT_NODE( nodeIndex32, uint32Array );
if ( intersectRay( rightIndex, float32Array, ray ) ) {
if ( intersectRay( rightIndex, float32Array, ray, near, far ) ) {

_raycast( rightIndex, bvh, side, ray, intersects );
_raycast( rightIndex, bvh, side, ray, intersects, near, far );

}

Expand Down
18 changes: 9 additions & 9 deletions src/core/cast/raycastFirst.template.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@

const _xyzFields = [ 'x', 'y', 'z' ];

export function raycastFirst/* @echo INDIRECT_STRING */( bvh, root, side, ray ) {
export function raycastFirst/* @echo INDIRECT_STRING */( bvh, root, side, ray, near, far ) {

BufferStack.setBuffer( bvh._roots[ root ] );
const result = _raycastFirst( 0, bvh, side, ray );
const result = _raycastFirst( 0, bvh, side, ray, near, far );
BufferStack.clearBuffer();

return result;

}

function _raycastFirst( nodeIndex32, bvh, side, ray ) {
function _raycastFirst( nodeIndex32, bvh, side, ray, near, far ) {

const { float32Array, uint16Array, uint32Array } = BufferStack;
let nodeIndex16 = nodeIndex32 * 2;
Expand All @@ -29,11 +29,11 @@

/* @if INDIRECT */

return intersectClosestTri_indirect( bvh, side, ray, offset, count );
return intersectClosestTri_indirect( bvh, side, ray, offset, count, near, far );

/* @else */

return intersectClosestTri( bvh, side, ray, offset, count );
return intersectClosestTri( bvh, side, ray, offset, count, near, far );

Check warning

Code scanning / CodeQL

Unreachable statement Warning

This statement is unreachable.

/* @endif */

Expand All @@ -60,8 +60,8 @@

}

const c1Intersection = intersectRay( c1, float32Array, ray );
const c1Result = c1Intersection ? _raycastFirst( c1, bvh, side, ray ) : null;
const c1Intersection = intersectRay( c1, float32Array, ray, near, far );
const c1Result = c1Intersection ? _raycastFirst( c1, bvh, side, ray, near, far ) : null;

// if we got an intersection in the first node and it's closer than the second node's bounding
// box, we don't need to consider the second node because it couldn't possibly be a better result
Expand All @@ -84,8 +84,8 @@

// either there was no intersection in the first node, or there could still be a closer
// intersection in the second, so check the second node and then take the better of the two
const c2Intersection = intersectRay( c2, float32Array, ray );
const c2Result = c2Intersection ? _raycastFirst( c2, bvh, side, ray ) : null;
const c2Intersection = intersectRay( c2, float32Array, ray, near, far );
const c2Result = c2Intersection ? _raycastFirst( c2, bvh, side, ray, near, far ) : null;

if ( c1Result && c2Result ) {

Expand Down
8 changes: 3 additions & 5 deletions src/core/utils/intersectUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* This function performs intersection tests similar to Ray.intersectBox in three.js,
* with the difference that the box values are read from an array to improve performance.
*/
export function intersectRay( nodeIndex32, array, ray ) {
export function intersectRay( nodeIndex32, array, ray, near, far ) {

let tmin, tmax, tymin, tymax, tzmin, tzmax;

Expand Down Expand Up @@ -67,14 +67,12 @@ export function intersectRay( nodeIndex32, array, ray ) {

if ( ( tmin > tzmax ) || ( tzmin > tmax ) ) return false;

// if ( tzmin > tmin || tmin !== tmin ) tmin = tzmin; // Uncomment this line if add the distance check
if ( tzmin > tmin || tmin !== tmin ) tmin = tzmin;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that this line is in the condition tmin !== tmin 😅. Likely some C-era method for checking for NaN I'd think. Might be worth fixing in three's implementation for the sake of clarify.


if ( tzmax < tmax || tmax !== tmax ) tmax = tzmax;

//return point closest to the ray (positive side)

if ( tmax < 0 ) return false;

return true;
return tmin <= far && tmax >= near;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work correctly if the ray origin is within the box? I'm having trouble visualizing the logic here. It may be easiest to understand if we just return a distance here (null if no hit) and check the distance against near / far in the calling function.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I see this makes the other code a bit more messy. Maybe this is easier to understand:

const hitDist = tmin >= 0 ? tmin : tmax;
return hitDist > near && hitDist < far;

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a good change to make to make thins more clearly with three.js' version that returns a this.at( tmin >= 0 ? tmin : tmax, target ) as the hit point


}
12 changes: 6 additions & 6 deletions src/core/utils/iterationUtils.template.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,27 @@
import { intersectTri } from '../../utils/ThreeRayIntersectUtilities.js';
import { setTriangle } from '../../utils/TriangleUtilities.js';

export function intersectTris/* @echo INDIRECT_STRING */( bvh, side, ray, offset, count, intersections ) {
export function intersectTris/* @echo INDIRECT_STRING */( bvh, side, ray, offset, count, intersections, near, far ) {

const { geometry, _indirectBuffer } = bvh;
for ( let i = offset, end = offset + count; i < end; i ++ ) {

/* @if INDIRECT */

let vi = _indirectBuffer ? _indirectBuffer[ i ] : i;
intersectTri( geometry, side, ray, vi, intersections );
intersectTri( geometry, side, ray, vi, intersections, near, far );

/* @else */

intersectTri( geometry, side, ray, i, intersections );
intersectTri( geometry, side, ray, i, intersections, near, far );

/* @endif */

}

}

export function intersectClosestTri/* @echo INDIRECT_STRING */( bvh, side, ray, offset, count ) {
export function intersectClosestTri/* @echo INDIRECT_STRING */( bvh, side, ray, offset, count, near, far ) {

const { geometry, _indirectBuffer } = bvh;
let dist = Infinity;
Expand All @@ -32,11 +32,11 @@
let intersection;
/* @if INDIRECT */

intersection = intersectTri( geometry, side, ray, _indirectBuffer ? _indirectBuffer[ i ] : i );
intersection = intersectTri( geometry, side, ray, _indirectBuffer ? _indirectBuffer[ i ] : i, undefined, near, far );
Fixed Show fixed Hide fixed
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: lets use null here instead of undefined.


/* @else */

intersection = intersectTri( geometry, side, ray, i );
intersection = intersectTri( geometry, side, ray, i, undefined, near, far );

/* @endif */

Expand Down
4 changes: 2 additions & 2 deletions src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ export class MeshBVH {

constructor( geometry: BufferGeometry, options?: MeshBVHOptions );

raycast( ray: Ray, materialOrSide: Side | Array<Material> | Material ): Array<Intersection>
raycast( ray: Ray, materialOrSide?: Side | Array<Material> | Material, near?: number, far?: number ): Array<Intersection>

raycastFirst( ray: Ray, materialOrSide: Side | Array<Material> | Material ): Intersection;
raycastFirst( ray: Ray, materialOrSide?: Side | Array<Material> | Material, near?: number, far?: number ): Intersection;

intersectsSphere( sphere: Sphere ): boolean;

Expand Down
15 changes: 12 additions & 3 deletions src/utils/ExtensionUtilities.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Ray, Matrix4, Mesh } from 'three';
import { Ray, Matrix4, Mesh, Vector3 } from 'three';
import { convertRaycastIntersect } from './GeometryRayIntersectUtilities.js';
import { MeshBVH } from '../core/MeshBVH.js';

const ray = /* @__PURE__ */ new Ray();
const direction = /* @__PURE__ */ new Vector3();
const tmpInverseMatrix = /* @__PURE__ */ new Matrix4();
const worldScale = /* @__PURE__ */ new Vector3();
const origMeshRaycastFunc = Mesh.prototype.raycast;

export function acceleratedRaycast( raycaster, intersects ) {
Expand All @@ -15,10 +17,17 @@ export function acceleratedRaycast( raycaster, intersects ) {
tmpInverseMatrix.copy( this.matrixWorld ).invert();
ray.copy( raycaster.ray ).applyMatrix4( tmpInverseMatrix );

this.getWorldScale( worldScale );
direction.copy( ray.direction ).multiply( worldScale );

Comment on lines +20 to +21
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scale axes change based on the rotation of the model so we'll want to make sure we account for the rotations when scaling here, as well. I think this should be correct:

direction.copy( ray.direction ).applyMatrix4( this.matrixWorld );

// we divide by the ray.direction just in case it's non normalized
const scaleFactor = direction.length() / ray.direction.length();

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be addressed, as well.

const scaleFactor = direction.length();
const near = raycaster.near / scaleFactor;
const far = raycaster.far / scaleFactor;

const bvh = this.geometry.boundsTree;
if ( raycaster.firstHitOnly === true ) {

const hit = convertRaycastIntersect( bvh.raycastFirst( ray, this.material ), this, raycaster );
const hit = convertRaycastIntersect( bvh.raycastFirst( ray, this.material, near, far ), this, raycaster );
if ( hit ) {

intersects.push( hit );
Expand All @@ -27,7 +36,7 @@ export function acceleratedRaycast( raycaster, intersects ) {

} else {

const hits = bvh.raycast( ray, this.material );
const hits = bvh.raycast( ray, this.material, near, far );
for ( let i = 0, l = hits.length; i < l; i ++ ) {

const hit = convertRaycastIntersect( hits[ i ], this, raycaster );
Expand Down
4 changes: 3 additions & 1 deletion src/utils/GeometryRayIntersectUtilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ export function convertRaycastIntersect( hit, object, raycaster ) {
hit.distance = hit.point.distanceTo( raycaster.ray.origin );
hit.object = object;

if ( hit.distance < raycaster.near || hit.distance > raycaster.far ) {
if ( hit.distance < raycaster.near || hit.distance > raycaster.far ) { // TODO probably this can be removed
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed if we can guarantee the function only returns triangle hits within the range of the near and far values. I don't recall if there are any yet but if not it would be nice to add a few tests that check to make sure the correct results are returned when the raycaster near and far are modified compared to the three.js results.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets go ahead and remove this now that we have tests!


console.error( "this should never happened now" ); // TODO remove this after test

return null;

Expand Down