Gamua / Starling-Framework

The Cross Platform Game Engine
http://www.starling-framework.org
Other
2.84k stars 819 forks source link

Sort optimisation #946

Open JohnBlackburne opened 7 years ago

JohnBlackburne commented 7 years ago

This adds two functions and associated constants, implementing a faster sort at the cost of some flexibility. Rather than a function it sorts on one of x, y or z. Sorting on y is commonly used to draw an overhead view, so objects recede into the distance as they move up towards the horizon. z is unused in Starling so can be used for arbitrary data. x is for completeness. In a simple test it seems to provide a 10 - 20% speedup over sortChildren using a function.

PrimaryFeather commented 7 years ago

Thanks a lot for the pull request, John! I appreciate your efforts! :smile:

Have you made any performance tests comparing this implementation with the existing way of passing a function? How much faster is this in typical use cases? (Plus: AOT vs. JIT?)

Furthermore, did you also compare this implementation (using the integer properties) to what's happening if you pass a String ("x") to the method instead, and then sorting by that?

I'm a little hesitant to duplicate the existing sortChildren, so I'd like to know if it's worth it. :wink:

JohnBlackburne commented 7 years ago

I used the following code to test it, based on code in this thread, more a quick hack than a thorough test, like the code. The speedup with this is around 20%.

package
{
    import flash.display.Sprite;
    import starling.core.Starling;

    [SWF(width="640", height="480", frameRate="60", backgroundColor="#bfbfbf")]
    public class SortTest extends Sprite {
        private var _starling:Starling;

        public function SortTest() {
            _starling = new Starling(MainStarling, stage);
            _starling.start();
        }
    }
}

    import starling.display.*
    import starling.text.*
    import starling.events.*
    import flash.utils.*
    import starling.utils.*

    class MainStarling extends Sprite
    {

        private var text:TextField;

        private var arr:Vector.<DisplayObject> = new Vector.<DisplayObject>();
        private var arrNum:Vector.<Number> = new Vector.<Number>();

        private var startTime:Number;
        private var endTime:Number;
        private var container:Sprite;

        public function MainStarling()
        {
            addEventListener(Event.ADDED_TO_STAGE, Start);
        }
        private function Start():void {

                container = new Sprite();

                var tf:TextFormat = new TextFormat("Verdana", 16, 0, "left", "bottom");

                //OUTPUT TEXT
                text = new TextField(500, 500, "hello", tf);
                text.border=true;
                text.autoSize="vertical";
                addChild(text);
                text.alignPivot();
                text.x = stage.stageWidth/ 2;
                text.y = stage.stageHeight / 3;

                addSprites(30000,true);
                addEventListener(Event.ENTER_FRAME, Tick);
        }

        //ENTER FRAME GAME LOOP
        private function Tick(e: Event):void {
            //SORT ON

            text.text = "TIMED SORTING TEST";

            //STARLING_SORT CHILDREN
            randomize();
            startTime = getTimer();
            container.sortChildren(sortOnY);
            endTime = getTimer();
            text.text +="\n"+ "old sort: " + (endTime-startTime);

            randomize();
            //NEW STARLING_SORT CHILDREN
            startTime = getTimer();
            container.sortChildrenOnKey(DisplayObjectContainer.ON_Y);
            endTime = getTimer();
            text.text += "\n" + "new sort: " + (endTime-startTime);
        }

        private function sortOnY(object1:DisplayObject, object2:DisplayObject):int {
            return (object1.y - object2.y);
        }

        private function addSprites(num:int,convertToInt:Boolean):void {

            for (var i:int = 0; i < num;++i ){

                var m:Sprite = new Sprite();
                m.y = (Math.random() * 1000) - 500;;
                container.addChild(m);

                //convert .y to int so it can be sorted
                if (convertToInt){
                    m.y = int(m.y);
                }
            }
        }

        private function randomize():void{

            for (var i:int = 0; i < container.numChildren;++i ){
                var sp:DisplayObject = container.getChildAt(i);
                sp.y = (Math.random() * 1000) - 500;
            }
        }
    }

So probably not something that makes sense as a feature of Starling. Anyone who wants it can implement it themselves. You also don’t need to make such drastic modifications to Starling to do this: you can just make _children public or create and accessor to it then sort that.

JohnBlackburne commented 7 years ago

I’ve uploaded another version, which simplifies how it works. Now instead of a new public method it instead works through sortChildren; if no function is supplied it calls the version without the function calls, which only has one way of working.

It’s basically the version I’m using; it was an idea I had during the discussions on the forum, and the easiest way to demonstrate it was code it, try it out and upload it. Apart from the above test code I wanted to test it in production code, to be sure it worked, even though the number of things I’m sorting is too small for sort performance to be an issue.

kheftel-old commented 7 years ago

I'm only one voice, but I just wanted to say that I think being able to sort by y at the system level would be a great feature, and a common enough usecase for a 2D framework that a case could be made for including it in core.

JohnBlackburne commented 7 years ago

It does not actually sort on y, any more: I simplified it and left in only the sort on 'sort', a new field added to DisplayObject, as that was more useful to me. The cost of this sort of optimisation is less flexibility, and how much you leave in is pretty arbitrary. Right now if you want sort on y you can grab the code and change '.sort' to '.y' in mergeSortNoFunction.

PrimaryFeather commented 7 years ago

Thanks for the new version, and thanks for your feedback, too, Kawika!

You're probably right, the performance enhancement alone is probably not big enough to justify adding the additional method. On the other hand, as Kawika said, a simple way to sort by "y" (or any other property) would simplify things. I'll definitely think about it!!

swapnilkhairnar commented 7 years ago

The main reason why its not working is in the roots of itself....i think neither y has any problem nor x problem is in tht arbitrary axis i.e the centre .