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

Defend renderer against NaN or non-number sprite size #199

Merged
merged 12 commits into from
Jul 9, 2024
Merged
6 changes: 3 additions & 3 deletions src/Renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,13 +504,13 @@ export default class Renderer {
sprite: Sprite | Stage,
options: RenderSpriteOptions
): void {
const spriteScale = "size" in sprite ? sprite.size / 100 : 1;
const drawable = this._getDrawable(sprite);

this._renderSkin(
this._getSkin(sprite.costume),
options.drawMode,
this._getDrawable(sprite).getMatrix(),
spriteScale,
drawable.getMatrix(),
drawable.getSpriteScale(),
sprite.effects,
options.effectMask,
options.colorMask,
Expand Down
37 changes: 36 additions & 1 deletion src/renderer/Drawable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,14 @@ export default class Drawable {
private _tightBoundingBox: Rectangle;
private _convexHullMatrixDiff: SpriteTransformDiff;

private _warnedBadSize: boolean;

public constructor(renderer: Renderer, sprite: Sprite | Stage) {
this._renderer = renderer;
this._sprite = sprite;

this._warnedBadSize = false;

// Transformation matrix for the sprite.
this._matrix = Matrix.create();
// Track when the sprite's transform changes so we can recalculate the
Expand Down Expand Up @@ -345,7 +349,7 @@ export default class Drawable {
}
}

const spriteScale = spr.size / 100;
const spriteScale = this.getSpriteScale();
Matrix.scale(m, m, spriteScale, spriteScale);
}

Expand Down Expand Up @@ -379,4 +383,35 @@ export default class Drawable {

return this._matrix;
}

public getSpriteScale(): number {
if (this._sprite instanceof Stage) {
return 1;
} else {
const { size } = this._sprite;
if (typeof size !== "number") {
this._warnBadSize(typeof size, "0%");
towerofnix marked this conversation as resolved.
Show resolved Hide resolved
return 0;
} else if (isNaN(size)) {
this._warnBadSize("NaN", "0%");
return 0;
} else if (size === -Infinity) {
this._warnBadSize("negative Infinity", "0%");
return 0;
} else if (size < 0) {
this._warnBadSize("less than zero", "0%");
towerofnix marked this conversation as resolved.
Show resolved Hide resolved
return 0;
} else {
return size / 100;
}
}
}

private _warnBadSize(description: string, treating: string): void {
if (!this._warnedBadSize) {
const { name } = this._sprite.constructor;
console.warn(`Expected a number, sprite ${name} size is ${description}. Treating as ${treating}.`);
this._warnedBadSize = true;
}
}
}
30 changes: 21 additions & 9 deletions src/renderer/VectorSkin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,17 @@ export default class VectorSkin extends Skin {

// TODO: handle proper subpixel positioning when SVG viewbox has non-integer coordinates
// This will require rethinking costume + project loading probably
private _createMipmap(mipLevel: number): void {
private _createMipmap(mipLevel: number): WebGLTexture | null {
// Instead of uploading the image to WebGL as a texture, render the image to a canvas and upload the canvas.
const ctx = this._drawSvgToCanvas(mipLevel);
this._mipmaps.set(
mipLevel,
// Use linear (i.e. smooth) texture filtering for vectors
// If the image is 0x0, we return null. Check for that.
ctx === null ? null : this._makeTexture(ctx.canvas, this.gl.LINEAR)
);

// If the image is 0x0, we return null. Check for that.
if (ctx === null) {
return null;
}

// Use linear (i.e. smooth) texture filtering for vectors.
return this._makeTexture(ctx.canvas, this.gl.LINEAR);
}

public getTexture(scale: number): WebGLTexture | null {
Expand All @@ -106,9 +108,19 @@ export default class VectorSkin extends Skin {
// This means that one texture pixel will always be between 0.5x and 1x the size of one rendered pixel,
// but never bigger than one rendered pixel--this prevents blurriness from blowing up the texture too much.
const mipLevel = VectorSkin.mipLevelForScale(scale);
if (!this._mipmaps.has(mipLevel)) this._createMipmap(mipLevel);
if (!this._mipmaps.has(mipLevel)) {
this._mipmaps.set(mipLevel, this._createMipmap(mipLevel));
}

return this._mipmaps.get(mipLevel) ?? null;
if (this._mipmaps.has(mipLevel)) {
// TODO: Awkward `as` due to microsoft/typescript#13086
// See: https://github.com/leopard-js/leopard/pull/199#discussion_r1669416720
return this._mipmaps.get(mipLevel) as WebGLTexture | null;
} else {
const mipmap = this._createMipmap(mipLevel);
this._mipmaps.set(mipLevel, mipmap);
return mipmap;
}
}

public destroy(): void {
Expand Down