Repository navigation
Possible floating point precision issue in corner case using BoundingBox.clip() #2556
Description
While working on getting jme compiled to javascript I've encountered an issue: clicks went through some ui elements (lemur).
After a very long debugging session i've isolated the problem to a floating point precision issue in BoundingBox.clip that happens when bounding boxes have zExtent=0.
jmonkeyengine/jme3-core/src/main/java/com/jme3/bounding/BoundingBox.java
Lines 1015 to 1073 in 8cc3086
| private boolean clip(float denom, float numerator, float[] t) { | |
| // Return value is 'true' if line segment intersects the current test | |
| // plane. Otherwise, 'false' is returned, in which case the line segment | |
| // is entirely clipped. | |
| if (denom > 0.0f) { | |
| // This is the old if statement... | |
| // if (numerator > denom * t[1]) { | |
| // | |
| // The problem is that what is actually stored is | |
| // numerator/denom. In non-floating point, this math should | |
| // work out the same but in floating point there can | |
| // be subtle math errors. The multiply will exaggerate | |
| // errors that may have been introduced when the value | |
| // was originally divided. | |
| // | |
| // This is especially true when the bounding box has zero | |
| // extents in some plane because the error rate is critical. | |
| // comparing a to b * c is not the same as comparing a/b to c | |
| // in this case. In fact, I tried converting this method to | |
| // double and the and the error was in the last decimal place. | |
| // | |
| // So, instead, we now compare the divided version to the divided | |
| // version. We lose some slight performance here as divide | |
| // will be more expensive than the divide. Some microbenchmarks | |
| // show divide to be 3x slower than multiple on Java 1.6. | |
| // BUT... we also saved a multiply in the non-clipped case because | |
| // we can reuse the divided version in both if checks. | |
| // I think it's better to be right in this case. | |
| // | |
| // Bug that I'm fixing: rays going right through quads at certain | |
| // angles and distances because they fail the bounding box test. | |
| // Many Bothans died bring you this fix. | |
| // -pspeed | |
| float newT = numerator / denom; | |
| if (newT > t[1]) { | |
| return false; | |
| } | |
| if (newT > t[0]) { | |
| t[0] = newT; | |
| } | |
| return true; | |
| } else if (denom < 0.0f) { | |
| // Old if statement... see above | |
| // if (numerator > denom * t[0]) { | |
| // | |
| // Note though that denom is always negative in this block. | |
| // When we move it over to the other side we have to flip | |
| // the comparison. Algebra for the win. | |
| float newT = numerator / denom; | |
| if (newT < t[0]) { | |
| return false; | |
| } | |
| if (newT < t[1]) { | |
| t[1] = newT; | |
| } | |
| return true; | |
| } else { | |
| return numerator <= 0.0; | |
| } |
As you can see, in this code there is a note by @pspeed42 regarding a similar issue that he fixed in the past.
My assumption is that, the way floating point numbers are represented in javascript (doubles?) caused this issue to reappear when the engine was compiled with teavm. I've ended up fixing it in this way.
I’m opening this issue to make it searchable in the future, in case this corner case ever arises during normal use of jME.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsNo status
Activity
coderabbitai commentedon Mar 3, 2026
Coding Plan
Summary
EPSILON = 1e-6fconstant in BoundingBox, matching the user's proven fix and standard geometric tolerance recommendationsclip()to use epsilon-based thresholds, preventing accumulated precision errors from causing false missesDesign Choices
Design Choice 1: Should the fix use a local epsilon constant or reuse FastMath constants?
Options Considered:
FastMath.FLT_EPSILON- machine epsilon, may be too small for accumulated errorsFastMath.ZERO_TOLERANCE- may be too large, could cause false positivesEPSILON = 1e-6fmatching the user's proven fixChosen Option: 3
Rationale: A local constant provides the appropriate tolerance for geometric intersection tests (per web search recommendations of 1e-6 to 1e-5 for single-precision), matches the user's tested fix, and follows the precedent of
BoundingSphere.RADIUS_EPSILON.Design Choice 2: Should this fix be applied universally or only for JavaScript/TeaVM builds?
Options Considered:
Chosen Option: 2
Rationale: JME's established pattern is to apply precision fixes universally (e.g.,
Vector3f.length()uses double-precision arithmetic on all platforms). Platform-specific code is reserved for system delegates, not core math operations.💡 User Tips
Regenerate the plan with different choices with
@coderabbitai <feedback>.Implementation Steps
Phase 1: Apply Epsilon-Based Precision Fix to clip() Method
This phase modifies the
BoundingBox.clip()method to use epsilon-based comparisons instead of exact floating-point comparisons. The changes prevent rays from incorrectly passing through or missing zero-extent bounding boxes due to accumulated floating-point errors.Task 1: Add Epsilon Constant to BoundingBox Class
Add a private static epsilon constant for use in geometric intersection tolerance checks.
private static final float EPSILON = 1e-6f;as a class-level constant injme3-core/src/main/java/com/jme3/bounding/BoundingBox.javaTask 2: Modify clip() Method Comparisons
Update the
clip()method to use epsilon-based comparisons that tolerate small floating-point errors while maintaining algorithm correctness.jme3-core/src/main/java/com/jme3/bounding/BoundingBox.java, modify theclip()method (lines 1015-1074):if (denom > 0.0f)→if (denom > EPSILON)to better handle near-parallel rayselse if (denom < 0.0f)→else if (denom < -EPSILON)for symmetryif (newT > t[1])→if (newT > t[1] + EPSILON)andif (newT > t[0])→if (newT > t[0] + EPSILON)if (newT < t[0])→if (newT < t[0] - EPSILON)andif (newT < t[1])→if (newT < t[1] - EPSILON)return numerator <= 0.0;→return numerator <= EPSILON;🤖 Prompt for AI agents
Phase 2: Add Unit Tests for clip() Method Edge Cases
Create comprehensive tests to verify the fix works correctly and prevent regression. The
clip()method currently has no test coverage.Task 1: Create Test Cases for Zero-Extent Bounding Box Intersections
Add unit tests that specifically exercise the problematic scenarios with flat bounding boxes.
jme3-core/src/test/java/com/jme3/bounding/TestBoundingBox.javaTask 2: Add Regression Tests for Normal Bounding Box Behavior
Ensure the epsilon changes don't break normal (non-zero-extent) bounding box intersection behavior.
jme3-core/src/test/java/com/jme3/bounding/TestBoundingBox.javafor:🤖 Prompt for AI agents
Research
The
BoundingBoxclass injme3-core/src/main/java/com/jme3/bounding/BoundingBox.javaimplements axis-aligned bounding box collision detection. Theclip()method (lines 1015-1074) is a private helper implementing the Liang-Barsky line clipping algorithm for ray-box intersection. It's called 6 times per intersection test (once per box face) fromcollideWithRay()methods. The method already contains a previous precision fix by@pspeed42that switched from multiplication-based to division-based comparisons, with extensive comments explaining why zero-extent boxes are particularly sensitive to floating-point errors. Currently, no unit tests exist for theclip()method.🚀 Next Steps
🤖 All AI agent prompts combined
💡 Iterate on the plan with:
@coderabbitai <feedback>💬 Have feedback or questions? Drop into our discord!