Spines use incorrect blend modes when batched if one or more is invisible.

[Kitsee]

Version

• Phaser Version: 3.23.0

• Operating system: Linux(Arch and Ubuntu)

• Browser: Firefox & Chrome

Description

Spines do not appear to be have the correct blend mode set when they get batched together. The GL blending mode appears to being set to ONE_MINUS_SRC_ALPHA instead of ONE when two spines are batched together resulting in everything but the last spine object in the batch being rendered incorrectly. Adding a game object with a differing type string such as image results in no batching and all the objects being rendered correctly.

We believe that the first blend mode used in a spine is not being checked against the last blend mode used in the previous spine resulting in the first element being rendered with the incorrect blend mode however when there is no batching, clearPipeline is being called at the start of every spine resulting in correct rendering. Please note we are not absolutely certain on this.

MASSIVE UPDATE:
Disabling batching caused an unacceptable drop in performance so we started debugging deeper. The bug appears to be triggered by having a invisible spines in between visible spines within a batch. when a invisible spine attempts to render it causes the following line of code to execute

    if (!skeleton || !willRender)
    {
        //  Reset the current type
        renderer.currentType = '';

(Dist/SpinePlugin.js:32214)

As you can see it sets the currentType to nothing, when the render continues to the next Spine in the display list it causes renderer.newType to be set to true as the last type was overridden to be "". this causes renderer.clearPipeline(); to be called resetting and overriding the blend state which had be previously set by the batcher. The clearPipeline call happens in the middle of a batch and as a result the blend state is not returned to the correct value.
Please note that sceneRenderer.begin(); will also get call as a result of newType being set however this method has safe guards interally which prevents the inappropriate call causing problems.

Our fix:
Our quick and dirty fix was to alter line 32231 from

    if (renderer.newType)
    {
        renderer.clearPipeline();
    }

to

    if (renderer.newType && !sceneRenderer.batcher.isDrawing)
    {
        renderer.clearPipeline();
    }

Example Test Code

The above image shows the results of the issue, in our application we have 15 spines running at the same time with some using the same spine asset and others using differing spine assets. Originally all spines were created together in the loop below. in order to work around the bug we had to add a empty image in between the spines to prevent batching.

this.reelSet.symbols.forEach((symbol,idx) => {
    thisAperSpines[idx] = scene.add.spine(0,0,symbol.winAnimConfig.spineKey).setActive(false).setVisible(false);
    //Adding the follow lines causes all spines GO to be seperated in the draw list, disabling batching.
    scene.add.image();
});

Additional Information

[spayton]

Sounds like the same issue I had, I think I had to add some extra spines with visibility turned off to get the previous ones to draw properly.

If you're using spines in containers you'll be in for another headache with performance, containers unnecessarily kill external render batching by resetting the pipeline when container changes. I did quite a bit of work on Phaser to get it to honour/cache the spine render settings as it does with its own objects and to also simplify the container code, but gave up as I was just fighting Phaser.

Phaser really needs a better way to integrate external rendering so that instead of blindly assuming that everything needs resetting, the external renderer can inform phaser which rendering components it has used (if any), so that phaser can maximise cache performance of rendering, masking etc. Until this happens Phaser will struggle with any decent usage of spines with containers.

[Kitsee]

@spayton thanks for the info. Our current method of handling this issue is we have a derived class of spine which adds or removes itself from the display list on setVisible. We did this partly for another reason, turns out invisible spines still fire a large number of webGL commands, if i remember correctly it was something stupid like ~30 commands per invisible spine which resulted in a large perform cost. Our total command count for a single frame went from something like 900 to 80.

[spayton]

@Kitsee sounds good, I'd be interested in saving commands for invisible spines too, though I thought I'd have noticed it was doing that. Or maybe the way I was hiding spines was accomplishing the same,
I moved unused spines to a hidden container and didn't mess with visibility.

[photonstorm]

Thank you for submitting this issue. We have fixed this and the fix has been pushed to the master branch. It will be part of the next release. If you get time to build and test it for yourself we would appreciate that.

Please see the new SpineContainer object, as well, which solves most of the batching issues when using Containers (or swapping between them on the display list)

[spayton]

If you get time to build and test it for yourself we would appreciate that.

@photonstorm I was just wondering this morning if the spine plugin was going to get some love, glad to see it has.

I ran into a problem trying it. It fails when no path has been set for the spine assets, in onFileComplete() this line now has an array accessor [0].

var path = GetFastValue(config, 'path', file.src.match(/^.*\//))[0]

As my test code had the spine assets in the root, no path setting was previously needed, now the workaround was to set it to root in preload, as follows:

this.load.setPath('./');

[photonstorm]

Hmm, this was changed in a commit from @FostUK - any idea Nick? You did this to allow Spine resources to be loaded from pack files, but clearly it has side-effects (although not ones I noticed in any of my tests!)

[FostUK]

Ahh sorry - it's possible I've added in the path but not taken account of a situation where no path has been set.

The path is set by the file manifest loader. I'll have a look at the current plugin code...