Prevent sprite drawing context from being corrupted (#951)

excaliburjs · Apr 6, 2018 · e173e8c · e173e8c
1 parent c06518c
commit e173e8c
Show file tree

Hide file tree

Showing 11 changed files with 144 additions and 71 deletions.
diff --git 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))
 
 <!--------------------------------- DO NOT EDIT BELOW THIS LINE --------------------------------->
 

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 = <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<SpriteImpl> {
    /** @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
@@ -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
@@ -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
@@ -63,8 +63,8 @@ export class Texture extends Resource<HTMLImageElement> {
          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
@@ -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();