Make setOrientationChanged() public

[b005t3r]

Currently, if you override the transformationMatrix property (so your object is translated/scaled/rotated according to something else than x/y/rotation/scale etc.) you need a hack to call setOrientationChanged(), like this one:

public function set myNewParam(value:Number):void {
    _myNewParam = value;

    // a hack
    var oldX:Number = x;
    x = oldX + 1; // calls setOrientationChanged()
    x = oldX; // we don't really want to change x
}

// my new transformation matrix
override public function get transformationMatrix():Matrix {
    var mat:Matrix = super.transformationMatrix;
    mat.translate(0, _myNewParam); // alter the matrix accordingly to the new param
    return mat;
}

I don't see any risks in making setOrientationChanged() public - setRequiresRedraw() is public after all and used similarly.

[PrimaryFeather]

Aha, I see what you mean! You're right, that probably would not hurt. I guess protected would also suffice? I can't imagine a scenario where this would be called from the outside.

May I ask what exactly you are doing with the matrix? So that I understand the use-case behind this request. Thanks in advance!

[b005t3r]

I', doing exactly what I showed in the example - translating it on the y-axis based on a given parameter - I have this sliding panel which shows one one of it parts based on the param's value (it's a Number which changes from 0.0 to 1.0, so it can be animated). It works with the workaround I showed above, but doing it the set hacky way would be much cleaner :)

Here's my code:

public function get upgradeDecksVisibility():Number { return _upgradeDecksVisibility; }
public function set upgradeDecksVisibility(value:Number):void {
    if(_upgradesSprite.y == 0)
        return;
    
    _upgradeDecksVisibility = value;

    setOrientationChangedWorkaround();
}

override public function get transformationMatrix():Matrix {
    if(_upgradesSprite.y == 0)
        return super.transformationMatrix;
    
    var mat:Matrix = super.transformationMatrix;
    mat.translate(0, -_upgradesSprite.y * _upgradeDecksVisibility);

    return mat;
}

override public function set transformationMatrix(matrix:Matrix):void {
    super.transformationMatrix = matrix;

    if(_upgradesSprite.y == 0)
        return;

    setOrientationChangedWorkaround();
}

[b005t3r]

And yes, protected is fine.

[b005t3r]

After playing with it for a while I found another problem.

If you take a look at how I changed transformationMatrix property, you'll see I'm using the value returned by super.transformationMatrix and modify if afterwards. This will work only if the super-call rebuilds the matrix from scratch, but for optimization purposes it often does not and I'm unable to guess if it did recreate the matrix or not.

To solve this I'd need another method as well, something to read _orientationChange field's value, e.g. isOrientationChanged(). It can be protected as well.

Also, it would be great to have more control when overriding transformationMatrix property. Probably moving the property's body to a protected helper method and just calling this method from inside of transformationMatrix would help a lot - then all I'd need to do is override the helper method.

public function get transformationMatrix():Matrix
{
    if (_orientationChanged)
    {
        _orientationChanged = false;
        
        rebuildTransformationMatrix(_transformationMatrix, _x, _y, _pivotX, _pivotY, _scaleX, _scaleY, _skewX, _skewY, _rotation);
    }

    return _transformationMatrix;

protected function rebuildTransformationMatrix(output:Matrix, x:Number, y:Number, pivotX:Number, pivotY:Number, scaleX:Number, scaleY:Number, skewX:Number, skewY:Number, rotation:Number):void
{
    if (_skewX == 0.0 && _skewY == 0.0)
    {
        // optimization: no skewing / rotation simplifies the matrix math
        
        if (_rotation == 0.0)
        {
            _transformationMatrix.setTo(_scaleX, 0.0, 0.0, _scaleY,
                _x - _pivotX * _scaleX, _y - _pivotY * _scaleY);
        }
        else
        {
            var cos:Number = Math.cos(_rotation);
            var sin:Number = Math.sin(_rotation);
            var a:Number   = _scaleX *  cos;
            var b:Number   = _scaleX *  sin;
            var c:Number   = _scaleY * -sin;
            var d:Number   = _scaleY *  cos;
            var tx:Number  = _x - _pivotX * a - _pivotY * c;
            var ty:Number  = _y - _pivotX * b - _pivotY * d;
            
            _transformationMatrix.setTo(a, b, c, d, tx, ty);
        }
    }
    else
    {
        _transformationMatrix.identity();
        _transformationMatrix.scale(_scaleX, _scaleY);
        MatrixUtil.skew(_transformationMatrix, _skewX, _skewY);
        _transformationMatrix.rotate(_rotation);
        _transformationMatrix.translate(_x, _y);
        
        if (_pivotX != 0.0 || _pivotY != 0.0)
        {
            // prepend pivot transformation
            _transformationMatrix.tx = _x - _transformationMatrix.a * _pivotX
                                          - _transformationMatrix.c * _pivotY;
            _transformationMatrix.ty = _y - _transformationMatrix.b * _pivotX
                                          - _transformationMatrix.d * _pivotY;
        }
    }    
}

[PrimaryFeather]

Ah, I see! Mhm, you're right, that would be necessary then, too.

Hm ... I understand your request, and that those changes would make it possible to implement your transformation-matrix modification like that. But isn't there an easier way to achieve this? I.e. how would you approach this with the original display list API?

(My problem is that I don't think that many developers will need to do what you're doing here, and I'm hesitant to enlarge the API footprint for something that's maybe not entirely necessary.)

[b005t3r]

Well, I think many developers won't need all of the fancy architectural choices in Starling, but it's still a good thing to have them there anyway, right? :)

I think that this small refactoring (adding the rebuildTransformationMatrix() method) won't hurt anybody and it will make my life easier. I needed this stuff in my previous game, I need it with the one I'm currently working on. I could use a workaround, of course, but if there's a cleaner way to do things, why not do it this way? I could manipulate pivot for instance, but if at any point of development I need to change this object's pivot, I'll have to redo all of this from scratch - having this new property makes my stuff much less prone for such modifications in the future.

Here's another scenario that I'll have to implement in my game (it's a card game btw) that would require a new property:
Cards in card games can be tapped or exhausted, which simply means they are rotated 90 degrees. But if I used rotation for tapping my cards, I wouldn't be, for instance, able to animate their rotation without messing with their tapper/untapped state, so adding a new property is very useful here. Now, to actually make this work I have to change the transformation matrix of an object accordingly to this newly added tapped property - without the proposed changes it's pretty impossible (or at least very, very hacky).

[PrimaryFeather]

Well, I think many developers won't need all of the fancy architectural choices in Starling, but it's still a good thing to have them there anyway, right? :)

Yeah, of course, I'm all with you — what I was trying to say is that I have to constantly question what to add and what not. Or, in other words, it's equally important to decide what's "in" than what's "out". 😉

[PrimaryFeather]

Thanks for the additional examples. So, in a nutshell, you want to be able to have additional properties on a display object influence the transformation matrix that's created. (And make sure that on changing any of those properties, the matrix is recreated.)

[b005t3r]

Yeah, I understand. But I see this kind of approach - having an internally used method which "get's it done" - separated from the internally used logic (like with the transformationMatrix - caching the previous value logic) is a pretty clean and safe thing to have. It makes one's life easier when subclassing and it doesn't have any negative impact on anything (e.g. performance).

Thanks for the additional examples. So, in a nutshell, you want to be able to have additional properties on a display object influence the transformation matrix that's created. (And make sure that on changing any of those properties, the matrix is recreated.)

Yes, exactly.

[b005t3r]

Just getting back to this. Did you make any changes maybe?

[b005t3r]

BTW, I just noticed there's a similar thing for Sprite3D - _transformationChanged. I think this would require a similar treatment.

Also, currently calling setOrientationChanged() on a Sprite3D does NOT set _transformationChanged - isn't it a bug?

[PrimaryFeather]

Sorry, I've been very busy during the last days to finally get the "Starling Handbook" out of the door. I hope to be able to do that next week — and then I'll look into all those open issues and make a decision and/or continue the discussion.

Sorry for the delay! I'm really bad at multitasking these things. 😉

[b005t3r]

Sure, I understand, been there :) Would be great if you made the change next week however, I need this in quite a lot of places in my project and I have to rely on local hacks currently :)

