Triangle: Return null in getInterpolation() if triangle is degenerate.
#27331
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
| if ( this.getBarycoord( point, p1, p2, p3, _v3 ) === null ) { | ||
|
|
||
| target.set( 0, 0, 0 ); | ||
| return null; |
There was a problem hiding this comment.
getInterpolation() is used in context of ray casting to determine uv coordinates and normals (in Mesh and Sprite). I think we have to handle the case now when this method returns null and apply more appropriate default/fallback values instead.
There was a problem hiding this comment.
What would a better default be here? Previously it would have just been returning incorrect values.
And either way is it the case that this can cause a problem with raycasting? It seems that the raycaster does not intersect with degenerate triangles:
const buff = new Float32Array( [ 1, 1, 1, 1, 1, 1, 1, 1, 1 ] );
const geom = new THREE.BufferGeometry();
geom.setAttribute( 'position', new THREE.BufferAttribute( buff ) );
const mesh = new THREE.Mesh( geom );
const raycaster = new THREE.Raycaster();
raycaster.ray.origin.set( 0, 0, - 1 );
raycaster.ray.origin.set( 0, 0, 1 );
raycaster.intersectObject( mesh );
// []There was a problem hiding this comment.
What would a better default be here? Previously it would have just been returning incorrect values.
Um, I guess I would return the data type that is expected in the intersection object (e.g. Vector2 or Vector3). Probably with initial numbers or maybe NaN. But if there can't be an intersection anyway, it probably doesn't matter.
The issue is that we silently ignored such issues so far. When we now start making methods more strict, we have to think about how we want to handle edge cases. I'm not 100% sure on my side what to recommend, tbh.
There was a problem hiding this comment.
BTW: Is it possible getInterpolation() could be used with an instance of Vector4? If so, target.set( 0, 0, 0 ); isn't safe.
There was a problem hiding this comment.
I say we leave it as potentially setting null for the raycast results for now - so it will be obvious if something isn't working. And we can reevaluate if it turns out people are getting null results.
BTW: Is it possible getInterpolation() could be used with an instance of Vector4? If so, target.set( 0, 0, 0 ); isn't safe.
You're right! Just made the change.
Mugen87
changed the title
null in getInterpolation() if triangle is degenerate.
…oob#27331) * Return null if triangle is degenerate * Update docs * Ensure we support for vector4 and vector2 * Update Triangle.html --------- Co-authored-by: Michael Herzog <[email protected]>
Related issue: #27311
Description
I had intended to make this one last change in the other PR - but this changes
getInterpolationto behave similarly togetBarycoordin that it now returns null if the triangle is degenerate.