SVGLoader: Implement ellipse transformations.

[nkrkv]

Related issue: #21330

Currently the following is not implemented:

1. Ellipses that are rotated on their own or within a group
2. Ellipse arcs that are rotated on their own or within a group
3. Ellipses that are rotated+skewed on their own or within a group
4. Ellipse arcs that are rotated+skewed on their own or within a group

This PR implements points (1), (2), and (3), leaving (4) unimplemented yet. I’m not smart enough to solve this math riddle 🤯 However, this fourth case is arguably quite rare and in any case the improvement is purely additive.

Test fixture (Inkscape)

Before

After

The coral pacmans (skewed 30° by X) are still more like Kandinsky art, but the full-circle and full-ellipse are correct. Other groups are correct too.

[Mugen87]

@yomboprime Does this look good to you?

[yomboprime]

@yomboprime Does this look good to you?

Yes. I've read the commit and seems good. Although I can't verify the maths in depth.

[WestLangley]

This is testing if the columns are orthogonal. Is this function reliable in practice given there will be round off?

[yomboprime]

It will give false positives, probably only will return false if the matrix is identity. The only solution I think of is testing against a very small angle.

[nkrkv]

In the case of a false-positive it will cause the compute-intensive branch which, nevertheless, gives the correct results for full circles and ellipses.

In practice, for a pure rotation it does not give false-positives since rotation matrices have their conjugate-diagonal set to sin θ -sin θ.

The only case to worry about is a long chain of nested group matrix multiplications. If we should handle it, I can update the PR to test against some Epsilon, not zero. Is it a good idea?

[yomboprime]

I think any arbitrary rotation can seem as skewed currently.

It is not elegant to introduce deltas but I guess the skewing can be detected with a big angle (1 or 2 degrees) as it is a very specific use case only for the ellipse.

[nkrkv]

Agree. I’ve added a tolerance a new commit. Please, take a look.

[WestLangley]

I'm inclined to think 1/100 of a degree would be more appropriate. I do not think you are going to get 1 degree round off error.

@Mugen87 What do you think?

[Mugen87]

I would normally start to test against Number.EPSILON unless there is a need for a greater threshold. I'm not sure I understand why this is the case for isTransformSkewed().

[nkrkv]

Possible errors can be estimated with some hard-core math and reasoning and I don’t think I’m able to do it. However, yes it sounds that an error should be a magnitude lower than 1°.

I’ve committed a comparison against Number.EPSILON. EPSILON is of order 1e-16, 1°/100 is of order 1e-8.

[yomboprime]

I just ran this test. It always gets success. With or without random increments, and variations testing against EPSILON or 0, indicates that a matrix constructed with m.rotate() always has basisDot equal to 0.

So the conclusion is that using Number.EPSILON as threshold works just fine.

const m = new Matrix3();
let error = false;
const useRandom = 1;

for ( let a = 0; a < 2.1 * Math.PI; a += 0.00000001 + Math.random() * 0.00000001 * useRandom ) {

    m.identity().rotate( a );

    if ( isTransformSkewed( m ) ) {
        error = true;
        break;
    }

}

console.log( error ? "Error." : "Success." );

[nkrkv]

Nice! So, are we ready for the merge?