Skip to content

Add Matrix4f affine inverse round-trip test#2804

Open
boni-teppanyaki wants to merge 2 commits into
jMonkeyEngine:masterfrom
boni-teppanyaki:new_test_case
Open

Add Matrix4f affine inverse round-trip test#2804
boni-teppanyaki wants to merge 2 commits into
jMonkeyEngine:masterfrom
boni-teppanyaki:new_test_case

Conversation

@boni-teppanyaki
Copy link
Copy Markdown

Summary

Adds an initial Matrix4fTest for com.jme3.math.Matrix4f, which previously had no dedicated test suite.

The test builds a typical affine transform using translation, scale, and rotation via setTransform, inverts it with invert(), multiplies the transform by its inverse with mult(), and verifies the result is approximately Matrix4f.IDENTITY.

Motivation

Matrix4f is used throughout jMonkeyEngine for world/view/projection matrices, camera math, skinning, shadows, and collision transforms. However, it currently has limited direct test coverage. This test introduces a basic mathematical property of affine transforms to exercise several important methods together, and provides a starting point for future, more comprehensive Matrix4f tests.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new unit test for Matrix4f that verifies the inversion of an affine transform containing translation, scale, and rotation. The reviewer suggested enhancing the test's robustness by checking the identity property for both left and right multiplication and adding descriptive failure messages to the assertions for better diagnostics.

Comment on lines +57 to +59
final Matrix4f product = transform.mult(inverse);

assertTrue(product.isSimilar(Matrix4f.IDENTITY, 1e-4f));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To make the test more robust and provide better diagnostics on failure, consider checking the inverse multiplication on both sides (transform * inverse and inverse * transform) and adding a failure message to the assertion that includes the actual result. This is standard practice for inversion tests to ensure the mathematical property holds in both directions.

        final Matrix4f product = transform.mult(inverse);
        assertTrue(product.isSimilar(Matrix4f.IDENTITY, 1e-4f),
                () -> "transform * inverse should be identity, but was:\n" + product);

        final Matrix4f inverseProduct = inverse.mult(transform);
        assertTrue(inverseProduct.isSimilar(Matrix4f.IDENTITY, 1e-4f),
                () -> "inverse * transform should be identity, but was:\n" + inverseProduct);
References
  1. Issues found in test code should be reported with a reduced priority, at most medium.

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 this pull request may close these issues.

1 participant