localToGlobal() fails on second item in a group after parent moves when applyMatrix=false

[arel]

When moving a Group with its applyMatrix set to false, the child item's localToGlobal() function works for only the first child of a group, but fails for any subsequent items in the group.

This is related to #1448: it seems that while #1552 fixed the issue for the first item in a group, it didn't for subsequent items.

Description/Steps to reproduce

(See Sketch below for working example)

Note:

• applyMatrix = false is required to see the problem
• also, the setTimeout() is required to see the problem. I had originally experienced this issue with Raster images and was getting the same problem in the onLoad() callback.

applyMatrix = false     // fails
// applyMatrix = true   // works

g = new Group({applyMatrix: applyMatrix})

r1 = new Shape.Circle({
    fillColor: "red",
    radius: 50
})

r2 = new Shape.Circle({
    strokeColor: "green",
    radius: 50
})

g.addChild(r1)
g.addChild(r2)

// fails:
setTimeout(() => {
    console.log("Group's position before move", g.position)
    g.position.x += 100
    console.log("Group's position after move", g.position)

    if (!r1.localToGlobal([0,0]).equals(r2.localToGlobal([0,0]))) {
        console.error("*** FAIL! ***")
        console.warn("OK: positions:", r1.position, r2.position)
        console.warn("OK: matrices:", r1.matrix, r2.matrix)
        console.warn("OK: parent's positions:", r1.parent.position, r2.parent.position)
        console.warn("OK: parent's matrices:", r1.parent.matrix, r2.parent.matrix)
        console.error("FAIL: Items' localToGlobal:", r1.localToGlobal([0,0]), r2.localToGlobal([0,0]))
        console.error("FAIL: Groups' localToGlobal:", g.localToGlobal([0,0]))
    }

}, 1000)

Link to reproduction test-case

Sketch reproducing issue

Expected result

Two identical items contained in a parent Group should give the same value for item.localToGlobal([0,0]) after moving the parent.

Actual result is that the first item in the group gives the correct localToGlobal value, but the second returns a value as if the parent had not moved.

Additional information

• Paper version 0.12.4
• ChromeOS Version 78.0.3904.106 (Official Build) (64-bit)

[arel]

Looking at #1552 I was able to find a workaround that fixes the symptom (though I'm not sure what the root cause is.)

It seems that at the time setTimeout runs, the _globalMatrix has been set for each of the items. Setting the position of the group invalidates its _globalMatrix (but not that of its children) by setting it to undefined.

Therefore, calling getGlobalMatrix() on the group (after setting the group's position), fixes the problem:

g.getGlobalMatrix() // this has the side-effect of fixing the problem!

Also, clearing the child's _globalMatrix (before or after setting the group's position) fixes the problem.

r2._globalMatrix = undefined

Sketch with workaround

---

It looks like maybe there should be a call to getGlobalMatrix() after an Item's _globalMatrix is cleared?

https://github.com/paperjs/paper.js/blob/a2069fc73db930dca2e76fc1d96746659ed3b805/src/item/Item.js#L222

if (flags & /*#=*/ChangeFlag.MATRIX) {
    this._globalMatrix = undefined;
    this.getGlobalMatrix();  // <----- rely on side-effect to clear children's cache too?
}

Possibility 1: getGlobalMatrix could be renamed and split into a side-effect-free getter and a method that does the updating such as updateGlobalMatrices.

Possibility 2: instead of directly setting the instance variable this._globalMatrix, create a setter function setGlobalMatrix() that takes care of invalidating the cache of its children and ancestors.

@ismailnamdar, want to take a look?

[sasensi]

Hi, thank you for the report and the investigation. Actually, https://github.com/paperjs/paper.js/pull/1552/commits/409a3ac46708b27dbe8281defab3f7dc1fadf5f4 did address this issue too (I tried reverting to that point) but it was later undone by https://github.com/paperjs/paper.js/commit/c5b822da and https://github.com/paperjs/paper.js/commit/c4150947.

So I think that coming back to the original idea of clearing children global matrices along with the parent's one seems to be the solution.
@lehni, what do you think ?

[WesleyKapow]

Bump! This is kind of a nasty bug.. simple work around but tough to pin down. I'm building a 2d-editor and this bit me in that I record current position values, so after a group is changed, I want to record the positions of the children.

For now I went with calling item._globalMatrix = null on each item before I fetch the new position values.

Seems possible there is a simple solution but perhaps with unwanted performance penalties? Of course current behavior is wrong.