fix: Bug in loading SVG though a spritesheet

[AnkushSarkar10]

Description of change

Fixed a bug where asset.data was undefined when loading a spritesheet which links to an svg as such

(spritessheet.json)

{
  "frames": {
    "default": {
         ...
    }
  },
  "meta": {
    "app": "...",
    "version": "...",
    "image": "someSvg.svg",
    "format": "..."
  }
}

Pre-Merge Checklist

• Lint process passed (npm run lint)
• Tests passed (npm run test)

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6b41d1e:

Sandbox	Source
pixi.js-sandbox	Configuration

[bigtimebuddy]

Would it be possible to add a simple test for this?

[bigtimebuddy]

Small suggestion to keep asset immutable.

Suggested change

	asset.data = asset.data ?? this.config;
	if (asset.data.parseAsGraphicsContext)
	const data = asset.data ?? this.config;
	if (data.parseAsGraphicsContext)

[AnkushSarkar10]

then if would fail in loadAsTexture() line 124 same file. For some reason asset.data is not set when loading an svg through a spritesheet (if i load the same svg file using await Asset.load("svgfile.svg") asset.data is there). I dont think adding this would break anything (it's just meta data).

[AnkushSarkar10]

I was trying to add a test case but I am not able to run the test suit for some reason. It just stuck here

I ran npm run start to build then ran npm test. I don't know what I am missing any suggestions?

I also tried running the test file i need directly, same thing.

[AnkushSarkar10]

then if would fail in loadAsTexture() line 124 same file. For some reason asset.data is not set when loading an svg through a spritesheet (if i load the same svg file using await Asset.load("svgfile.svg") asset.data is there). I dont think adding this would break anything (it's just meta data).

pixijs/src/assets/loader/Loader.ts

Line 111 in 54029ce

asset = await parser.load(url, data, this);

this line does not add data to asset for some reason

[bigtimebuddy]

If we need to reassign asset.data it means there's likely some other problem upstream from this.

[AnkushSarkar10]

Small suggestion to keep asset immutable.

done

[AnkushSarkar10]

@bigtimebuddy let me know if this looks good

[AnkushSarkar10]

@bigtimebuddy added a test case, but i can't run them so not sure if it's correct (it looks like it should pass)

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 34276d7:

Sandbox	Source
pixi.js-sandbox	Configuration

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 150deeb:

Sandbox	Source
pixi.js-sandbox	Configuration

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ab07198:

Sandbox	Source
pixi.js-sandbox	Configuration

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 29c3727:

Sandbox	Source
pixi.js-sandbox	Configuration

[bigtimebuddy]

Thanks for the test case. I talked with @Zyie about this earlier and we agreed that there is something upstream that needs to be fixed. asset.data should exist. He thought it might be because the spritesheet images go through the loader.load() not through Assets.load(). That might account for the difference in not setting the data. Either way, that's probably the correct fix.

[AnkushSarkar10]

I think we just need to call this at some point upstream (maybe right before the parser.load??).

This is from Assets.ts

@bigtimebuddy @Zyie

[AnkushSarkar10]

any updates on this @bigtimebuddy @Zyie

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit dbdf200:

Sandbox	Source
pixi.js-sandbox	Configuration

[AnkushSarkar10]

@bigtimebuddy @Zyie fixed upsteam

[AnkushSarkar10]

[Zyie]

Closing this in favor of #10694 as it more directly fixes the issue, but thank you for pointing me in the right direction!