From e173e8c25ea93df5b5d73ff3c098d439f5d18e48 Mon Sep 17 00:00:00 2001 From: Erik Onarheim Date: Thu, 5 Apr 2018 20:18:04 -0500 Subject: [PATCH] Prevent sprite drawing context from being corrupted (#951) --- CHANGELOG.md | 1 + sandbox/src/game.ts | 8 +++- src/engine/Actor.ts | 4 +- src/engine/Drawing/Animation.ts | 18 ++++---- src/engine/Drawing/Polygon.ts | 18 ++++---- src/engine/Drawing/Sprite.ts | 69 +++++++++++++++--------------- src/engine/Drawing/SpriteSheet.ts | 14 +++--- src/engine/Interfaces/IDrawable.ts | 8 ++-- src/engine/Resources/Texture.ts | 4 +- src/engine/Util/CullingBox.ts | 4 +- src/spec/ActorSpec.ts | 67 +++++++++++++++++++++++++++++ 11 files changed, 144 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d3a02fb1..568a5dfc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Offscreen culling in HiDPI mode ([#949](https://github.com/excaliburjs/Excalibur/issues/949)) - Correct bounds check to check drawWidth/drawHeight for HiDPI - suppressHiDPIScaling now also suppresses pixel ratio based scaling +- Extract and separate Sprite width/height from drawWidth/drawHeight ([#951](https://github.com/excaliburjs/Excalibur/pull/951)) diff --git a/sandbox/src/game.ts b/sandbox/src/game.ts index 47d68f179..93c14798a 100644 --- a/sandbox/src/game.ts +++ b/sandbox/src/game.ts @@ -76,7 +76,13 @@ game.add(heart); // Turn on debug diagnostics game.isDebug = false; //var blockSprite = new ex.Sprite(imageBlocks, 0, 0, 65, 49); -var blockSprite = new ex.Sprite({image: imageBlocks, x:0, y: 0, width: 65, height: 49}); +var blockSprite = new ex.Sprite({ + image: imageBlocks, + x: 0, + y: 0, + width: 65, + height: 49 +}); // Create spritesheet //var spriteSheetRun = new ex.SpriteSheet(imageRun, 21, 1, 96, 96); var spriteSheetRun = new ex.SpriteSheet({ image: imageRun, diff --git a/src/engine/Actor.ts b/src/engine/Actor.ts index dfa197fed..036e78e9c 100644 --- a/src/engine/Actor.ts +++ b/src/engine/Actor.ts @@ -1237,8 +1237,8 @@ export class ActorImpl extends Class implements IActionable, IEvented, IPointerE if (this.currentDrawing) { var drawing = this.currentDrawing; // See https://github.com/excaliburjs/Excalibur/pull/619 for discussion on this formula - var offsetX = (this._width - drawing.naturalWidth * drawing.scale.x) * this.anchor.x; - var offsetY = (this._height - drawing.naturalHeight * drawing.scale.y) * this.anchor.y; + var offsetX = (this._width - drawing.width * drawing.scale.x) * this.anchor.x; + var offsetY = (this._height - drawing.height * drawing.scale.y) * this.anchor.y; if (this._effectsDirty) { this._reapplyEffects(this.currentDrawing); diff --git a/src/engine/Drawing/Animation.ts b/src/engine/Drawing/Animation.ts index 575cda2cb..88c777773 100644 --- a/src/engine/Drawing/Animation.ts +++ b/src/engine/Drawing/Animation.ts @@ -59,10 +59,10 @@ export class AnimationImpl implements IDrawable { */ public flipHorizontal: boolean = false; + public drawWidth: number = 0; + public drawHeight: number= 0; public width: number = 0; - public height: number= 0; - public naturalWidth: number = 0; - public naturalHeight: number = 0; + public height: number = 0; /** * Typically you will use a [[SpriteSheet]] to generate an [[Animation]]. @@ -92,11 +92,11 @@ export class AnimationImpl implements IDrawable { } if (sprites && sprites[0]) { - this.height = sprites[0] ? sprites[0].height : 0; - this.width = sprites[0] ? sprites[0].width : 0; + this.drawHeight = sprites[0] ? sprites[0].drawHeight : 0; + this.drawWidth = sprites[0] ? sprites[0].drawWidth : 0; - this.naturalWidth = sprites[0] ? sprites[0].naturalWidth : 0; - this.naturalHeight = sprites[0] ? sprites[0].naturalHeight : 0; + this.width = sprites[0] ? sprites[0].width : 0; + this.height = sprites[0] ? sprites[0].height : 0; this.freezeFrame = sprites.length - 1; } @@ -289,8 +289,8 @@ export class AnimationImpl implements IDrawable { // add the calculated width if (currSprite) { - this.width = currSprite.width; - this.height = currSprite.height; + this.drawWidth = currSprite.drawWidth; + this.drawHeight = currSprite.drawHeight; } } diff --git a/src/engine/Drawing/Polygon.ts b/src/engine/Drawing/Polygon.ts index 521ab0c12..3b9bfa63b 100644 --- a/src/engine/Drawing/Polygon.ts +++ b/src/engine/Drawing/Polygon.ts @@ -12,11 +12,11 @@ import { Vector } from '../Algebra'; export class Polygon implements IDrawable { public flipVertical: boolean; public flipHorizontal: boolean; + public drawWidth: number; + public drawHeight: number; + public width: number; public height: number; - - public naturalWidth: number; - public naturalHeight: number; /** * The color to use for the lines of the polygon @@ -53,7 +53,7 @@ export class Polygon implements IDrawable { return Math.max(prev, curr.x); }, 0); - this.width = maxX - minX; + this.drawWidth = maxX - minX; var minY = this._points.reduce((prev: number, curr: Vector) => { return Math.min(prev, curr.y); @@ -62,10 +62,10 @@ export class Polygon implements IDrawable { return Math.max(prev, curr.y); }, 0); - this.height = maxY - minY; + this.drawHeight = maxY - minY; - this.naturalHeight = this.height; - this.naturalWidth = this.width; + this.height = this.drawHeight; + this.width = this.drawWidth; } /** @@ -129,12 +129,12 @@ export class Polygon implements IDrawable { ctx.strokeStyle = this.lineColor.toString(); if (this.flipHorizontal) { - ctx.translate(this.width, 0); + ctx.translate(this.drawWidth, 0); ctx.scale(-1, 1); } if (this.flipVertical) { - ctx.translate(0, this.height); + ctx.translate(0, this.drawHeight); ctx.scale(1, -1); } diff --git a/src/engine/Drawing/Sprite.ts b/src/engine/Drawing/Sprite.ts index f339f990e..02ef17797 100644 --- a/src/engine/Drawing/Sprite.ts +++ b/src/engine/Drawing/Sprite.ts @@ -18,8 +18,16 @@ export class SpriteImpl implements IDrawable { public x: number = 0; public y: number = 0; - public width: number = 0; - public height: number = 0; + + + public get drawWidth(): number { + return this.width * this.scale.x; + } + + public get drawHeight(): number { + return this.height * this.scale.y; + } + /** @obsolete ex.[[Sprite.sx]] will be deprecated in 0.17.0 use ex.[[Sprite.x]] */ public get sx() { @@ -83,8 +91,8 @@ export class SpriteImpl implements IDrawable { public effects: Effects.ISpriteEffect[] = []; - public naturalWidth: number = 0; - public naturalHeight: number = 0; + public width: number = 0; + public height: number = 0; private _spriteCanvas: HTMLCanvasElement = null; private _spriteCtx: CanvasRenderingContext2D = null; @@ -104,8 +112,8 @@ export class SpriteImpl implements IDrawable { if (imageOrConfig && !(imageOrConfig instanceof Texture)) { x = imageOrConfig.x || imageOrConfig.sx; y = imageOrConfig.y || imageOrConfig.sy; - width = imageOrConfig.width || imageOrConfig.swidth; - height = imageOrConfig.height || imageOrConfig.sheight; + width = imageOrConfig.drawWidth || imageOrConfig.swidth; + height = imageOrConfig.drawHeight || imageOrConfig.sheight; image = imageOrConfig.image; if (!image) { const message = 'An image texture is required to contsruct a sprite'; @@ -115,8 +123,6 @@ export class SpriteImpl implements IDrawable { this.x = x || 0; this.y = y || 0; - this.width = width || 0; - this.height = height || 0; this._texture = image; this._spriteCanvas = document.createElement('canvas'); @@ -132,8 +138,8 @@ export class SpriteImpl implements IDrawable { this.logger.error('Error loading texture ', this._texture.path, e); }); - this.naturalWidth = width; - this.naturalHeight = height; + this.width = width; + this.height = height; } private _loadPixels() { @@ -142,12 +148,12 @@ export class SpriteImpl implements IDrawable { var naturalHeight = this._texture.image.naturalHeight || 0; if (this.width > naturalWidth) { - this.logger.warn('The sprite width', this.width, 'exceeds the width', - naturalWidth, 'of the backing texture', this._texture.path); + this.logger.warn(`The sprite width ${this.width} exceeds the width + ${naturalWidth} of the backing texture ${this._texture.path}`); } if (this.height > naturalHeight) { - this.logger.warn('The sprite height', this.height, 'exceeds the height', - naturalHeight, 'of the backing texture', this._texture.path); + this.logger.warn(`The sprite height ${this.height} exceeds the height + ${naturalHeight} of the backing texture ${this._texture.path}`); } this._spriteCtx.drawImage(this._texture.image, clamp(this.x, 0, naturalWidth), @@ -279,7 +285,8 @@ export class SpriteImpl implements IDrawable { var naturalHeight = this._texture.image.naturalHeight || 0; this._spriteCtx.clearRect(0, 0, this.width, this.height); - this._spriteCtx.drawImage(this._texture.image, clamp(this.x, 0, naturalWidth), + this._spriteCtx.drawImage(this._texture.image, + clamp(this.x, 0, naturalWidth), clamp(this.y, 0, naturalHeight), clamp(this.width, 0, naturalWidth), clamp(this.height, 0, naturalHeight), @@ -321,13 +328,11 @@ export class SpriteImpl implements IDrawable { ctx.save(); ctx.translate(x, y); ctx.rotate(this.rotation); - const scaledSWidth = this.width * this.scale.x; - const scaledSHeight = this.height * this.scale.y; - var xpoint = (scaledSWidth) * this.anchor.x; - var ypoint = (scaledSHeight) * this.anchor.y; + var xpoint = this.drawWidth * this.anchor.x; + var ypoint = this.drawHeight * this.anchor.y; ctx.strokeStyle = Color.Black.toString(); - ctx.strokeRect(-xpoint, -ypoint, scaledSWidth, scaledSHeight); + ctx.strokeRect(-xpoint, -ypoint, this.drawWidth, this.drawHeight); ctx.restore(); } @@ -342,35 +347,29 @@ export class SpriteImpl implements IDrawable { this._applyEffects(); } - // calculating current dimensions - this.width = this.naturalWidth * this.scale.x; - this.height = this.naturalHeight * this.scale.y; - + // calculating current dimensions ctx.save(); - var xpoint = this.width * this.anchor.x; - var ypoint = this.height * this.anchor.y; + var xpoint = this.drawWidth * this.anchor.x; + var ypoint = this.drawHeight * this.anchor.y; ctx.translate(x, y); ctx.rotate(this.rotation); - - var scaledSWidth = this.width * this.scale.x; - var scaledSHeight = this.height * this.scale.y; // todo cache flipped sprites if (this.flipHorizontal) { - ctx.translate(scaledSWidth, 0); + ctx.translate(this.drawWidth, 0); ctx.scale(-1, 1); } if (this.flipVertical) { - ctx.translate(0, scaledSHeight); + ctx.translate(0, this.drawHeight); ctx.scale(1, -1); } ctx.drawImage(this._spriteCanvas, 0, 0, this.width, this.height, -xpoint, -ypoint, - scaledSWidth, - scaledSHeight); + this.drawWidth, + this.drawHeight); ctx.restore(); } @@ -406,10 +405,10 @@ export interface ISpriteArgs extends Partial { /** @obsolete ex.[[Sprite.sy]] will be deprecated in 0.17.0 use ex.[[Sprite.y]] */ sy?: number; width?: number; - /** @obsolete ex.[[Sprite.swidth]] will be deprecated in 0.17.0 use ex.[[Sprite.swidth]] */ + /** @obsolete ex.[[Sprite.swidth]] will be deprecated in 0.17.0 use ex.[[Sprite.width]] */ swidth?: number; height?: number; - /** @obsolete ex.[[Sprite.sheight]] will be deprecated in 0.17.0 use ex.[[Sprite.sheight]] */ + /** @obsolete ex.[[Sprite.sheight]] will be deprecated in 0.17.0 use ex.[[Sprite.height]] */ sheight?: number; rotation?: number; anchor?: Vector; diff --git a/src/engine/Drawing/SpriteSheet.ts b/src/engine/Drawing/SpriteSheet.ts index b3a6275a4..2b909f791 100644 --- a/src/engine/Drawing/SpriteSheet.ts +++ b/src/engine/Drawing/SpriteSheet.ts @@ -151,15 +151,15 @@ export class SpriteSheetImpl { let coord = spriteCoordinates[i]; // no need to pass image again if using a spritesheet coord.image = coord.image || this.image; - maxWidth = Math.max(maxWidth, coord.width); - maxHeight = Math.max(maxHeight, coord.height); + maxWidth = Math.max(maxWidth, coord.drawWidth); + maxHeight = Math.max(maxHeight, coord.drawHeight); sprites[i] = new Sprite(coord); } let anim = new Animation(engine, sprites, speed); - anim.width = maxWidth; - anim.height = maxHeight; + anim.drawWidth = maxWidth; + anim.drawHeight = maxHeight; return anim; } } @@ -308,12 +308,12 @@ export class SpriteFontImpl extends SpriteSheet { var sprite = this.sprites[0]; // find the current height fo the text in pixels - var height = sprite.height; + var height = sprite.drawHeight; // calculate appropriate scale for font size var scale = options.fontSize / height; - var length = (text.length * sprite.width * scale) + (text.length * options.letterSpacing); + var length = (text.length * sprite.drawWidth * scale) + (text.length * options.letterSpacing); var currX = x; if (options.textAlign === TextAlign.Left || options.textAlign === TextAlign.Start) { @@ -354,7 +354,7 @@ export class SpriteFontImpl extends SpriteSheet { charSprite.scale.x = scale; charSprite.scale.y = scale; charSprite.draw(ctx, currX, currY); - currX += (charSprite.width + options.letterSpacing); + currX += (charSprite.drawWidth + options.letterSpacing); } catch (e) { Logger.getInstance().error(`SpriteFont Error drawing char ${character}`); } diff --git a/src/engine/Interfaces/IDrawable.ts b/src/engine/Interfaces/IDrawable.ts index e2fed7e90..9b92c7177 100644 --- a/src/engine/Interfaces/IDrawable.ts +++ b/src/engine/Interfaces/IDrawable.ts @@ -16,20 +16,20 @@ export interface IDrawable { /** * Indicates the current width of the drawing in pixels, factoring in the scale */ - width: number; + drawWidth: number; /** * Indicates the current height of the drawing in pixels, factoring in the scale */ - height: number; + drawHeight: number; /** * Indicates the natural width of the drawing in pixels, this is the original width of the source image */ - naturalWidth: number; + width: number; /** * Indicates the natural height of the drawing in pixels, this is the original height of the source image */ - naturalHeight: number; + height: number; /** * Adds a new [[ISpriteEffect]] to this drawing. diff --git a/src/engine/Resources/Texture.ts b/src/engine/Resources/Texture.ts index 6bcfb91e6..86ce0c11e 100644 --- a/src/engine/Resources/Texture.ts +++ b/src/engine/Resources/Texture.ts @@ -63,8 +63,8 @@ export class Texture extends Resource { this.image = new Image(); this.image.addEventListener('load', () => { this._isLoaded = true; - this.width = this._sprite.width = this._sprite.naturalWidth = this._sprite.width = this.image.naturalWidth; - this.height = this._sprite.height = this._sprite.naturalHeight = this._sprite.height = this.image.naturalHeight; + this.width = this._sprite.width = this.image.naturalWidth; + this.height = this._sprite.height = this.image.naturalHeight; this.loaded.resolve(this.image); complete.resolve(this.image); }); diff --git a/src/engine/Util/CullingBox.ts b/src/engine/Util/CullingBox.ts index 83f527623..c15ffeaad 100644 --- a/src/engine/Util/CullingBox.ts +++ b/src/engine/Util/CullingBox.ts @@ -22,8 +22,8 @@ export class CullingBox { private _yMaxWorld: number; public isSpriteOffScreen(actor: Actor, engine: Engine): boolean { - var drawingWidth = actor.currentDrawing.width; - var drawingHeight = actor.currentDrawing.height; + var drawingWidth = actor.currentDrawing.drawWidth; + var drawingHeight = actor.currentDrawing.drawHeight; var rotation = actor.rotation; var anchor = actor.getCenter(); var worldPos = actor.getWorldPos(); diff --git a/src/spec/ActorSpec.ts b/src/spec/ActorSpec.ts index 1ef2e3463..116496a1d 100644 --- a/src/spec/ActorSpec.ts +++ b/src/spec/ActorSpec.ts @@ -1,3 +1,4 @@ +/// /// /// @@ -10,6 +11,7 @@ describe('A game actor', () => { var mock = new Mocks.Mocker(); beforeEach(() => { + jasmine.addMatchers(imagediff.jasmine); engine = TestUtils.engine({width: 100, height: 100}); actor = new ex.Actor(); actor.collisionType = ex.CollisionType.Active; @@ -1256,6 +1258,71 @@ describe('A game actor', () => { expect(propSpy).toHaveBeenCalledTimes(1); }); + it('should not corrupt shared sprite ctxs', (done) => { + engine = TestUtils.engine({ + width: 62, + height: 64, + suppressHiDPIScaling: true + }); + + let texture = new ex.Texture('base/src/spec/images/SpriteSpec/icon.png', true); + texture.load().then(() => { + let sprite = new ex.Sprite({ + image: texture, + x: 0, + y: 0, + width: 62, + height: 64, + scale: new ex.Vector(2, 2) + // rotation: Math.PI / 4, + // anchor: ex.Vector.Half.clone() + }); + + var actor = new ex.Actor({ + x: engine.halfCanvasWidth, + y: engine.halfCanvasHeight, + width: 10, + height: 10, + rotation: Math.PI / 4 + }); + + engine.add(actor); + let s = texture.asSprite(); + s.scale.setTo(1, 1); + actor.addDrawing(s); + + let a1 = new ex.Actor({scale: new ex.Vector(3, 3)}); + a1.scale.setTo(3, 3); + a1.addDrawing(texture); + + let a2 = new ex.Actor({scale: new ex.Vector(3, 3)}); + a1.scale.setTo(3, 3); + a2.addDrawing(texture); + + + a1.draw(engine.ctx, 100); + a2.draw(engine.ctx, 100); + actor.draw(engine.ctx, 100); + engine.ctx.fillRect(0, 0, 200, 200); + + a1.draw(engine.ctx, 100); + a2.draw(engine.ctx, 100); + actor.draw(engine.ctx, 100); + engine.ctx.fillRect(0, 0, 200, 200); + + + a1.draw(engine.ctx, 100); + a2.draw(engine.ctx, 100); + + engine.ctx.clearRect(0, 0, 200, 200); + actor.draw(engine.ctx, 100); + + imagediff.expectCanvasImageMatches('SpriteSpec/iconrotate.png', engine.canvas, done); + + }); + }); + + describe('lifecycle overrides', () => { let actor: ex.Actor = null;