[b005t3r]

Daniel, if you want me to create a pull request for this, let me know.

[PrimaryFeather]

Thanks a lot for the offer, Łukasz, but I want to play around this with myself, anyway — check out performance considerations, etc. Thanks, anyway!

I'm afraid, though, it definitely won't be ready this week. I really wanted to make it happen, but some personal issues intervened ... sorry!! ☹️

[b005t3r]

Sure, not a problem.

I don't think there're any performance issues to consider here however :) rebuildTransformationMatrix() is called only when you need to rebuild the current matrix, which is already a costly operation and additional method call won't make it that much more costly. If in somebody's code calling transformationMatrix turns out to be a bottleneck, their only option is to call it less often anyway - there's nothing that can be done here to make it go faster.

[b005t3r]

BTW, a very similar approach to the one I proposed is already implemented in Sprite3D (a method called updateMatrices()).

Would be cool to replace it with rebuildTransformationMatrix() where both matrices as well as all of the params (scales, position, etc.) are passed as method's parameters. Currently it requires a lot of hacking to work around updateMatrices().

[b005t3r]

OK, so I played a bit more with this and here's what I've changed:

1. DisplayObject

protected var _orientationChanged:Boolean; // was private
/*...*/
protected function setOrientationChanged():void // was private
/*...*/
public function get transformationMatrix():Matrix
{
    if (_orientationChanged)
    {
        _orientationChanged = false;
        rebuildTransformationMatrices(_transformationMatrix, null, true, _x, _y, NaN, _pivotX, _pivotY, NaN, _scaleX, _scaleY, NaN, _skewX, _skewY, NaN, NaN, NaN, _rotation);
    }
    
    return _transformationMatrix;
}

