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

feat: [7877] Allow loading SVG with viewBox only #7878

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 58 additions & 6 deletions packages/core/src/textures/resources/SVGResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ export class SVGResource extends BaseImageResource
resolve(this);
};

const svgString = this.svg;

// Convert SVG inline string to data-uri
if (SVGResource.SVG_XML.test(this.svg.trim()))
{
Expand All @@ -126,18 +128,19 @@ export class SVGResource extends BaseImageResource
(this as any).svg = `data:image/svg+xml;base64,${btoa(unescape(encodeURIComponent(this.svg)))}`;
}

this._loadSvg();
this._loadSvg(svgString);
});

return this._load;
}

/**
* Loads an SVG image from `imageUrl` or `data URL`.
*
* @param {string} svgString - the raw string of SVG for falling back to
* viewBox when width or height is not specified. This happens in Firefox.
* @private
*/
private _loadSvg(): void
private _loadSvg(svgString: string): void
{
const tempImage = new Image();

Expand All @@ -162,12 +165,21 @@ export class SVGResource extends BaseImageResource
return;
}

const svgWidth = tempImage.width;
const svgHeight = tempImage.height;
let svgWidth = tempImage.width;
let svgHeight = tempImage.height;

if (!svgWidth || !svgHeight)
{
throw new Error('The SVG image must have width and height defined (in pixels), canvas API needs them.');
const size = SVGResource.parseWidthHeightFromViewBox(svgString);

svgWidth = size.width;
svgHeight = size.height;
}

if (!svgWidth || !svgHeight)
{
throw new Error('The SVG image must have width, height'
+ 'or viewBox defined (in pixels), canvas API needs them.');
}

// Set render size
Expand Down Expand Up @@ -199,6 +211,46 @@ export class SVGResource extends BaseImageResource
};
}

/**
* This function gets the width and height from the viewBox attribute from
* provided SVG.
* @param {string} svg - the string of the SVG, not base64 encoded.
* @param {ISize} scaleTo - optional, if provide, will try to scale viewBox
* the provided size.
* @return {PIXI.ISize} scaled viewBox size if parse success, return (0, 0)
* otherwise.
*/
static parseWidthHeightFromViewBox(svg: string, scaleTo?: ISize): ISize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this private?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this name is pretty verbose, why not parseViewBoxSize?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About unit test, I had something similar to load svg inline.
However, it will not really test this particular issue since it is a Firefox issue.
When I invoke the unit test framework, it is running on chrome, by default chrome will never use this.
Any suggestion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood that this is only specific to Firefox, that's fine.

{
const size: ISize = { width: 0, height: 0 };

try
{
const parser = new DOMParser();
const svgDocument = parser.parseFromString(svg, 'image/svg+xml');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only problem with this is parsing SVG with DOMParser is that it isn't supported in IE9. We have not yet dropped support for this browser, so we need to find another approach.

https://caniuse.com/mdn-api_domparser_parsefromstring_svg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I check on MDN it says fully supported on IE9. I don't really have access to IE9 to verify which one.

Quick google shows something about it works on IE9. Do you have recommended method to verify it on IE9?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See IE10 for SVG only. But application/xml does work.

Screen Shot 2021-10-18 at 1 09 51 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of parsing, would you suggest a simple RegExp to grab the viewBox value?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would work

const element = svgDocument.querySelector('svg');

size.width = element.viewBox.baseVal.width;
size.height = element.viewBox.baseVal.height;
if (scaleTo === null || scaleTo === undefined) return size;

const factor = scaleTo.height < scaleTo.width
? scaleTo.height / size.height : scaleTo.width / size.width;

size.width *= factor;
size.height *= factor;

return size;
}
catch (err)
{
size.width = 0;
size.height = 0;

return size;
}
}

/**
* Get size from an svg string using regexp.
*
Expand Down
30 changes: 30 additions & 0 deletions packages/core/test/SVGResource.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,36 @@ describe('SVGResource', function ()
expect(Object.keys(svgSize).length)
.to.equal(0);
});

it('should get size from viewBox', function ()
{
const url = path.join(this.resources, 'viewBoxOnly.svg');
const svgString = fs.readFileSync(url, 'utf8');
const svgSize = SVGResource.parseWidthHeightFromViewBox(svgString);

expect(svgString.includes('viewBox')).to.equal(true);
expect(svgSize.width).to.equal(8);
expect(svgSize.height).to.equal(11);
});

it('should get scaled size from viewBox', function ()
{
const url = path.join(this.resources, 'viewBoxOnly.svg');
const svgString = fs.readFileSync(url, 'utf8');
let scale = { width: 16, height: 100 };
let svgSize = SVGResource.parseWidthHeightFromViewBox(svgString, scale);

expect(svgString.includes('viewBox')).to.equal(true);
expect(svgSize.width).to.equal(16);
expect(svgSize.height).to.equal(22);

scale = { width: 100, height: 33 };
svgSize = SVGResource.parseWidthHeightFromViewBox(svgString, scale);

expect(svgString.includes('viewBox')).to.equal(true);
expect(svgSize.width).to.equal(24);
expect(svgSize.height).to.equal(33);
});
});

describe('test', function ()
Expand Down
13 changes: 13 additions & 0 deletions packages/core/test/resources/viewBoxOnly.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.