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

[as3] TexturePacker atlases don't work #939

Closed
badlogic opened this issue Jul 11, 2017 · 11 comments
Closed

[as3] TexturePacker atlases don't work #939

badlogic opened this issue Jul 11, 2017 · 11 comments
Assignees

Comments

@badlogic
Copy link
Collaborator

See http://esotericsoftware.com/forum/AS3-Starling-runtime-Rendering-problem-8969, repro in PM.

@badlogic badlogic self-assigned this Jul 11, 2017
@badlogic
Copy link
Collaborator Author

@vync79
Copy link

vync79 commented Jul 14, 2017

Hello,
I did some investigations on my side and here is what I found:

  • a display bug with Starling runtime / TexturePacker atlases appeared with the commit of March 10th:
    [starling] Added two color tinting. Closes part of #843

  • when using the TexturePacker format, it looks like only the mesh types are rendered properly whereas the standard region types of attachment are broken.

For instance, the image below show a small example using the spine goblins skeletons:

  • on the left, it is using the Spine atlas format
  • on the right this is the TexturePacker atlas version.
    All mesh attachments are fine on both cases however the "shield" is wrong with the TexturePacker version.
    image

[UPDATE]
Actually looking at it a bit more in details, I found the root cause and a possible fix:

Root Cause:
With the March 10th integration, you're now always relying on a SkeletonMesh renderer instead of an Image for standard non-mesh region attachments.
When calling mesh.setTexCoords(ii, uvs[iii], uvs[iii + 1]); in the SkeletonSprite.render method, the mesh relies on its MeshStyle.setTexCoords method as follow:

/** Sets the texture coordinates of the vertex at the specified index to the given values. */
        public function setTexCoords(vertexID:int, u:Number, v:Number):void
        {
            if (_texture) _texture.setTexCoords(_vertexData, vertexID, "texCoords", u, v);
            else _vertexData.setPoint(vertexID, "texCoords", u, v);
            setVertexDataChanged();
        }

In the atlas case, the MeshStyle has a texture which is an actual SubTexture object.
The call to _texture.setTexCoords internally handles a conversion from the local subtexture UVs to the global UVs of the root atlas texture.
The problem is that the UV values that you are sending to it were already converted to the global UV space in StarlingAtlasAttachmentLoader.newRegionAttachment method as follow:

if (subTexture) {
        var root : Texture = subTexture.root;
        var rectRegion : Rectangle = atlas.getRegion(path);        
        if (!rotated) {
          attachment["regionU"] = rectRegion.x / root.width;
          attachment["regionV"] = rectRegion.y / root.height;
          attachment["regionU2"] = (rectRegion.x + subTexture.width) / root.width;
          attachment["regionV2"] = (rectRegion.y + subTexture.height) / root.height;
        } else {
          attachment["regionU2"] = rectRegion.x / root.width;
          attachment["regionV2"] = rectRegion.y / root.height;
          attachment["regionU"] = (rectRegion.x + subTexture.width) / root.width;
          attachment["regionV"] = (rectRegion.y + subTexture.height) / root.height;
        }
        attachment.setUVs(attachment["regionU"], attachment["regionV"], attachment["regionU2"], attachment["regionV2"], atlas.getRotation(path));
      }

In short, the UV of the region attachment are zoomed in twice.

Fix Proposal:
Simply remove the local to global UV conversion done in StarlingAtlasAttachmentLoader.newRegionAttachment method

The code would even become simpler by unifying SubTexture and Texture cases as follow:

  public function newRegionAttachment(skin : Skin, name : String, path : String) : RegionAttachment 
  {
      var texture : Texture = atlas.getTexture(path);      
      if (texture == null)
        throw new Error("Region not found in Starling atlas: " + path + " (region attachment: " + name + ")");
      var attachment : RegionAttachment = new RegionAttachment(name);
      var rotated : Boolean = atlas.getRotation(path);      
      attachment.rendererObject = new Image(Texture.fromTexture(texture)); // Discard frame.
      var frame : Rectangle = texture.frame;
      attachment.regionOffsetX = frame ? -frame.x : 0;
      attachment.regionOffsetY = frame ? -frame.y : 0;
      attachment.regionWidth = texture.width;
      attachment.regionHeight = texture.height;
      attachment.regionOriginalWidth = frame ? frame.width : texture.width;
      attachment.regionOriginalHeight = frame ? frame.height : texture.height;
      if (rotated) {
        var tmp : Number = attachment.regionOriginalWidth;
        attachment.regionOriginalWidth = attachment.regionOriginalHeight;
        attachment.regionOriginalHeight = tmp;
        tmp = attachment.regionWidth;
        attachment.regionWidth = attachment.regionHeight;
        attachment.regionHeight = tmp;
      }
      if (!rotated) {
        attachment["regionU"] = 0;
        attachment["regionV"] = 0;
        attachment["regionU2"] = 1;
        attachment["regionV2"] = 1;
      } else {
        attachment["regionU2"] = 0;
        attachment["regionV2"] = 1;
        attachment["regionU"] = 1;
        attachment["regionV"] = 0;
      }
      attachment.setUVs(attachment["regionU"], attachment["regionV"], attachment["regionU2"], attachment["regionV2"], atlas.getRotation(path));
      return attachment;
    }

I checked that it fixes the problem with "non-rotated" case:
image

Cheers,
Vince.

@badlogic
Copy link
Collaborator Author

Hi Vince,

Thanks for all your work! I'll try to get to this today and incorporate it in the runtime and hopefully also fix the rotated case.

Cheers!
Mario

@vync79
Copy link

vync79 commented Jul 17, 2017

Hi Mario,
You're welcome, I'm glad I could help.
Thanks for your fantastic framework and the efforts you make to keep it compatible with such a list of runtimes!
Bye,
Vince

@evilbert103
Copy link

Nice find and quick update, I was also exploring this issue so much appreciated!

I believe I'm honing in on another issue in the RegionAttachment. I've been playing with a spine animation that runs a sprite sheet animation internally (not ideal but nonetheless). When it runs in spine the object that should sit stationary is moving around as the frames swap (5-10 pixels along both axis).

My concern is that the trig math in RegionAttachment's updateOffset doesn't work unless the distance to each corner of the drawing rectangle is the same length. Once the image frame contains regionOffsetX and regionOffsetY (unless the image was trimmed evenly from all sides) this would be the case. I ran a test export from TexturePacker that does not trim any alpha from assets and sure enough my animation looked perfect.

If this is the case, it could also be causing a lot of very slight, nearly unnoticeable issues throughout many people's animations. The trig math in there was non-trivial to me so I'm going to walk through it and see if I can't figure out another solution.

@badlogic
Copy link
Collaborator Author

badlogic commented Jul 17, 2017 via email

@aram-ahak
Copy link

@badlogic badlogic reopened this Jul 18, 2017
@badlogic
Copy link
Collaborator Author

Sorry @aram-ahak I'm looking into it.

@badlogic badlogic added the bug label Jul 19, 2017
@badlogic
Copy link
Collaborator Author

@aram-ahak this doesn't appear to be an issue with the atlas. Using the Spine texture atlas also produces the issue. Investigating.

@badlogic
Copy link
Collaborator Author

This is a bug in all Spine runtimes, flipping and shear don't work together as they should. @aram-ahak I opened a new issue here #951

@NathanSweet NathanSweet removed the bug label Jul 19, 2017
@badlogic
Copy link
Collaborator Author

@aram-ahak i've fixed the issue you ran into, please update your runtime copy. See #951

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

No branches or pull requests

5 participants