protected function rebuildTransformationMatrices(output2D:Matrix, output3D:Matrix3D, is2D:Boolean, x:Number, y:Number, z:Number, pivotX:Number, pivotY:Number, pivotZ:Number, scaleX:Number, scaleY:Number, scaleZ:Number, skewX:Number, skewY:Number, skewZ:Number, rotationX:Number, rotationY:Number, rotationZ:Number):void
{
    if (skewX == 0.0 && skewY == 0.0)
    {
        // optimization: no skewing / rotation simplifies the matrix math
        if (rotationZ == 0.0)
        {
            output2D.setTo(scaleX, 0.0, 0.0, scaleY, x - pivotX * scaleX, y - pivotY * scaleY);
        }
        else
        {
            var cos:Number = Math.cos(rotationZ);
            var sin:Number = Math.sin(rotationZ);
            var a:Number   = scaleX *  cos;
            var b:Number   = scaleX *  sin;
            var c:Number   = scaleY * -sin;
            var d:Number   = scaleY *  cos;
            var tx:Number  = x - pivotX * a - pivotY * c;
            var ty:Number  = y - pivotX * b - pivotY * d;

            output2D.setTo(a, b, c, d, tx, ty);
        }
    }
    else
    {
        output2D.identity();
        output2D.scale(scaleX, scaleY);
        MatrixUtil.skew(output2D, skewX, skewY);
        output2D.rotate(rotationZ);
        output2D.translate(x, y);

        if (pivotX != 0.0 || pivotY != 0.0)
        {
            // prepend pivot transformation
            output2D.tx = x - output2D.a * pivotX - output2D.c * pivotY;
            output2D.ty = y - output2D.b * pivotX - output2D.d * pivotY;
        }
    }
}

2. Sprite3D

