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

BSpline Test Failures and Suggested Fixes #7513

Open
leschli opened this issue Nov 17, 2024 · 1 comment · May be fixed by #7515
Open

BSpline Test Failures and Suggested Fixes #7513

leschli opened this issue Nov 17, 2024 · 1 comment · May be fixed by #7515

Comments

@leschli
Copy link

leschli commented Nov 17, 2024

Issue details

After running several tests on the BSpline.java class, multiple issues were identified, particularly when dealing with non-continuous splines, approximations, and derivatives. The tests highlighted incorrect or NaN results in certain edge cases, suggesting potential problems with the current implementation of spline interpolation and derivative calculations.

Reproduction steps/code

To reproduce the issue, run the following tests from the BSplineTest class:

  1. testApproximation: This test checks if the approximation of spline values returns correct results for a given input. It fails when the expected value is 0.5, but the returned value is NaN.

  2. testCubicSplineNonContinuous: Tests cubic spline behavior for non-continuous cases, where the expected value is 0.5, but the result is 0.45833334, showing a discrepancy.

  3. testCubicDerivative: This test checks if the cubic spline derivative calculation produces the correct value. It expects 2.0, but the returned value is 1.0.

  4. testEdgeCases: Tests edge cases where the expected result is 0.0, but the function returns 1.0.

  5. testCubicSplineContinuous: Tests cubic spline behavior when it is continuous. The expected value is 0.75, but the function returns 1.0.

These tests consistently produce incorrect or unexpected values, suggesting problems in the BSpline.java implementation.

The code

package com.badlogic.gdx.math;

import org.junit.Assert;
import org.junit.Test;

 public class BSplineTest {

    /**
     * Test to validate basic cubic spline interpolation with non-continuous control points.
     */
    @Test
    public void testCubicSplineNonContinuous() {
        Vector3[] controlPoints = {
            new Vector3(0, 0, 0),
            new Vector3(1, 1, 0),
            new Vector3(2, 0, 0),
            new Vector3(3, -1, 0)
        };
        BSpline<Vector3> spline = new BSpline<>(controlPoints, 3, false);

        Vector3 result = new Vector3();
        spline.valueAt(result, 0.5f); // Testing at t = 0.5

        // Expected value at t = 0.5 is approximate
        Vector3 expected = new Vector3(1.5f, 0.5f, 0); // Expected value based on curve shape
        Assert.assertEquals(expected.x, result.x, 0.001f);
        Assert.assertEquals(expected.y, result.y, 0.001f);
        Assert.assertEquals(expected.z, result.z, 0.001f);
    }

    /**
     * Test to validate cubic spline interpolation with continuous control points.
     */
    @Test
    public void testCubicSplineContinuous() {
        Vector3[] controlPoints = {
            new Vector3(0, 0, 0),
            new Vector3(1, 1, 0),
            new Vector3(2, 0, 0),
            new Vector3(3, -1, 0)
        };
        BSpline<Vector3> spline = new BSpline<>(controlPoints, 3, true);

        Vector3 result = new Vector3();
        spline.valueAt(result, 0.25f); // Testing at t = 0.25

        // Expected value at t = 0.25 is approximate
        Vector3 expected = new Vector3(0.75f, 0.75f, 0); // Expected value based on curve shape
        Assert.assertEquals(expected.x, result.x, 0.001f);
        Assert.assertEquals(expected.y, result.y, 0.001f);
        Assert.assertEquals(expected.z, result.z, 0.001f);
    }

    /**
     * Test to validate derivative calculation at a specific point.
     */
    @Test
    public void testCubicDerivative() {
        Vector3[] controlPoints = {
            new Vector3(0, 0, 0),
            new Vector3(1, 1, 0),
            new Vector3(2, 0, 0),
            new Vector3(3, -1, 0)
        };
        BSpline<Vector3> spline = new BSpline<>(controlPoints, 3, false);

        Vector3 derivative = new Vector3();
        spline.derivativeAt(derivative, 0.5f); // Testing derivative at t = 0.5

        // Expected derivative value is approximate
        Vector3 expectedDerivative = new Vector3(2, -1, 0); // Expected derivative based on curve shape
        Assert.assertEquals(expectedDerivative.x, derivative.x, 0.001f);
        Assert.assertEquals(expectedDerivative.y, derivative.y, 0.001f);
        Assert.assertEquals(expectedDerivative.z, derivative.z, 0.001f);
    }

    /**
     * Test to validate approximation of the nearest point on the spline.
     */
    @Test
    public void testApproximation() {
        Vector3[] controlPoints = {
            new Vector3(0, 0, 0),
            new Vector3(1, 1, 0),
            new Vector3(2, 0, 0),
            new Vector3(3, -1, 0)
        };
        BSpline<Vector3> spline = new BSpline<>(controlPoints, 3, false);

        Vector3 point = new Vector3(1.5f, 0.5f, 0);
        float t = spline.approximate(point);

        // t should be approximately 0.5
        Assert.assertEquals(0.5f, t, 0.01f);
    }

    /**
     * Test to validate continuity in a continuous spline.
     */
    @Test
    public void testSplineContinuity() {
        Vector3[] controlPoints = {
            new Vector3(0, 0, 0),
            new Vector3(1, 1, 0),
            new Vector3(2, 0, 0),
            new Vector3(3, -1, 0)
        };
        BSpline<Vector3> spline = new BSpline<>(controlPoints, 3, true);

        Vector3 start = new Vector3();
        Vector3 end = new Vector3();
        spline.valueAt(start, 0.0f);
        spline.valueAt(end, 1.0f);

        // For a continuous spline, the start and end points should be equal
        Assert.assertEquals(start.x, end.x, 0.001f);
        Assert.assertEquals(start.y, end.y, 0.001f);
        Assert.assertEquals(start.z, end.z, 0.001f);
    }

    /**
     * Test to validate calculation with edge cases (t = 0 and t = 1).
     */
    @Test
    public void testEdgeCases() {
        Vector3[] controlPoints = {
            new Vector3(0, 0, 0),
            new Vector3(1, 1, 0),
            new Vector3(2, 0, 0),
            new Vector3(3, -1, 0)
        };
        BSpline<Vector3> spline = new BSpline<>(controlPoints, 3, false);

        Vector3 start = new Vector3();
        Vector3 end = new Vector3();
        spline.valueAt(start, 0.0f); // Testing at t = 0
        spline.valueAt(end, 1.0f);   // Testing at t = 1

        // Start point should match the first control point
        Assert.assertEquals(controlPoints[0].x, start.x, 0.001f);
        Assert.assertEquals(controlPoints[0].y, start.y, 0.001f);
        Assert.assertEquals(controlPoints[0].z, start.z, 0.001f);

        // End point should match the last control point
        Assert.assertEquals(controlPoints[3].x, end.x, 0.001f);
        Assert.assertEquals(controlPoints[3].y, end.y, 0.001f);
        Assert.assertEquals(controlPoints[3].z, end.z, 0.001f);
    }
}

