add decompose function

[chrvadala]

No description provided.

[peebeebee]

Hi Chrvadala, are you still working on this?
Are you open to let me check this if I can do an implementation and a PRQ?

I saw that https://www.npmjs.com/package/transformation-matrix-js is deprecated, and I'm looking into an alternative, but we do need the decompose function.

[chrvadala]

This is great feature that I'd like to have, but actually I don't have enough time to work on it.
A PR is welcome ;-)

[chrvadala]

Hi @peebeebee, do you have some progress on this?

[peebeebee]

No, I’d normally do it during my work hours, since it’s work related, but we’ve found a way around the problem sadly. Still on my to-do list though!

…

On 13 Jan 2019, at 23:33, Christian ***@***.***> wrote: Hi @peebeebee <https://github.com/peebeebee>, do you have some progress on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIRVJ9EAHeMngwQ2zmE3kiPbLxurwjJks5vC7Q4gaJpZM4MI5If>.

[chrvadala]

you’re right, are you open to submit a PR?

[chrvadala]

https://github.com/WebKit/webkit/blob/ef68f7e8a9eb4f83d68f69aca80f3a52ea0eecb0/Source/WebCore/platform/graphics/transforms/AffineTransform.cpp#L397

[hillin]

We've been using the decompose method in PR #60 for a while, but unfortunately it cannot handle transform with flip/mirror/reflect or whatever it is called. Here is our take to implement this method:

export interface Transform {
    readonly translate: Point;
    readonly rotation: number;
    readonly scale: Vector;
}

export class Transform {
    static toMatrix(transform: Transform): Matrix {
        return matrix.compose(
            matrix.translate(transform.translate.x, transform.translate.y),
            matrix.rotate(transform.rotation),
            matrix.scale(transform.scale.x, transform.scale.y));
    }
}


/**
 * Decompose a matrix into rigid transform components (i.e. translate, rotation and scale).
 * Note unlike SlideTransform, scale contains flip, so its components can be negative.
 * @param m
 */
export function decompose(m: Matrix, horizontalFlip = false, verticalFlip = false): Transform {

    // Remove flip from the matrix first - flip could be incorrectly interpreted as 
    // rotations (e.g. h-flip + v-flip = rotate by 180 degrees).
    if (horizontalFlip) {
        if (verticalFlip) {
            m = matrix.compose(m, matrix.scale(-1, -1));
        } else {
            m = matrix.compose(m, matrix.scale(-1, 1));
        }
    } else if (verticalFlip) {
        m = matrix.compose(m, matrix.scale(1, -1));
    }

    const a = m.a, b = m.b;
    const c = m.c, d = m.d;
    let scaleX: number, scaleY: number, rotation: number;
    
    if (a !== 0 || c !== 0) {
        const hypotAc = Math.hypot(a, c);
        scaleX = hypotAc;
        scaleY = (a * d - b * c) / hypotAc;
        const acos = Math.acos(a / hypotAc);
        rotation = c > 0 ? -acos : acos;
    } else if (b !== 0 || d !== 0) {
        const hypotBd = Math.hypot(b, d);
        scaleX = (a * d - b * c) / hypotBd;
        scaleY = hypotBd;
        const acos = Math.acos(b / hypotBd);
        rotation = Math.PI / 2 + (d > 0 ? -acos : acos);
    } else {
        scaleX = 0;
        scaleY = 0;
        rotation = 0;
    }

    // put the flip factors back
    if (horizontalFlip) {
        scaleX = -scaleX;
    }

    if (verticalFlip) {
        scaleY = -scaleY;
    }

    return {
        translate: { x: m.e, y: m.f },
        rotation: rotation,
        scale: { x: scaleX, y: scaleY }
    }
}

Feel free to use it in your code.

[chrvadala]

That is fantastic! It should close this old issue 🙂
Are you available to submit a PR?
Do you have some references (book, link, algorithm name,...) about this algorithm?

[hillin]

I think we've referenced several posts on Stack Overflow and Mathematics Stack Exchange, and a few other implementations in other libraries (it's ridiculous that it takes so much effort to make such a primitive algorithm work!); and with something homebrew (mostly the flip removing part).

I'd be happy to submit a PR, but what's the code of conduct of tests here? We didn't write any test for this function, it just worked well in our production environment.

[chrvadala]

I know, this algorithm is really tricky and it is important that it covers all the cases. This is why I asked you about some algorithm references. Unfortunately there are some algorithm that covers just a subset of transformation matrices.
Anyway, let's understand it with testing.

Regarding the tests I think that it will be really useful to create some matrices (leveraging on the other functions available in this library) and verify that they are "decomposable". We have to test both primitive matrices (e.g. translate) and composed matrices (e.g. translate+scale+rotate)

examples

const matrix = translate(42, 42)

expect(decompose(matrix)).toEqual({
        translate: { tx: 20, y: 20 },
        rotation: 0,
        scale: { x: 1, y: 1 }
})

Just one note about the returned object. In order to keep consistency with https://github.com/chrvadala/transformation-matrix#fromdefinitiondefinitionorarrayofdefinition--arraymatrix signature, can I suggest the following alternative signature? What do you think?

{
        translate: { tx: 0, ty: 0 },
        rotation: { angle: 0 },
        scale: { sx: 1, sy: 1 }
}

[hillin]

I‘m totally OK with the alternative signature, in our case we are just reusing some existing structs.

I'll see what I can do with then PR when I can catch some time.

[hillin]

I've created PR #88.
I find it a little bit disorienting when using the flipX and flipY method, which I instinctively thought stands for horizontal flip and vertical flip respectively, while the truth is contrary. Anyways I've adapted the code to this naming convention.

[chrvadala]

Released with v2.10.0