protected static const E:Number = 0.00001; // was private
/*...*/
protected var _transformationChanged:Boolean; // was private
protected var _is2D:Boolean; // was private
/*...*/
public override function get transformationMatrix():Matrix
{
    if (_transformationChanged)
    {
        rebuildTransformationMatrices(_transformationMatrix, _transformationMatrix3D, _is2D, x, y, _z, pivotX, pivotY, _pivotZ, scaleX, scaleY, _scaleZ, skewX, skewY, 0, _rotationX, _rotationY, rotation);
        //updateMatrices();

        _transformationChanged = false;
    }

    return _transformationMatrix;
}
/*...*/
public override function get transformationMatrix3D():Matrix3D
{
    if (_transformationChanged)
    {
        rebuildTransformationMatrices(_transformationMatrix, _transformationMatrix3D, _is2D, x, y, _z, pivotX, pivotY, _pivotZ, scaleX, scaleY, _scaleZ, skewX, skewY, 0, _rotationX, _rotationY, rotation);
        //updateMatrices();

        _transformationChanged = false;
    }

    return _transformationMatrix3D;
}
/*...*/
override protected function rebuildTransformationMatrices(output2D:Matrix, output3D:Matrix3D, is2D:Boolean, x:Number, y:Number, z:Number, pivotX:Number, pivotY:Number, pivotZ:Number, scaleX:Number, scaleY:Number, scaleZ:Number, skewX:Number, skewY:Number, skewZ:Number, rotationX:Number, rotationY:Number, rotationZ:Number):void {
    output3D.identity();

    if(scaleX != 1.0 || scaleY != 1.0 || scaleZ != 1.0)
        output3D.appendScale(scaleX || E, scaleY || E, scaleZ || E);

    if(rotationX != 0.0)
        output3D.appendRotation(rad2deg(rotationX), Vector3D.X_AXIS);

    if(rotationY != 0.0)
        output3D.appendRotation(rad2deg(rotationY), Vector3D.Y_AXIS);

    if(rotationZ != 0.0)
        output3D.appendRotation(rad2deg(rotationZ), Vector3D.Z_AXIS);

    if(x != 0.0 || y != 0.0 || z != 0.0)
        output3D.appendTranslation(x, y, z);

    if(pivotX != 0.0 || pivotY != 0.0 || pivotZ != 0.0)
        output3D.prependTranslation(-pivotX, -pivotY, -pivotZ);

    if(is2D)
        MatrixUtil.convertTo2D(output3D, output2D);
    else
        output2D.identity();
}

I tested with my project, everything works as expected.

Also, I think there's no point in keeping _transformationChanged - it's basically the same as _orientationChanged, but for Sprite3D - I can't see why _orientationChanged can't be used for both. But maybe there's a reason for that I didn't see, so I left it as it was.

Performance wise, nothing has changed. Rebuilding matrices (which is already a CPU heavy operation) happens in a separate method now, so the cost of the whole thing increased only by the cost of calling a method, which is neglectable when compared to the cost of what this method actually does.

[b005t3r]

Daniel, maybe we can get back to this now?

I tested it with my current project, I can't see any problems with it. One more thing I had to do was changing _is2D and E to protected (I made this change in my comment above). This is needed to turn off the rendering optimization if for instance you introduced a new parameter which changes object's position along the Z-axis.

[PrimaryFeather]

I'm looking into it right now, Łukasz! Finally ...!

I know I must be super annoying to you by now (:bowtie:), but I'm still not 100% sold that this is a necessary change. The main issue is that we're talking about the DisplayObject class — this makes it really special, because 99% of all code that people write is happening in subclasses of that class. Having something protected in, say, the Button class, is not problematic at all: in this case, it's useful to people who extend Button, and all others don't even see it (e.g. via autocomplete).

Anything that's protected in DisplayObject, on the other hand, is constantly visible to basically everybody. And even if you don't need it, you see that it's there, and you might wonder if calling setOrientationChanged is something that must be done when you change the orientation of object — for example. Meaning: the fact that it's there alone will make many people use it, even though they do not exactly understand what it's for. And in this specific case, it's not exactly easy to explain.

[In other words, it's not the additional method calls that are problematic — if it were just a case of creating that rebuildTransformationMatrix method, I would do it immediately. But it's the dirty-flag (_orientationChanged) stuff that I don't like, and even more in the Sprite3D class; _is2D is not something anybody should see.]

So, I know you've got some code that depends on such a feature, and it would be useful to you to have that feature like you explained. I can totally understand that. But isn't there a way you could achieve the same with existing functionality?

For example, if we take your card example: I had similar requirements in the past, and I always did it by encapsulating the card in a container. The Card class is a DisplayObjectContainer that contains an Image that's the actual card. Then to mark your cards as "tapped" or "exhausted", you can rotate the card Image by 90 degrees (using a property called cardRotation on the container, that forwards to _image.rotation). → You've got two sets of properties: one from the container and one from the card inside the container. They add up, but you can also access the individual components any time.

This worked very well for my use-cases!

What should also make us suspicious is that what you want to have is not possible in the standard Flash display list API, and still people got around well with it. This is not a good argument — we should always add functionality to Starling where the original display list is lacking important functionality. But, as I said, in the case of the DisplayObject class, of all classes — we need to be especially careful.

I know that this is not what you wanted to hear, but please consider my arguments and check out if you could achieve what you want by different means — not via dirty trick, of course, but simply an alternative approach.

[b005t3r]

And even if you don't need it, you see that it's there, and you might wonder if calling setOrientationChanged is something that must be done when you change the orientation of object — for example.

But the thing is - it is something you have to call when you change the orientation of the object :) The problems is, currently, that you can't call it in your subclass, because it's private and there clearly is no reason to make it private.
Making _orientationChanged protected as well is not necessary, a protected method like isOrientationChanged() would be a better approach. But having it private makes no sense at all - it's something very similar to setRequiresRedraw(), with all the internal optimizations you need to have methods like these to invalidate the internally kept state. But please note these methods do not change the internal state, they just notify whatever is implemented inside the base classes to revalidate, so in other words, you won't break anything by calling these methods (if internals will change in the future, everything will still work, things will just be revalidated in a different way).

For example, if we take your card example: I had similar requirements in the past, and I always did it by encapsulating the card in a container.

For a simple trick, this would work. But it's insanely problematic with more than one such property, sometimes you'd have to put containers inside containers inside containers just to have a simple effect which, with these changes, can be done in a couple of lines (and changed in no time, which is very important when you're designing a feature you're not really sure of how it should work). So, no, definitely this isn't an option.

What should also make us suspicious is that what you want to have is not possible in the standard Flash display list API, and still people got around well with it.

But you see, what I'm proposing won't change the standard/internal behaviour - it'll just add new ways of doing things. Nothing will change for people using Starling - they won't even notice :)

Seriously, these things should be made more accessible, because it won't only make some things possible, it will also make DisplayObject's and Sprite3D's internals more understandable.

So, if you don't see any problems in introducing rebuildTransformationMatrices(), please do so - this change alone makes a lot of things possible instantaneously.

As for orientationChanged/transformationChanged flags I don't see why these shouldn't have protected methods for setting and reading them. These are basically the tools for people creating their subclasses which notify the internals that something has to be revalidated (much like setRequiresRedraw()) - you can't really go wrong calling setOrientationChanged(), it'll just invalidate transformation matrices and that's it. You can put what it does in the comment as you did for other methods, I can't see how this would confuse anybody more than not having these methods and having to find out when and how these flags are set.

For _is2D I don't have an idea better than what I came up with for rebuildTransformationMatrices() - a new protected method which is responsible for evaluating the new value (so it can be overridden in subclasses if needed). Same as with transformation matrices - it'd let you use the value produced by the default implementation (by calling super.rebuildIs2D() or something like that) and do something with it if you really have to.

And let me emphasize it one more time: these changes are not aimed at messing with Starling's internals, they do the opposite - they isolate or encapsulate the internally used stuff. Currently the internal implementation is monolithic and not easy to get your head around - you change one thing and you need to make sure you checked where it's used, if it's not overridden somewhere internally, if it won't produce any side effects etc., but with these change you can more easily make sure they're changing only the thing they wan't to change. Because, let's be clear, it's not like by making everything private you're protecting people from breaking they code - they will make hacks the same way I did and those hacks will blow up in some time for sure. On the other hand, having the internals better structured will cause less hacks and more code which should work with future versions of Starling with no problems as well.

I hope I made myself clear :) And I hope I made you see these won't make things more exposed to errors in the future as Starling changes, but it will in fact make the opposite.