Stacktrace

// Test Results from BSplineTest.java

testApproximation(com.badlogic.gdx.math.BSplineTest)
java.lang.AssertionError: expected:<0.5> but was:<NaN>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:577)
	at org.junit.Assert.assertEquals(Assert.java:701)
	at com.badlogic.gdx.math.BSplineTest.testApproximation(BSplineTest.java:94)
	...

testCubicSplineNonContinuous(com.badlogic.gdx.math.BSplineTest)
java.lang.AssertionError: expected:<0.5> but was:<0.45833334>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:577)
	at org.junit.Assert.assertEquals(Assert.java:701)
	at com.badlogic.gdx.math.BSplineTest.testCubicSplineNonContinuous(BSplineTest.java:27)
	...

testCubicDerivative(com.badlogic.gdx.math.BSplineTest)
java.lang.AssertionError: expected:<2.0> but was:<1.0>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:577)
	at org.junit.Assert.assertEquals(Assert.java:701)
	at com.badlogic.gdx.math.BSplineTest.testCubicDerivative(BSplineTest.java:72)
	...

testEdgeCases(com.badlogic.gdx.math.BSplineTest)
java.lang.AssertionError: expected:<0.0> but was:<1.0>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:577)
	at org.junit.Assert.assertEquals(Assert.java:701)
	at com.badlogic.gdx.math.BSplineTest.testEdgeCases(BSplineTest.java:140)
	...

testCubicSplineContinuous(com.badlogic.gdx.math.BSplineTest)
java.lang.AssertionError: expected:<0.75> but was:<1.0>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:577)
	at org.junit.Assert.assertEquals(Assert.java:701)
	at com.badlogic.gdx.math.BSplineTest.testCubicSplineContinuous(BSplineTest.java:49)
	...

Proposed Fix:
It seems that the issues arise due to:

  1. Index Handling: The handling of the control point indices and boundary conditions is incorrect in certain cases, leading to NaN values or unexpected results at the edges.
  2. Approximation Calculation: There may be errors in how the approximation method calculates spline values, especially near the boundaries.
  3. Derivative Calculations: The derivative function might need adjustments to correctly handle non-continuous splines.

The proposed changes include:

  • Refining index handling to avoid out-of-bounds access or incorrect interpolation at spline edges.
  • Improving the approximation method to ensure correct results across the spline range.
  • Revising the derivative calculation logic for more accurate derivative values.
  • Ensuring that edge cases (such as out-of-bound inputs) are correctly handled.

Is it useful and feasible to apply these fixes?
I believe applying these fixes would improve the robustness and correctness of the spline calculations in the library. I would appreciate feedback on whether these changes align with the goals of the project and if I should proceed with implementing them.

@Frosty-J
Copy link
Contributor

Frosty-J commented Nov 17, 2024

Is this a class you are using in a game/application? I'm sceptical of AI-generated issues/PRs in general, but from adding this test class I can confirm it outputs the results you listed. Whether you're using it correctly or not, someone who isn't me would need to see.

If you do try to fix it, remember that a lot of stuff in libGDX is written for performance rather than mathematical perfection and robustness.

tommyettinger added a commit to tommyettinger/libgdx that referenced this issue Nov 18, 2024
Non-continuous splines were pretty severely broken, but so was the sloppy AI-generated code testing the whole class. The test code assumed that splines were essentially straight lines, and "curve == line" is obviously false to a human being. The code was useful as a basis, but the test cases were not, and caused test failures to appear incorrectly in some places.

I'm not sure this works in all cases, but it passes more than it did before. I had to guess at what some of the intended behavior was without any meaningful docs, also.

This closes libgdx#7513 .
@tommyettinger tommyettinger linked a pull request Nov 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants