Add PixiJS v8 support to spine

[GoodBoyDigital]

This is an inital PR for the Spine v8 Pixi runtime. Basically a copy of this guy.

[davidetan]

Sorry if I wasn't able to merge this yet. I'm slowly working on it, but I found a couple of issues that I haven't had the time to fix yet. Especially one, where two different gameobject created from the same assets have some conflicts with the textures.

[GoodBoyDigital]

Sorry if I wasn't able to merge this yet. I'm slowly working on it, but I found a couple of issues that I haven't had the time to fix yet. Especially one, where two different gameobject created from the same assets have some conflicts with the textures.

no worries at all! If you have an example - I would be happy to take a look?

[davidetan]

Sorry if I wasn't able to merge this yet. I'm slowly working on it, but I found a couple of issues that I haven't had the time to fix yet. Especially one, where two different gameobject created from the same assets have some conflicts with the textures.

no worries at all! If you have an example - I would be happy to take a look?

I've pushed a repro example in spine-pixi-v8/example/dragon.html. I've added a small UI to manually advance time. If you advance time for the first skeleton, when the texture of the sequence is changed, also the second skeleton has a texture change or UVs.

I guess the problem is related to the fact that the cache data shares the underlying data structure.

[GoodBoyDigital]

hey @davidetan pushed up a fix! We need to clone the uvs of the attachments rather than using the attachment.uvs directly as they can change when reused across animations. Pushed up a little fix directly!

[davidetan]

Awesome! We usually copy uvs, but I wanted to see if we could actually avoid that.

I'll test it a little more with other use cases, but I think that we're ready to merge.

[GoodBoyDigital]

Awesome! We usually copy uvs, but I wanted to see if we could actually avoid that.

I'll test it a little more with other use cases, but I think that we're ready to merge.

good stuff - would have loved to avoid that too :/ I guess if we know there is one model we could remove the copy op?

fingers crossed the other use cases all work as intended!

[davidetan]

I guess if we know there is one model we could remove the copy op?

I guess that might be ok, but at the cost of counting the skeleton instances using the same data.

[GoodBoyDigital]

I guess if we know there is one model we could remove the copy op?

I guess that might be ok, but at the cost of counting the skeleton instances using the same data.

let me have a think on this..

[GoodBoyDigital]

heya! I think theres no easy way around the copy, as the array the cache data holds onto is modified by the runtime outside of our control.

I also pushed a small tweak that will make the BatchableSpineSlot compatible with a future release of v8!

[davidetan]

heya! I think theres no easy way around the copy, as the array the cache data holds onto is modified by the runtime outside of our control.

We'll keep this in mind and see in the future if this might be a problem.

I also pushed a small tweak that will make the BatchableSpineSlot compatible with a future release of v8!

Awesome! I didn't find any other particular problem apart from my commit of yesterday.

I just want to compare this with our existing spine-pixi(-v7) runtime and align the differences. For example, the Spine.from:

• v8: static from ({ skeleton: string, atlas: string, scale = 1 }): Spine
• v7: static from (skeletonAssetName: string, atlasAssetName: string, options?: { scale?: number, autoUpdate?: boolean, slotMeshFactory?: () => ISlotMesh } } ): Spine

That makes documenting the runtimes easier.

[GoodBoyDigital]

heya! I think theres no easy way around the copy, as the array the cache data holds onto is modified by the runtime outside of our control.

We'll keep this in mind and see in the future if this might be a problem.

I also pushed a small tweak that will make the BatchableSpineSlot compatible with a future release of v8!

Awesome! I didn't find any other particular problem apart from my commit of yesterday.

I just want to compare this with our existing spine-pixi(-v7) runtime and align the differences. For example, the Spine.from:

• v8: static from ({ skeleton: string, atlas: string, scale = 1 }): Spine
• v7: static from (skeletonAssetName: string, atlasAssetName: string, options?: { scale?: number, autoUpdate?: boolean, slotMeshFactory?: () => ISlotMesh } } ): Spine

That makes documenting the runtimes easier.

Ah yes. v8 tends to embrace single option objects a lot more than v7. We could update both to use the more modern single object and have deprecation for the current v7 method (on both)?

[davidetan]

Yes, I also wanted to do something like that 👍

[davidetan]

@GoodBoyDigital I've noticed another difference in how we usually render skeletons using black tint.

In spine-pixi-v8, we switch batchers each time we encounter a slot using black tint. While it's unlikely that consecutive slots use dark tint, there's a good chance that non-consecutive slots do. This implies constant switching between the normal batcher and the dark tint batcher.

For example, with a skeleton composed of 4 slots - normal, dark, normal, dark - the renderer will continuously switch batchers. As a result, no batching will occur at all.

Typically, we instead render the whole skeleton using the dark tint batcher if at least one slot uses black tint. With this strategy, the previous example would render all four slots in a single draw call. In spine-pixi(-v7), users can decide which batcher to use for each skeleton. If no option is provided, the batcher is selected using the strategy mentioned above (see here).

What's your opinion about this?

[GoodBoyDigital]

@GoodBoyDigital I've noticed another difference in how we usually render skeletons using black tint.

In spine-pixi-v8, we switch batchers each time we encounter a slot using black tint. While it's unlikely that consecutive slots use dark tint, there's a good chance that non-consecutive slots do. This implies constant switching between the normal batcher and the dark tint batcher.

For example, with a skeleton composed of 4 slots - normal, dark, normal, dark - the renderer will continuously switch batchers. As a result, no batching will occur at all.

Typically, we instead render the whole skeleton using the dark tint batcher if at least one slot uses black tint. With this strategy, the previous example would render all four slots in a single draw call. In spine-pixi(-v7), users can decide which batcher to use for each skeleton. If no option is provided, the batcher is selected using the strategy mentioned above (see here).

What's your opinion about this?

I like that, lets stick to the same approach as v7 of using the regular batcher, unless a darktint slot is found - in which case we make everything darktint.

[davidetan]

I've pushes a couple of changes.

In the first one, as discussed above, dark tint is enabled for the whole skeleton if it has at least a slot using tint black.

In the second commit, I've used the the clipTrianglesUnpacked that does not pack attributes as we usually use, but provides clipped vertices, uvs and indices in three arrays.

[davidetan]

@GoodBoyDigital I've noticed a bug while using clipping.

Some triangles appears where there should not be like in the images below:

I've locally reverted my two previous commits I did ealier, but the problem still occur, so it's not something that I've introduced, but something that we did not noticed earlier.

Right now in updateClippingData, we set spineAttachmentsDirty only if vertices/indices count changed. If I set spineAttachmentsDirty always to true, the rendering happens as expected.

I'll check better tomorrow, but I was wondering if there's any caveat on positions/indices/uvs when we use updateRenderable rather than addRenderable.

[GoodBoyDigital]

@GoodBoyDigital I've noticed a bug while using clipping.

Some triangles appears where there should not be like in the images below:

I've locally reverted my two previous commits I did ealier, but the problem still occur, so it's not something that I've introduced, but something that we did not noticed earlier.

Right now in updateClippingData, we set spineAttachmentsDirty only if vertices/indices count changed. If I set spineAttachmentsDirty always to true, the rendering happens as expected.

I'll check better tomorrow, but I was wondering if there's any caveat on positions/indices/uvs when we use updateRenderable rather than addRenderable.

heya! We are so close :) is there a specific example? or situation where this occours? Thanks!

[davidetan]

I've just pushed a repro example called "coin.html", where you can slowly move the time (use the mouse wheel or trackpad scrool to be more precise) and see the issue.

[davidetan]

In the indices copy loop https://github.com/GoodBoyDigital/spine-runtimes/blob/pixi-v8/spine-ts/spine-pixi-v8/src/Spine.ts#L491, I've noticed that if set the spineAttachmentsDirty to false when an index differs from the previous one, the issue is gone.

for (let i = 0; i < indices.length; i++) {
    if (indices[i] !== clippedTriangles[i]) {
        this.spineAttachmentsDirty = true;
    }
    indices[i] = clippedTriangles[i];
}

I'm not sure though if that is a side effect, or is it correct to invalidate the renderable when indices change.

[GoodBoyDigital]

In the indices copy loop https://github.com/GoodBoyDigital/spine-runtimes/blob/pixi-v8/spine-ts/spine-pixi-v8/src/Spine.ts#L491, I've noticed that if set the spineAttachmentsDirty to false when an index differs from the previous one, the issue is gone.

for (let i = 0; i < indices.length; i++) {
  if (indices[i] !== clippedTriangles[i]) {
      this.spineAttachmentsDirty = true;
  }
  indices[i] = clippedTriangles[i];
}

I'm not sure though if that is a side effect, or is it correct to invalidate the renderable when indices change.

You are spot on - if the length of indices changes then we should set this.spineAttachmentsDirty = true; we care about the length change - rather than the values inside, so could be an easier check!

if(indices.length !== clippedTriangles.length) this.spineAttachmentsDirty = true

[davidetan]

In the indices copy loop https://github.com/GoodBoyDigital/spine-runtimes/blob/pixi-v8/spine-ts/spine-pixi-v8/src/Spine.ts#L491, I've noticed that if set the spineAttachmentsDirty to false when an index differs from the previous one, the issue is gone.

for (let i = 0; i < indices.length; i++) {
    if (indices[i] !== clippedTriangles[i]) {
        this.spineAttachmentsDirty = true;
    }
    indices[i] = clippedTriangles[i];
}

I'm not sure though if that is a side effect, or is it correct to invalidate the renderable when indices change.

You are spot on - if the length of indices changes then we should set this.spineAttachmentsDirty = true; we care about the length change - rather than the values inside, so could be an easier check!

if(indices.length !== clippedTriangles.length) this.spineAttachmentsDirty = true