[PrimaryFeather]

But the thing is - it is something you have to call when you change the orientation of the object :)

No, only when you add additional properties that modify the orientation of the object. Normally, you use x, y, scaleX/Y, rotation, pivotX/Y to change the orientation. That's been enough for all users of the standard Flash API, for years. 😉

[setRequiresRedraw is different: it's required when the orientation does not change, but a custom style requires the object to be redrawn.]

It's not that I don't understand your reasoning; I just don't understand yet how generations of Flash programmers could live without that feature if it lacking makes everything sooo complicated. ;-)

But let me play around with the code a little more — perhaps I can come up with a solution I can live with. The main difference in our opinions is maybe that I'm seeing things as implementation details (private) which you want to customise (protected). And my mantra "if in doubt, leave it out", forces me to be very conspicuous, because I'm still having doubts and every method I add to DisplayObject is a method that might be back haunting me forever.

... btw, since it's related to my doubts — how would you have tackled this issue in classic Flash?

[PrimaryFeather]

(And, again, there actually is no such thing as protected in the DisplayObject class. Because 99% of all Starling code inherit from DisplayObject. So it's visible for anybody, and that's the API footprint I like to keep minimal.)

(Just to maybe help understand where my hesitation is coming from.)

[b005t3r]

It's not that I don't understand your reasoning; I just don't understand yet how generations of Flash programmers could live without that feature if it lacking makes everything sooo complicated. ;-)

Oh, come on :) I hear the same reasoning from people who still code in C every day - how come I now need all of this OOP stuff when we can do almost anything with a bit of structural hacking :) I'm not saying things can't be done the way people used to do it for the past years - I'm looking for ways of doing it better :)

And, again, there actually is no such thing as protected in the DisplayObject class. Because 99% of all Starling code inherit from DisplayObject. So it's visible for anybody, and that's the API footprint I like to keep minimal.

But still, there's a huge difference between protected and public. The later can be called from anywhere in the code, the former can be called only from the class itself or from inside of its base classes (you can't ever call a protected method of another object, even of the same class: https://stackoverflow.com/questions/30401767/accessing-protected-functions-from-another-class-flash).

And again about this whole idea, it's not really like I'm reinventing the wheel :) I'm proposing to introduce two concepts which have been used in similar scenarios for years.

