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

Bug: Resolver ignores resolution of asset in v8? #10108

Closed
gabrielecirulli opened this issue Jan 12, 2024 · 5 comments
Closed

Bug: Resolver ignores resolution of asset in v8? #10108

gabrielecirulli opened this issue Jan 12, 2024 · 5 comments
Assignees
Labels

Comments

@gabrielecirulli
Copy link

Current Behavior

Using Pixi v8, I'm unable to load an asset of the desired resolution when creating a sprite using code such as:

new Sprite(Texture.from(textureId));

Take the following manifest as an example. It is generated by a slightly modified version of assetpack which adds the format of the underlying images to a SpriteSheet JSON file's name:

{
  "bundles": [
    {
      "name": "default",
      "assets": []
    },
    {
      "name": "textures/house",
      "assets": [
        {
          "alias": [
            "textures/house/house.json"
          ],
          "src": [
            "textures/house/house@0.1x.png.json",
            "textures/house/house@0.2x.png.json",
            "textures/house/house@0.3x.png.json",
            "textures/house/house@1x.png.json"
          ],
          "data": {
            "tags": {
              "m": true,
              "tps": true
            }
          }
        }
      ]
    }
  ]
}

Note how the src property includes 4 different resolutions. Due to how I'm packaging my assets, @1x is actually 10 times the size of the "natural" size of the assets, therefore 0.1x would actually be a @1x asset, 0.2x would be a @2x asset, and so on.

I initialize Assets as follows:

await Assets.init({
      manifest: "/assets/manifest.json",
      basePath: "/assets",
      texturePreference: { resolution: 0.3 },
    });

Note how by setting texturePreference.resolution to 0.3 I am signaling that I want textures/house/house@0.3x.png.json to be used.

This worked fine in Pixi.js v7 but doesn't seem to work in v8.

Stepping through the execution of Pixi's Resolver with a debugger, I noticed the following:

  • The resolve method of Resolver.ts looks for an asset matching the resolution 0.3 here.
  • When inspecting each asset of assets (defined as this._assetMap[key]), the asset has the following shape:
{
    "src": "/assets/textures/house/house@0.3x.png.json",
    "alias": [
        "textures/house/house.json",
        "textures/house-textures/house/house.json"
    ],
    "data": {
        "tags": {
            "m": true,
            "tps": true
        }
    },
    "format": "json"
}

Notice how the asset object has a format property, which is used by the preferredOrder logic to check for a match. However, it doesn't have a resolution property. resolution is what would be used to identify whether the asset matches the desired resolution.

Due the resolution property missing, the resolver defaults to this._resolverHash[key] = assets[0]; (in my case picking the 0.1x resolution asset).

Digging deeper, within the add there is logic for detecting the "retina" resolution of an asset, but in my specific case it's not being triggered.

  • Within add there is a section where this._parsers are used on the asset.
  • The one parser present in my case is { "extension": "resolve-parser", parse: Function, test: Function }
  • The parse function is defined in resolveTextureUrl. Notice how it sets a resolution field if it detects a resolution in the file name.
  • However before parse is run, test must run successfully
  • Inspecting test, we see that it's loadTextures.test
  • The relevant part of it is the call to checkExtension. Note how it checks that the extension is within ['.jpeg', '.jpg', '.png', '.webp', '.avif'].
  • This check fails because the extension is .json.

Expected Behavior

The correct resolution asset is loaded

Steps to Reproduce

The first defined (lowest) resolution asset is loaded instead.

Environment

  • pixi.js version: 8.0.0-rc.3

Possible Solution

Consider adding .json as a valid image format within loadTextures.test or switching to a different method of testing the asset.

Additional Information

This worked fine in v7 but must have been broken due to the extensive refactoring of v8

@GoodBoyDigital
Copy link
Member

heya! thankyou for posting, Im not sure .json extension is considered a valid image format?

@GoodBoyDigital
Copy link
Member

if the names were changed from textures/house/house@0.1x.png.json -> textures/house/house@0.1x.png do things work as expect? Thank you for checking!

@Zyie
Copy link
Member

Zyie commented Jan 12, 2024

it looks like you are using assetpack, so I think you need to add a json resolver so pixi can tell what resolution it is
There is a small section of docs about it here

and here is the code snippet

import { Resolver, extensions, resolveTextureUrl, ResolveURLParser, ExtensionType } from 'pixi.js';

export const resolveJsonUrl = {
    extension: ExtensionType.ResolveParser,
    test: (value: string): boolean =>
        Resolver.RETINA_PREFIX.test(value) && value.endsWith('.json'),
    parse: resolveTextureUrl.parse,
} as ResolveURLParser;

extensions.add(resolveJsonUrl);

it is also interesting that this worked in v7 but not v8. I dont believe much was changed between the two versions when it comes to assets

@Zyie Zyie added the v8 label Jan 12, 2024
@gabrielecirulli
Copy link
Author

gabrielecirulli commented Jan 13, 2024

@Zyie's solution works. I think where I got side-tracked is that back when I was attempting to use assetpack with Pixi.js, after extensive debugging I realized that resolutions were not being considered because assetpack would output names such as filename@2x.json instead of filename@2x.png.json, which is the only pattern that Pixi v7 would recognize. That's why I modified assetpack to support this, but such modification may not make sense anymore in light of the ability to add a custom resolver.

It may be worth documenting whether the above approach works with both v7 and v8, or only with v8.

@Zyie
Copy link
Member

Zyie commented Jan 30, 2024

I've added the json resolver by default now, so hopefully all is fixed

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

No branches or pull requests

3 participants