Some lines above we already check it: https://github.com/GoodBoyDigital/spine-runtimes/blob/pixi-v8/spine-ts/spine-pixi-v8/src/Spine.ts#L465

const verticesCount = clippedVertices.length / 2;
const indicesCount = clippedTriangles.length;

...

const clippedData = cacheData.clippedData;

const sizeChange = clippedData.vertexCount !== verticesCount || indicesCount !== clippedData.indicesCount;

cacheData.skipRender = verticesCount === 0;

if (sizeChange) {
    this.spineAttachmentsDirty = true;

    if (clippedData.vertexCount < verticesCount) {
        // buffer reuse!
        clippedData.vertices = new Float32Array(verticesCount * 2);
        clippedData.uvs = new Float32Array(verticesCount * 2);
    }

    if (clippedData.indices.length < indicesCount) {
        clippedData.indices = new Uint16Array(indicesCount);
    }
}

...

clippedData.vertexCount = verticesCount;

...

clippedData.indicesCount = indicesCount;

But that does not seem to be enough. Am I missing something?

[GoodBoyDigital]

ahhh, actually you are correct - if the indicies change, we should invalidate. As the indicies are assumped to be constant between 'update' renders.

Options

• lets set it so that this.spineAttachmentsDirty = true if there is a clipping mask. This would work well if the indicies change a lot when clipping and also if the indicies are large)

• We could check the indicies values for differences and flag this.spineAttachmentsDirty = true if a value differs. This would be best if the indicies change less frequently, are shorter and clipping masks are less frequent (eg 1 or two per animation). We could probably write a fast check if the arrays are typed arrays too (as we can do 64bit checking!).

leaning towards option 2 myself! Do you have a preference?

[davidetan]

Ok, glad we found the issue! I've no particular preferences and both use cases can occur. In general, we recommend to use clipping attachments less as possible since the vertices/indices calculation is heavy. But users still use them a lot :)

[GoodBoyDigital]

Ok, glad we found the issue! I've no particular preferences and both use cases can occur. In general, we recommend to use clipping attachments less as possible since the vertices/indices calculation is heavy. But users still use them a lot :)

haha, i know that feeling :)

[davidetan]

We could check the indicies values for differences and flag this.spineAttachmentsDirty = true if a value differs. This would be best if the indicies change less frequently, are shorter and clipping masks are less frequent (eg 1 or two per animation). We could probably write a fast check if the arrays are typed arrays too (as we can do 64bit checking!).

Unfortunately the clipper does not use typed arrays :( But I'll keep this in mind since we could actually start to use them for the clipper too. For now, I just stick to what I showed you before, we check for differences while copying.

[davidetan]

@GoodBoyDigital I made another commit to implement a final missing feature. Several users requested that slot objects added through addSlotObject be clipped by clipping attachments, as if they were actual Spine attachments. This feature is present in spine-pixi(-v7), so I reimplemented it in a similar way.

The approach uses the vertices that form the clipping attachment perimeter to define a Pixi Graphics mask. Since the clipping mask can be animated, vertices can change position each frame, the Pixi mask vertices need to be updated accordingly.

I'd really appreciate if you could take a look at it and comment on the implementation/performance.

[GoodBoyDigital]

@GoodBoyDigital I made another commit to implement a final missing feature. Several users requested that slot objects added through addSlotObject be clipped by clipping attachments, as if they were actual Spine attachments. This feature is present in spine-pixi(-v7), so I reimplemented it in a similar way.

The approach uses the vertices that form the clipping attachment perimeter to define a Pixi Graphics mask. Since the clipping mask can be animated, vertices can change position each frame, the Pixi mask vertices need to be updated accordingly.

I'd really appreciate if you could take a look at it and comment on the implementation/performance.

heya! good stuff! il aim to take a look this side of the week! :)

[davidetan]

@GoodBoyDigital I made another commit to implement a final missing feature. Several users requested that slot objects added through addSlotObject be clipped by clipping attachments, as if they were actual Spine attachments. This feature is present in spine-pixi(-v7), so I reimplemented it in a similar way. The approach uses the vertices that form the clipping attachment perimeter to define a Pixi Graphics mask. Since the clipping mask can be animated, vertices can change position each frame, the Pixi mask vertices need to be updated accordingly. I'd really appreciate if you could take a look at it and comment on the implementation/performance.

heya! good stuff! il aim to take a look this side of the week! :)

That's great, thanks! I didn't find any more issues or differences with v7, so I'm officially updating the documentation and preparing the news for the release 😄

[davidetan]

@GoodBoyDigital I think we're ready to merge. If you agree, I'll go ahead with it. I'll reach out to you directly about the blog post! :)

[GoodBoyDigital]

@GoodBoyDigital I think we're ready to merge. If you agree, I'll go ahead with it. I'll reach out to you directly about the blog post! :)

Nice work man :) lets merge!

[SerG-Y]

Great job, guys! @davidetan, do you know the release date?

[davidetan]

Great job, guys! @davidetan, do you know the release date?

Soon, probably today 👍