GetRemappedClone on MeshAttachments / Sprite Pivots

callumtw

Hi, we've hit an issue with the Unity runtime that might be a bug, or certainly seems like strange behaviour, so would appreciate some information on how to work around it. Apologies for the long post, but I'll do my best to explain everything as clearly as possible.

In our game, characters can equip many different weapons, so our character skeletons have various region attachments that serve as weapon templates. At runtime, we enable the right template for a given weapon (small / large etc), and then take the Weapon's Unity Sprite and get a remapped clone of the template attachment to use as the "actual" attachment. We use the GetRemappedClone() method for this. With region attachments, we are able to adjust the pivot point of each unity Sprite, which is then used as a positional offset on the cloned attachment - this is all working great / produces the desired behaviour.

We've now come to need one such template to be a MeshAttachment. I was expecting this to work fine, as GetRemappedClone() has dedicated logic for MeshAttachments, and can even create a linked mesh so animation carries over. However, we then found the cloned attachments had incorrect UVs. After writing a tool to diagnose this, we discovered that a Sprite's pivot point will offset the cloned MeshAttachment's UVs, rather than the vertex position. I believe this is caused by SetRegion() setting the regionOffsetX/Y, when presumably for meshes it shouldn't.

In case it's of any consequence, after all of the character's skins are enabled, and the weapon attachments have been cloned, we bake the active skin down to a single atlas / material, using GetRepackedSkin(). However, skipping this step still results in incorrect UVs on the cloned attachments.

In this image, you can see the difference between the UVs.

So my questions are:

Is a Unity Sprite's pivot point corresponding to regionOffsetX/Y on MeshAttachments expected behaviour? If so, why does the behaviour not match the positional behaviour of RegionAttachments?
How would you recommend I offset the UVs for this attachment back to their correct position? I found that setting regionOffsetX/Y to zero in SetRegion() would result in correct UVs for the cloned attachment, but would be incorrect after repacking the skin.
How would you recommend I offset the position of the Mesh Attachment using the Sprite's pivot, so that I get the same behaviour as with RegionAttachments? My guess is I need to update the vertex positions, but any info on how this offset is usually "scaled" for the skeleton, or whether this could interfere with animation, would be great.
This is a side question, but the resulting cloned attachment (with cloneMeshAsLinked set to true) is not inheriting animation like it says it should. This one could be an issue with how we've set some stuff up in the skeleton, but I thought it might be worth asking if there were any known issues with animation on cloned MeshAttachments.

Extra Info:
We're using spine-unity-3.8-2019-09-27. If this has been fixed in a later release, I'd really appreciate if you could link the relevant commits so we can make the changes ourselves, as we're fairly tied down with this project / probably can't update and replace the package.

Thanks for any help you can provide!

[Edit as of 17:28]

Got a workaround going. I created a modified version GetRemappedClone() and replaced its call to SetRegion() with a modified version of that method as well. In the new SetRegion, I set the attachment's regionOffsetX/Y to zero, and the region's offsetX/Y both to zero before calling UpdateUVs(), which continues to work after repacking.

/// <summary>Sets the region (image) of a MeshAttachment</summary>
public static void SetWeaponRegion(this MeshAttachment attachment, AtlasRegion region, bool updateUVs = true)
{
   if (region == null) throw new System.ArgumentNullException("region");

   // (AtlasAttachmentLoader.cs)
   attachment.RendererObject = region;
   attachment.RegionU = region.u;
   attachment.RegionV = region.v;
   attachment.RegionU2 = region.u2;
   attachment.RegionV2 = region.v2;
   attachment.RegionRotate = region.rotate;
   attachment.regionWidth = region.width;
   attachment.regionHeight = region.height;
   attachment.regionOriginalWidth = region.originalWidth;
   attachment.regionOriginalHeight = region.originalHeight;
   attachment.regionOffsetX = 0;
   attachment.regionOffsetY = 0;
   

   UpdateMeshOffset(attachment, region);

   region.offsetX = 0;
   region.offsetY = 0;
   

   if(updateUVs) attachment.UpdateUVs();
}

Before setting the region's offsetX/Y to zero, I use it to offset the attachment's vertex positions, which I managed to get working by following similar logic to RegionAttachment.UpdateOffset(). It doesn't account for all the rotation calculations that regions do, but I don't need that currently.

//Modified from RegionAttachment.UpdateOffset()
private static void UpdateMeshOffset (MeshAttachment mesh, AtlasRegion region) 
{
   float width = mesh.Width;
   float height = mesh.Height;

   float offsetX = 0f;
   float offsetY = 0f;

   if (region.originalWidth != 0) // if (region != null)
   { 
      offsetX += region.offsetX / region.originalWidth * width;
      offsetY += region.offsetY / region.originalHeight * height;
   }
   

   for (int i = 0; i < mesh.vertices.Length; i++)
   {
      mesh.vertices[i] += i % 2 == 0 ? offsetX : offsetY;
   }
}

