ruffle icon indicating copy to clipboard operation
ruffle copied to clipboard

avm2: Implement DisplayObject.transform and most of Transform

Open Aaron1011 opened this issue 3 years ago • 4 comments

This PR implements the 'DisplayObject.transform' getters/setters, and most of the getters/setters in the Transform class

From testing in FP, it appears that each call to the 'DisplayObject.transform' property produces a new 'Transform' instance, which is permanently tied to the owner 'DisplayObject'. All of the getters/setters in Transform operate directly on owner DisplayObject. However, note that the Matrix and ColorTransform valuse produced the getter are plain ActionScript objects, and have no further tie to the DisplayObject.

Using the DisplayObject.transform setter results in values being copied from the input Transform object. The input object retains its original owner DisplayObject.

Not implemented:

  • Transform.concatenatedColorTransform
  • Transform.pixelBounds

When a DisplayObject is not a descendant of the stage, the concatenatedMatrix property produces a bizarre matrix: a scale matrix that the depends on the global state quality. Any DisplayObject that is a descendant of the stage has a concatenatedMatrix that does not depend on the stage quality. I'm not sure why the behavior occurs - for now, I just manually mimic the values prdduced by FP. However, these values may indicate that we need to do some internal scaling based on stage quality values, and then 'undo' this in certain circumstances when constructing an ActionScript matrix.

Unfortunately, some of the computed 'concatenatedMatrix' values are off by f32::EPSILON. This is likely due to us storing some internal values in pixels rather than twips (the rounding introduced by round-trip twips conversions could cause this slight difference0. For now, I've opted to mark these tests as 'approximate'.

To support this, I've extended our test framework to support providing a regex that matches floating-point values in the output. This allows us to print out 'Matrix.toString()' and still perform approximate comparisons between strings of the format '(a=0, b=0, c=0, d=0, tx=0, ty=0)'

Aaron1011 avatar Jul 25 '22 06:07 Aaron1011

I don't understand this part of the commit message:

However, note that the `Matrix` and `ColorTransform`
valuse *produced* the getter are plain ActionScript objects,
and have no further tie to the `DisplayObject`.

torokati44 avatar Aug 09 '22 06:08 torokati44

And this is just a nit, but I think this line: a scale matrix that the depends on the global state quality. Should be changed to: a scale matrix that depends on the global stage quality.

Also: difference0 -> difference.

torokati44 avatar Aug 09 '22 06:08 torokati44

Furthermore, maybe this could be split into (at least) three separate commits? One adding the implementations, one extending the test framework, and one adding the tests. That could also cut down two small pieces of the lengthy commit message. It doesn't make that much of a difference, just for the sake of tidiness.

torokati44 avatar Aug 09 '22 07:08 torokati44

I don't understand this part of the commit message:

However, note that the Matrix and ColorTransform valuse produced the getter are plain ActionScript objects, and have no further tie to the DisplayObject.

@torokati44 Any modifications that you make to the Matrix and ColorTransform have no effect on the Transform (just like any other Matrix/ColorTransform). You need to explicitly write to the fields of Tranform to make changes.

Aaron1011 avatar Aug 09 '22 21:08 Aaron1011

I'm a bit late, and thanks for the explanation @Aaron1011; what I meant is that, that sentence didn't immediately make sense to me grammatically. Perhaps there is a missing "by" or "in" after "produced"? (Also a typo in "valuse", but oh well, it can still be understood.) Either way, it's fine now, just wanted to add a bit more explanation.

torokati44 avatar Aug 15 '22 07:08 torokati44