Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batching doesn't group identical blend modes of siblings #1045

Open
NB13 opened this issue Sep 23, 2018 · 3 comments
Open

Batching doesn't group identical blend modes of siblings #1045

NB13 opened this issue Sep 23, 2018 · 3 comments

Comments

@NB13
Copy link

NB13 commented Sep 23, 2018

Starling 2.4 has problems with batching animated display objects having one mesh style.
Here is demo of error and hot fix for current (2.4) version. In first case we will get 100 draw calls, in the second only 1.

import starling.animation.Tween;
import starling.core.Starling;
import starling.display.BlendMode;
import starling.display.Image;
import starling.display.Sprite;
import starling.textures.Texture;

public class Game extends Sprite {
    private static const NUM_IMAGES:int = 100;
    private static const TWEEN_DURATION:int = 100;

    public function start():void {
        var texture:Texture = Texture.fromColor(100, 100, 0xffffff);
        wrongCase(texture);
        hotFixCase(texture);
    }

    private function wrongCase(texture:Texture):void {
        for (var i:int = 0; i < NUM_IMAGES; i++) {
            var image:Image = new Image(texture);
            image.blendMode = BlendMode.ADD;

            var tween:Tween = new Tween(image, TWEEN_DURATION);
            tween.animate("alpha", 0);
            Starling.juggler.add(tween);

            addChild(image);
        }
    }

    private function hotFixCase(texture:Texture):void {
        var sprite:Sprite = new Sprite();
        sprite.blendMode = BlendMode.ADD;
        for (var i:int = 0; i < NUM_IMAGES; i++) {
            var image:Image = new Image(texture);

            var tween:Tween = new Tween(image, TWEEN_DURATION);
            tween.animate("alpha", 0);
            Starling.juggler.add(tween);

            sprite.addChild(image);
        }
        addChild(sprite);
        sprite.x = 150;
    }
}
}
@PrimaryFeather
Copy link
Contributor

You mean that in the "wrongCase" method, you're ending up with one draw call per display object, correct?

That's unfortunately a negative side effect of the overhaul of the rendering architecture I introduced in Starling 2.

Short description: the current behavior is a trade off that allowed simplification of the render API in lots of places. It's unlikely to change anytime soon.

Long description:

During the lifetime of Starling 1.x, I ran into countless bugs related to the blend mode. The main reason was that, basically, whenever I drew one object, I had to check the blend mode of the previous object, and that would decide if a new batch (draw call) was started. That happened at lots of different places, and complicated things quite a bit.

So, in Starling 2.0, I simplified the blend mode logic by making it part of the general render state logic. Basically, when iterating over the display list, I "push" a new render state when moving down the list (i.e. from a parent to a child), and I "pop" to the previews render state when moving up (i.e. from child to parent). For every push and pop, the following logic is executed:

if (blendModeChanges || targetChanges || clipRectChanges || cullingChanges || depthMaskChanges || depthTestChanges)
{
    drawCurrentBatch(); 
}

And that's what's responsible for those draw calls. When iterating over the children of a container during rendering, I have to restore the previous (parent) state after every child, to undo the changes the previous child did to the render state. In the example you provided, this means that each child uses its own draw call.

Which is unfortunate — but I haven't found an elegant way around it, that's not so complicated that it would mean lots of fragile code elsewhere.

I hope you understand my reasons for keeping it like this for now! At least there is a rather simple workaround, as you discovered yourself (setting the blend mode on the complete container instead of the individual children).

@PrimaryFeather PrimaryFeather changed the title Batching error Batching doesn't group together identical blend modes of siblings Sep 23, 2018
@PrimaryFeather PrimaryFeather changed the title Batching doesn't group together identical blend modes of siblings Batching doesn't group identical blend modes of siblings Sep 23, 2018
@NB13
Copy link
Author

NB13 commented Sep 24, 2018

Hi, Daniel! Sorry for writing issue in a hurry, yes - separate draw call for each child is the issue. Thank you for detailed answer.
Maybe I'll try to fix this issue, but I'm afraid to break "fragile code" you've mentioned. Do you have some auto tests?

@PrimaryFeather
Copy link
Contributor

PrimaryFeather commented Sep 24, 2018

Unfortunately, I don't have any automated tests for rendering - I'd have to set up something that would allow me to read and compare actual pixels, and I never quite pushed myself to do that. In cases like this, it would definitely have been worth it, though. :bowtie:

A part of the issues that came up with blending over time can be found by looking through the commit messages that mention blending, like here. Perhaps that helps.

Otherwise, I could look into it again when I find some time to squeeze it in. And that workaround with the additional container is always an option, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants