getScaleX/getScaleY return 1 if scale is applied through a matrix

[iamdustan]

First issue, assigning a scale via the matrix attribute, then getting the scale doesn’t return the applied scale.

myDisplayObject.attr('matrix', new Matrix(0.5, 0, 0, 0.5, 0 0));
`myDisplayObject.attr('scaleX')` // returns 1. Expects 0.5

Additionally

myDisplayObject.attr('scaleX', 0.5); // if applying a scale without using the matrix setters causes all future calls to setMatrix do nothing
myDisplayObject.attr('matrix', new Matrix(1, 0, 0, 1, 0, 0));

Is this expected?

[davidaurelio]

No, this is quite unexpected. Thanks for filing, this is a bug.

[iamdustan]

:thumbsup: that may be something I can tackle now that I’m face down in these things.

[davidaurelio]

That would be greatly appreciated

[iamdustan]

Making some progress, though the getters/setters and _prop/prop methods are a bit confusing to understand.

What is the difference and purpose behind DisplayObject._attributes._prop/prop? Primarily, for the matrix at this point.

Also, I don’t think I understand why the matrix getter (https://github.com/uxebu/bonsai/blob/master/src/runner/display_object.js#L73-L87) clones the _matrix and then does some processing of the numbers before returning it, while setting it just does a straight set to _matrix.

[davidaurelio]

What is the difference and purpose behind DisplayObject._attributes._prop/prop? Primarily, for the matrix at this point.

It’s implicit: Everything prefixed with “_” is considered private data, e.g. to store values returned by setters/getters. Unprefixed properties are considered public and can be updated through the attr() method.

The problem with the current accessor based code is that we are not really able to share code between different types. We want to replace it with a system as in sproutcore 1, where each attribute get/set operations will look for get_<name>/set_<name> methods on the attribute object. These methods are not mutators – attr() will pass the unprefixed value/the assignment value into them and use the return value.

This will allow for better code sharing and cheaper lookups, since most getter/setter pairs we have in place really only need the setter.

Also, I don’t think I understand why the matrix getter (https://github.com/uxebu/bonsai/blob/master/src/runner/display_object.js#L73-L87) clones the _matrix and then does some processing of the numbers before returning it, while setting it just does a straight set to _matrix.

The commit in the “old” (pre-public) repo and ticket there explain it. Unfortunately, the code misses an explanation. The reason is simple: If the programmer sets scale to 0, any immediate re-calculation of the matrix would set .a, .b., .c, and .d to 0 as well. That’d be a destructive operation. Setting scale afterwards would lead to a matrix like {a: NaN, b: 0, c: 0, d: NaN, tx: tx, ty: ty}. Therefore, we keep the scale separate and apply it only when getting the matrix (the user retrieves it or we send it to the renderer).

[iamdustan]

It’s implicit: Everything prefixed with “_” is considered private data

I know about the js convention to mark things as private with a prefixed _ but I got confused through looking at a few differents getter/setters in that file. Primarily the x and y pairs where both _matrix and matrix are used for calculations on the same line.

We want to replace it with a system as in sproutcore 1, where each attribute get/set operations will look for get/set methods on the attribute object.

I assume this means a rewrite is on the roadmap eventually.

The commit in the “old” (pre-public) repo and ticket there explain it. Unfortunately, the code misses an explanation.

It’s still out in the bikeshed? ;) Saw you had another commit with a comment. That helps a lot, thank you.

[davidaurelio]

Yes, these are weird. I tried a different, simpler implementation today, but didn’t commit, since I wasn’t sure whether it would work so well with rotation. I need to try and add other testcases, if needed

[iamdustan]

Copy that. I am burying myself in matrices right now so I could probably provide some test cases. If you want to look at it at all, feel free to join the IRC and I can localtunnel to you ;)