1. InvalidateX - you can only ask to invalidate X from outside (X is later used internally, we don't know how and we can't ever change it which makes it possible for the internal implementation to change in the future), you can't change it directly. This concept is already introduced in Starling in some places, but it's not there in some other places for no particular reason.
2. ValidateX - a separate, well defined and only one place for X to be given a new value. This disallows changing X from outside as well as changing in indirectly in different context than the one specified by the internal implementation (like with the rebuildMatrices() - you can't call it wherever and whenever you want and change the transformation matrix). It allows however to change X from inside in a controlled and a very precisely defined manner. It even allows to modify the value returned by the default implementation instead of rebuilding it from scratch.

That's all I'm asking for. As I said, it's not like it encourages to mess with the internals. What we have now forces you to hack the internals. Like with my example, I can either hack it which is not safe and requires a lot of work OR I can go with the container approach which is only a bit more safe (it would still relay on things like how batching is implemented, when things are invalidated, etc.), requires much more work and it a nightmare if you have to introduce changes (because the actual functionality is unnaturally spread amongst a couple of classes). The proposed approach does not have these issues and it does not introduce any new issues - it does not expose the internals, it encapsulates and separates the internals even more than they are separated now; people who want to mess with the internals will be able to do it in a more controlled, safe way, which means less hacks, which means less bugs and more code which will work with the future versions of the framework.

[PrimaryFeather]

Okay, you'll find an approach to solve this in the commit I just made. What actually changed my mind was your remark that a lot of the Sprite3D code would also benefit from this change. 😉

I still think that this change is going to be used only rarely, but it's easier to sell if my own code benefits from the change, too. 😜

There are now two additional methods in the starling_internal namespace – which is the compromise that will make sure only people who know what they are doing are actually seeing it. That's

• function updateTransformationMatrix()
• function setTransformationChanged() (replacing the private setOrientationChanged)

I don't think we really need read-access to _transformationChanged, since checking this value only needs to be done in the base class, anyway, as I'm seeing it. As for is2D, I could add a public property for that, if you need it – but we'd have to find another name (isFlat maybe?) to differentiate it from the existing is3D property. If I keep the name like it is, people would end up with objects that have both is3D and is2D return true – which will definitely raise a few eyebrows. 😉

Please have a look at these changes and tell me what you think.

[b005t3r]

First of all, you're the man, Daniel :)

I'll check your changes tomorrow and let you know. I think isFlat is a much better name for the current is2D - much more self-explanatory.

[b005t3r]

OK, one thing that's not currently there and is quite essential for this idea to really shine - updateTransformationMatrix() needs to take all the params as input parameters, like so:

updateTransformationMatrix(output2D:Matrix, output3D:Matrix3D, is2D:Boolean, x:Number, y:Number, z:Number, pivotX:Number, pivotY:Number, pivotZ:Number, scaleX:Number, scaleY:Number, scaleZ:Number, skewX:Number, skewY:Number, skewZ:Number, rotationX:Number, rotationY:Number, rotationZ:Number):void

I know it might not look pretty (it's not that bad IMO), but it lets you to modify the input only when overriding updateTransformationMatrix() and this is sufficient for many use cases - no need to rewrite the whole algorithm, you can easily modify the params and let the default implementation do its job, eg.:

override function updateTransformationMatrix(output2D:Matrix, output3D:Matrix3D, is2D:Boolean, x:Number, y:Number, z:Number, pivotX:Number, pivotY:Number, pivotZ:Number, scaleX:Number, scaleY:Number, scaleZ:Number, skewX:Number, skewY:Number, skewZ:Number, rotationX:Number, rotationY:Number, rotationZ:Number):void {
    x += someParam * 5;

    super.updateTransformationMatrix(/* ... */);
}

[b005t3r]

OK, so I gave your changes a look and here are my thoughts:

1. I'd need those params to be added to updateTransformationMatrix() - it makes it just a lot easier to override this method (implementation is quite complex and in most of the times messing about with input params and output values is all that needs to be done; I can provide an example how I do it if you want me to).
2. As you said, isFlat is a better name for is2D.
3. I'd need an internal method like updateIsFlat() (no additional params needed here in my case), because if you modify things like for instance z input param of updateTransformationMatrix() it won't be reflected unless isFlat returns false (due to optimization). In other words, if you're messing about with those params for some scenarios you need to be able to change isFlat as well (I can provide a piece of my code if you need to see what I mean for this as well).

[PrimaryFeather]

You should now have all the features you need! Let me know if that works for you.

I'm still not sold that this problem could not have been solved by wrapping the object in a container, so consider this a special gift for a loyal Starling user. 😉

[b005t3r]

Brilliant, Daniel, I'll give it a go today :)

I'm still not sold that this problem could not have been solved by wrapping the object in a container

It could, but as I explained, this would require much more work when making modifications, because often you'd need ti change the entire hierarchy built this way to add a new feature or change how a given feature works. With this approach it's just changing an input param and it works :)

And the possibilities are really vast - now with just a couple of lines you can add a new anchor point (similar to pivot), which would work only for some special transformations (I use it myself, so I don't need to change pivot constantly, but I can animate different rotations around a moving point).

To push it even further, I think DisplayObjectContainer's render() is another good candidate for moving part of the logic to a separate function (e.g. renderChild() which would render a single child only, without looping and push/pop logic). Currently, when doing custom rendering, you need to rewrite the render method entirely and additionally turn the cache off for that object (because caching logic is strictly private).

I don't need it that much right now however 😄 but I encourage you to give it a look and think about it if you get'll a minute.

And thanks one more time 👍

[PrimaryFeather]

Yes, I'm sure there are ways to make good use of this – maybe I'll be thankful to have it like that, too, soon enough. 😉

As for the render method: I'm all with you with that one, I don't like the way everything's bundled in that DisplayObjectContainer method – but here, performance just overrules everything, IMHO, so it's a very delicate manner. Perhaps it can be "bought" with some optimisations in other parts of the API, though. 😄

[b005t3r]

Daniel, can you change updateIsFlat() to actually return the value, instead of assigning it internally? Currently I can't simply override the method to add my custom logic and return the modified value.

performance just overrules everything

I understand, but do you think that it would make a difference to have one more method call? It can't be that bad. I know it'd be called each frame but there are things happening which are relatively much, much slower, so it shouldn't matter.

[b005t3r]

A similar thing is needed for updateTransformationMatrix() - both matrices have to be passed as params there to make it possible to change the resulting matrix. I feels quite unsafe to call the getter from inside of updateTransformationMatrix() which is in the process of validating the value returned by the getter (currently it looks safe, but if this changed in the future, there would be a problem with existing code - passing both matrices as params is much safer).

[PrimaryFeather]

I just simplified isCached altogether. Since it's such a simple check, I don't think it's necessary to cache the result. That should make it very easy for you to override and make use of the super call.

As for updateTransformationMatrix: no, that's not necessary — you can be sure that transformationMatrix (and the 3D variant) stay the same throughout the lifetime of the object. I'm relying on that myself in the Sprite3D class, where I'm now copying references to the two matrices (2D and 3D) right in the constructor.

[b005t3r]

As for updateTransformationMatrix: no, that's not necessary

I don't think we're talking about the same thing :) I want to override updateTransformationMatrix() which has been called by a call to get transformationMatrix, so I shouldn't call get transformationMatrix from inside of updateTransformationMatrix() - do you know what I mean? :)

[PrimaryFeather]

I understand, but do you think that it would make a difference to have one more method call? It can't be that bad. I know it'd be called each frame but there are things happening which are relatively much, much slower, so it shouldn't matter.

We're talking about one method call per object, per frame in the worst case, and that worst case occurs quite frequently. Do this in a few places, and that quickly adds up! I've spent many many hours optimising method calls away — trust me, in AS3, it does unfortunately make a difference.

[b005t3r]

OK, I understand.

About the updateTransformationMatrix() I'd need this additional input params:

• output2D:Matrix
• output3D:Matrix3D
• z:Number
• pivotZ:Number
• scaleZ:Number
• skewZ:Number
• rotationZ:Number

[b005t3r]

Also, isn't it a bug? DisplayObject has it's own _transformationMatrix and _transformationMatrix3D and Sprite3D has its own as well. It sort of defeats the purpose of having the updateTransformationMatrix() which is supposed to let easily reuse the cached instance in subclasses. It also creates new Matrix and Matrix3D instances which makes the old ones just an additional work for GC.

[PrimaryFeather]

DisplayObject has it's own _transformationMatrix and _transformationMatrix3D and Sprite3D has its own as well.

No, they just store references to the matrices from the super class.

[b005t3r]

OK, I see how it works. But you wouldn't need it, if you passed the matrices as params to updateTransformationMatrix(). It's not really safe as well (if for some reason, in the future, there will be a use case that creates a new instance in the base class, this whole thing is not going to work anymore).

[PrimaryFeather]

Yes, I'm already working on that. 😉

[PrimaryFeather]

I think this solution is rather clean! Looking forward to your code review. 😉

[b005t3r]

I like the idea with two methods - one for 2D stuff and the other which adds the 3D stuff.

A minor thing, it would be cool to have this as a public/protected constant:

private static const E:Number = 0.00001;

I have found a bug however - out3D can be null and it crashes the whole thing, but I didn't find out why is it null.

It's probably because in my scenario get transformationMatrix3D hasn't been called yet (and it initializes the member variable).

[PrimaryFeather]

Ooops, yes, I could reproduce that error. Fixed with 3060cd1.

As for E: that's really an implementation detail — just declare a copy of that variable yourself, please. What I'd really like to do here is call MathUtil.isEquivalent, but I inlined that for performance reasons.

[b005t3r]

OK, it looks like everything works as expected now :)

Thanks one more time for making the changes, Daniel!

[PrimaryFeather]

I'm glad to hear everything is working fine now! I'm happy I could help. 😎