-
-
Notifications
You must be signed in to change notification settings - Fork 246
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
Changes from 6 commits
0b05e62
db3a65d
3d00e02
eefa65e
3ab2009
17b7868
36eb4d1
8758246
62fde15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ); | ||
|
||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just noticed that this line is in the condition |
||
|
||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 ); | ||
|
||
|
||
/* @else */ | ||
|
||
intersection = intersectTri( geometry, side, ray, i ); | ||
intersection = intersectTri( geometry, side, ray, i, undefined, near, far ); | ||
|
||
/* @endif */ | ||
|
||
|
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 ) { | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ); | ||
|
@@ -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 ); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
||
|
There was a problem hiding this comment.
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