GitBrincie212 / Apel-Mod

Apel is a library that brings particle animations to the table with flexible behaviour and a clean developer interface. It promises also lots of predefined shapes & paths to help the developer on their particle scene
Other
2 stars 1 forks source link

Triangle check for valid vertices is incorrect #58

Closed DarthSharkie closed 3 weeks ago

DarthSharkie commented 1 month ago

When fixing the line leak problem, I tested all the shapes that render themselves with multiple lines. Triangle and tetrahedron gave me problems, though, since they incorrectly flagged sets of vertices as invalid for their shapes.

For example, constructing the following triangle was disallowed:

        ParticleTriangle triangle = ParticleTriangle.builder()
                .vertex1(new Vector3f(2f))
                .vertex2(new Vector3f(3f))
                .vertex3(new Vector3f(1f, 4f, 6f))
                .amount(7)
                .particleEffect(ParticleTypes.END_ROD)
                .build();

These three points are not collinear, which is fairly easy to see: the first two points are multiples of the unit vector (1, 1, 1), while the third is not. However, the algorithm used detects them as not forming a valid triangle. The algorithm is this:

        float dotProduct1 = vertex3.dot(new Vector3f(vertex1).cross(vertex2));
        if (dotProduct1 == 0) {
            throw new IllegalArgumentException("Provided vertices do not produce a triangle");
        }

The operation performed there, if the vertices are named $a$, $b$, and $c$, with corresponding vectors of $\vec{A}$, $\vec{B}$, and $\vec{C}$, is the scalar triple product. If it evaluates to zero, then the three vectors are co-planar (this is checked in the tetrahedron, but using the fourth vertex, not the origin, as the starting point for each vector). Since $\vec{A}$ and $\vec{B}$ are clearly collinear, the three vectors must be co-planar.

The triangle check should only verify that $||\vec{AB} \times \vec{AC}|| \neq 0$. This proves two vectors are not collinear, which is all that is needed to prove a valid triangle.

DarthSharkie commented 1 month ago

I have a fix mostly ready, just adding a couple tests so it stays fixed.

DarthSharkie commented 3 weeks ago

Fixed in https://github.com/GitBrincie212/Apel-Mod/commit/6851e8dd40d259e36795968fbcf7ad9bc5701ce7