Regarding the animation on the cloned mesh not working, I discovered that animations will not work on linked meshes moved to a different slot:

The reason for this limitation [all linked meshes in same slot] is DeformTimeline, which changes the vertices for a mesh. To find which mesh it should change, it knows what slot to look in. If a linked mesh could be in any slot, it would have to look in ALL slots for potential meshes it should change.

Frankly this is really disappointing, not only because it renders all this work entirely redundant (we're only using MeshAttachments as templates in order to get deformation), but it seems like such an obvious workflow. I couldn't see why a list of slots that required deformation couldn't be updated when the linked mesh was attached to a new slot. I began looking into doing this myself until I realised that all the animation / deform timelines are stored in the SkeletonData, so in order to track the additional slots, I'd have to modify the SkeletonData, which I can't because it's an asset in the Unity project which means modifying it would cause all sorts of issues.

This seems like a real oversight, perhaps there needs to be some way of supplying additional timelines per-skeleton. Either way, we'll probably have to scrap our plans for deforming templates, as the LinkedMesh limitation renders our template system incompatible with mesh deformation.

Harald

Thanks for your detailled writeup and diagnostic screenshots.

callumtw escribió
However, we then found the cloned attachments had incorrect UVs. After writing a tool to diagnose this, we discovered that a Sprite's pivot point will offset the cloned MeshAttachment's UVs, rather than the vertex position. I believe this is caused by SetRegion() setting the regionOffsetX/Y, when presumably for meshes it shouldn't.

Thanks for reporting, and sorry for the troubles! We'll have a look at what's going wrong and let you know as soon as we have any updates on the issue.

callumtw escribió
Either way, we'll probably have to scrap our plans for deforming templates, as the LinkedMesh limitation renders our template system incompatible with mesh deformation.

From your description above I don't see why you want to use a different Slot when switching weapons via templates. Is there a reason why you cannot use the same Slot for multiple weapon templates?

If you really need to do that, then there is the following issue ticket in this thread regarding the discussed feature:
https://github.com/EsotericSoftware/spine-editor/issues/195
You can subscribe to the ticket to receive notification upon any updates.

callumtw

Thanks for the reply! I'm fairly certain the issue is with using the Sprite pivot as the regionOffset for MeshAttachments. I say that, because I can't see a practical use for why you'd ever want the Sprite pivot to offset the UVs in this way over the position (particularly when this is inconsistent with the behaviour of RegionAttachment).

Harald escribió
From your description above I don't see why you want to use a different Slot when switching weapons via templates. Is there a reason why you cannot use the same Slot for multiple weapon templates?

Yes, I didn't mention this in my post, but each character's skeleton contains multiple poses to accomodate how they should hold the different weapon types (e.g. Swords, vs Bows, vs Crossbows, vs Shields etc). As such, there are many different hand/weapon slots (at least 8 ). We put all the weapon templates in a dedicated "WeaponTemplates" slot on the root, so there is a single entry point to them at runtime, clone them once, then iterate all the "real" slots and set the cloned attachments on them.

This avoids having 4+ different templates each in 2 sets of hand slots for several different poses, and means if we ever need to change or add a new weapon template, we only have to do it once, rather than for each slot. I should also mention this is for a live game, where we already have 6 characters set up and working with the existing system, so the cost to reconfigure them is quite significant.

Harald

Unfortunately at the moment we cannot provide the same behaviour at MeshAttachment as for RegionAttachment as it does not provide any vertex offset property that we could utilize. As the best solution for now the GetRemappedClone() now provides an additional parameter pivotShiftsMeshUVCoords for MeshAttachment to prevent uv shifts at a non-central Sprite pivot. This parameter defaults to true to maintain previous behaviour.

A new 3.8 unitypackage can be downloaded here as usual:
Spine Unity Download

The issue has been tracked under this issue ticket:
https://github.com/EsotericSoftware/spine-runtimes/issues/1809

Harald escribió
Yes, I didn't mention this in my post, but each character's skeleton contains multiple poses to accomodate how they should hold the different weapon types (e.g. Swords, vs Bows, vs Crossbows, vs Shields etc). As such, there are many different hand/weapon slots (at least 8 ). We put all the weapon templates in a dedicated "WeaponTemplates" slot on the root, so there is a single entry point to them at runtime, clone them once, then iterate all the "real" slots and set the cloned attachments on them.

Thanks for the explanation. I still don't quite understand why you don't use the following setup:

- Swords slot, one MeshAttachment that suits all swords, clone this MeshAttachment, assign at this slot.
- Bows slot, one MeshAttachment that suits all bows, clone this MeshAttachment, assign at this slot.
- Crossbows slot, same here 
- Shields slot, same here

What I don't understand is why do you need to move linked mesh attachments to another slot than the original one.

callumtw

Harald escribió
Unfortunately at the moment we cannot provide the same behaviour at MeshAttachment as for RegionAttachment as it does not provide any vertex offset property that we could utilize.

I'm not sure if it was clear because it's a lengthy post, but I've fixed this. I shared a method above, "UpdateMeshOffset" that offsets a mesh's vertices by the sprite pivot (though I just edited it to fix an issue where I was only applying half the offset, from when I was doing tests). This method works similarly to RegionAttachment's UpdateOffset(), with the exception of not applying rotation. Personally I think meshes not respecting the pivot offset is a bit of a deal breaker, at least in most of our workflows, so this method can be called at a similar point to when RegionAttachments have UpdateOffset() called. I appreciate if this isn't "allowed" in the interests of API consistency though.

Harald escribió

Thanks for the explanation. I still don't quite understand why you don't use the following setup:

- Swords slot, one MeshAttachment that suits all swords, clone this MeshAttachment, assign at this slot.
- Bows slot, one MeshAttachment that suits all bows, clone this MeshAttachment, assign at this slot.
- Crossbows slot, same here 
- Shields slot, same here

What I don't understand is why do you need to move linked mesh attachments to another slot than the original one.

There isn't just "one" slot where the weapons need to appear, because of the poses. I think I forgot to explain that a "pose" here is essentially a nested skeleton, because the pose needed to wield a crossbow is significantly different to the pose needed for a 2H sword, so it has different meshes and bones for the torso and arms, (as can be seen from the atlas images), and in turn it also has a different pair of hand bones and weapon slots.

We have a few key "poses" (nested skeletons); Idle, Attack, ReceiveDamage, but these each have variations like Idle_1H, Idle_2H, Idle_Bow, Idle_Crossbow, Attack_1H, Attack_2H, Attack_Bow etc.

So either you do what we're doing already, and have one slot to hold all the weapon templates, clone the appropriate templates, and move them to each pose's weapon slots, or you have to have a set of all those^ weapon templates for each "hand" slot in each pose. If we were to do that, with the 4 weapons listed above (and we have more than 4), we'd have to maintain 48 individual weapon slots/templates (4 weapon slots per hand, 2 hands per pose, 6 poses in total), which just isn't practical.

If this still isn't making sense, it's probably not worth your time to continue discussing it here, honestly. I think our needs necessitate quite a different setup to most users!

Harald

callumtw escribió
I'm not sure if it was clear because it's a lengthy post, but I've fixed this. I shared a method above, "UpdateMeshOffset" that offsets a mesh's vertices by the sprite pivot (though I just edited it to fix an issue where I was only applying half the offset, from when I was doing tests). This method works similarly to RegionAttachment's UpdateOffset(), with the exception of not applying rotation.

I have seen your code, thanks for posting it. The problem is that your code is modifying the MeshAttachment's mesh.vertices:

mesh.vertices[i] += i % 2 == 0 ? offsetX : offsetY;

This is not a solution that we can use in the official runtime since it will also modify the parent's vertices when cloning using linked meshes, or when the MeshAttachment is already a linked mesh. The clean solution would be to add two offset parameters to either the existing MeshAttachment class or to create a subclass OffsetMeshAttachment with these additional parameters. Then the offset values would be used in a modified version of ComputeWorldVertices().

Since modifications to one of the core runtime shall be ported to all runtimes, it's something that needs to be well thought through. Therefor I'm asking whether you have considered potential alternatives. I think I can now imagine what your setup looks like.

callumtw

Harald escribió
This is not a solution that we can use in the official runtime since it will also modify the parent's vertices when cloning using linked meshes, or when the MeshAttachment is already a linked mesh.

Gotcha, thanks for explaining. I considered wrapping this and adding an additional offset parameter for ComputeWorldVertices(), but I wasn't sure how frequently that method was called, and wanted to ensure I only calculated that offset once / on initialisation. As we're only ever cloning these templates once, the potential issues with linked meshes don't affect us.

Sorry if I appear stubborn to change our setup; we spent a good amount of time considering the dis/advantages of different configurations, and this was by far the best for us. Since we have a lot of work invested in that setup now, there's a high cost to changing it 🙂. Had we been aware that deformation data was stored per-slot, we might have taken a different route, but foregoing mesh attachments, albeit a shame, is mostly just foregoing a small amount of polish (most weapons are rigid, after all!).

Harald

callumtw escribió
Sorry if I appear stubborn to change our setup; we spent a good amount of time considering the dis/advantages of different configurations, and this was by far the best for us.

Sorry if I came across that way, I did not intend to say "that's not supported, don't do that". We are happy to improve the runtimes where we can, we just need to understand the use case to be sure that 1) it makes sense to us as well and 2) so that any improvements also fit the requirements. Inconsistent behaviour between RegionAttachment and MeshAttachment is basically enough reason, but it helps even more to know what you are working for 🙂.