[ts][pixi] Spine pixi fixes

[GordonTombola]

Changes to spine-pixi

Spine.ts

Empty Attachment keyframes

slots with keyframes on the attachment property will continue rendering their previous attachment if they were keyed to remove the attachment.

Solution

we now check for the existence of the mesh when there is no attachment and set its renderable property to false if thats the case.

Weighted clipping polygons

In the case of clipping attachments, the vertices properties can be very different to a simple x,y pairing if it uses a weighted clipping polygon. When calculating the worldVertices for the pixi graphics object, the resulting vertices array was being used as a basis for the end result rather than the vertexCount length, producing garbage data at the tail end of the world vertices array.

example clipping attachment data:

"vertexCount": 4,
"vertices": [ 1, 15, -47.71, 34.91, 1, 1, 18, -47.86, -35.06, 1, 1, 17, 47.8, -34.9, 1, 1, 16, 47.81, 34.87, 1 ]

this would produce a world vertices array of length 12 when the intended size was 8

Solution

we now create the world vertices array using the worldVerticesLength property to determine its length

[davidetan]

Thanks for this PR!

I cannot repro the first problem. If I set a key to remove an attachment from the slot, that attachment is not rendered. In any case, I think that is correct what you are doing and I'm willing to merge it. My only question is if you have any particular reason to set to false the renderable property, rather than the visible property. We don't need to have the updateTransform of the mesh called. If we do that, we don't event need to reset it to true, since getMeshForSlot already does that.

Regarding the second issue, I think we can improve it further without the need of creating a new array each time, but just "drawing" with Pixi API. Let me give a look into that.

[davidetan]

For the first issue, I think we can simply use the visible property as I said above. So, rather thank making all the changes you proposed, we can simply add these two lines of code:

} else {
    const mesh = this.meshesCache.get(slot);
    if (mesh) mesh.visible = false;
    Spine.clipper.clipEndWithSlot(slot);
    this.pixiMaskCleanup(slot);
    continue;
}

For the second issue, I think we can avoid to allocate a new array each time just by reusing the same one and drawing the polygon with moveTo and lineTo linke this:

const worldVerticesLength = clippingAttachment.worldVerticesLength;
if (this.clippingVertAux.length < worldVerticesLength) this.clippingVertAux = new Float32Array(worldVerticesLength);
clippingAttachment.computeWorldVertices(pixiMaskSource.slot, 0, worldVerticesLength, this.clippingVertAux, 0, 2);
mask.clear().lineStyle(0).beginFill(0x000000);
mask.moveTo(this.clippingVertAux[0], this.clippingVertAux[1]);
for (let i = 2; i < worldVerticesLength; i+=2) {
    mask.lineTo(this.clippingVertAux[i], this.clippingVertAux[i+1]);
}
mask.finishPoly();

Where clippingVertAux is a private Float32Array:

private clippingVertAux = new Float32Array(6);

---

Do you see any problem in this?

[GordonTombola]

hi @davidetan

sorry I've only just seen this, the attachment rendering issue appears for me when I have a skeleton with an attachment on with keys to remove the attachment, not sure if you had any luck re-producing it but it was definitely continuing to render the original attachment on the slot.

I've not seen lineTo/finishPoly methods use instead of drawPolygon but assuming it works, yeah looks good, having the private array variable makes sense too!

I was using renderable over visible as I wasn't certain if spine slot objects relied upon the slot mesh to keep their transforms up to date but if it doesn't yeah visible will do all the same.

The only thing is making sure the visibility is set to true again when it is expecting to be rendered

[GordonTombola]

The main reason I'd written the method:

protected hasMeshForSlot(slot: Slot) {
    return this.meshesCache.has(slot);
}

was due to the overridable nature of the getMeshForSlot method, I didn't want to use this.meshesCache directly only to have someone override getMeshForSlot and have a developer confused why their mesh caching isn't working

[davidetan]

@GordonTombola I've pushed the changed I mention before and eventually kept the hasMeshForSlot.

Since I was not able to repro the first issue, can you try if it still fix it with the changes I made? Thx!

[GordonTombola]

will do, I'll message back once I've tested

[GordonTombola]

yeah, working brilliantly @davidetan

[davidetan]

Cool, merged 🎉 I'll release 4.2.58 in a moment.