From ab8bb6020ec2d06b504be3ea14298a8444138424 Mon Sep 17 00:00:00 2001 From: melchiar <9057412+melchiar@users.noreply.github.com> Date: Tue, 19 Mar 2019 21:09:42 -0700 Subject: [PATCH 1/3] fixed strokeUniform cache size bug --- src/mixins/object_geometry.mixin.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mixins/object_geometry.mixin.js b/src/mixins/object_geometry.mixin.js index 408b8a85099..eae19ae828b 100644 --- a/src/mixins/object_geometry.mixin.js +++ b/src/mixins/object_geometry.mixin.js @@ -570,8 +570,8 @@ */ _getNonTransformedDimensions: function() { var strokeWidth = this.strokeWidth, - w = this.width + strokeWidth, - h = this.height + strokeWidth; + w = this.width + strokeWidth / (this.strokeUniform ? this.scaleX : 1), + h = this.height + strokeWidth / (this.strokeUniform ? this.scaleY : 1); return { x: w, y: h }; }, From e6b1e30695d06616c2425a80a21748de4b8dcf13 Mon Sep 17 00:00:00 2001 From: melchiar <9057412+melchiar@users.noreply.github.com> Date: Wed, 20 Mar 2019 21:08:47 -0700 Subject: [PATCH 2/3] first attempt at changing _getCacheCanvasDimensions to use _getTransformedDimensions --- src/mixins/object_geometry.mixin.js | 4 ++-- src/shapes/object.class.js | 18 +++++++----------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/mixins/object_geometry.mixin.js b/src/mixins/object_geometry.mixin.js index eae19ae828b..408b8a85099 100644 --- a/src/mixins/object_geometry.mixin.js +++ b/src/mixins/object_geometry.mixin.js @@ -570,8 +570,8 @@ */ _getNonTransformedDimensions: function() { var strokeWidth = this.strokeWidth, - w = this.width + strokeWidth / (this.strokeUniform ? this.scaleX : 1), - h = this.height + strokeWidth / (this.strokeUniform ? this.scaleY : 1); + w = this.width + strokeWidth, + h = this.height + strokeWidth; return { x: w, y: h }; }, diff --git a/src/shapes/object.class.js b/src/shapes/object.class.js index e0525ffee76..51aeb82187a 100644 --- a/src/shapes/object.class.js +++ b/src/shapes/object.class.js @@ -734,18 +734,14 @@ */ _getCacheCanvasDimensions: function() { var objectScale = this.getTotalObjectScaling(), - dim = this._getNonTransformedDimensions(), - zoomX = objectScale.scaleX, - zoomY = objectScale.scaleY, - width = dim.x * zoomX, - height = dim.y * zoomY; + dim = this._getTransformedDimensions(); return { - // for sure this ALIASING_LIMIT is slightly crating problem - // in situation in wich the cache canvas gets an upper limit - width: width + ALIASING_LIMIT, - height: height + ALIASING_LIMIT, - zoomX: zoomX, - zoomY: zoomY, + // for sure this ALIASING_LIMIT is slightly creating problem + // in situation in which the cache canvas gets an upper limit + width: dim.x / objectScale.scaleX + ALIASING_LIMIT, + height: dim.y / objectScale.scaleY + ALIASING_LIMIT, + zoomX: objectScale.scaleX, + zoomY: objectScale.scaleY, x: dim.x, y: dim.y }; From 968d46908ab4012cf9940bd552e60e2c66f7c0cf Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Mon, 8 Apr 2019 10:21:38 +0200 Subject: [PATCH 3/3] this could work --- src/mixins/object_geometry.mixin.js | 2 ++ src/shapes/object.class.js | 18 +++++++++++------- test/unit/object.js | 15 ++++++++++++++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/mixins/object_geometry.mixin.js b/src/mixins/object_geometry.mixin.js index 8a3433bf39f..90aa87ee75b 100644 --- a/src/mixins/object_geometry.mixin.js +++ b/src/mixins/object_geometry.mixin.js @@ -577,6 +577,8 @@ /* * Calculate object bounding box dimensions from its properties scale, skew. + * @param {Number} skewX, a value to override current skewX + * @param {Number} skewY, a value to override current skewY * @private * @return {Object} .x width dimension * @return {Object} .y height dimension diff --git a/src/shapes/object.class.js b/src/shapes/object.class.js index cb3a94d1a71..c8e3c53ae39 100644 --- a/src/shapes/object.class.js +++ b/src/shapes/object.class.js @@ -734,16 +734,20 @@ */ _getCacheCanvasDimensions: function() { var objectScale = this.getTotalObjectScaling(), - dim = this._getTransformedDimensions(); + // caculate dimensions without skewing + dim = this._getTransformedDimensions(0, 0), + neededX = dim.x * objectScale.scaleX / this.scaleX, + neededY = dim.y * objectScale.scaleY / this.scaleY; return { // for sure this ALIASING_LIMIT is slightly creating problem // in situation in which the cache canvas gets an upper limit - width: dim.x / objectScale.scaleX + ALIASING_LIMIT, - height: dim.y / objectScale.scaleY + ALIASING_LIMIT, + // also objectScale contains already scaleX and scaleY + width: neededX + ALIASING_LIMIT, + height: neededY + ALIASING_LIMIT, zoomX: objectScale.scaleX, zoomY: objectScale.scaleY, - x: dim.x, - y: dim.y + x: neededX, + y: neededY }; }, @@ -792,8 +796,8 @@ this._cacheContext.setTransform(1, 0, 0, 1, 0, 0); this._cacheContext.clearRect(0, 0, canvas.width, canvas.height); } - drawingWidth = dims.x * zoomX / 2; - drawingHeight = dims.y * zoomY / 2; + drawingWidth = dims.x / 2; + drawingHeight = dims.y / 2; this.cacheTranslationX = Math.round(canvas.width / 2 - drawingWidth) + drawingWidth; this.cacheTranslationY = Math.round(canvas.height / 2 - drawingHeight) + drawingHeight; this.cacheWidth = width; diff --git a/test/unit/object.js b/test/unit/object.js index c43d48fc6d4..38e75b3a877 100644 --- a/test/unit/object.js +++ b/test/unit/object.js @@ -1057,7 +1057,20 @@ object.scaleX = 2; object.scaleY = 3; dims = object._getCacheCanvasDimensions(); - assert.deepEqual(dims, { width: 26, height: 38, zoomX: 2, zoomY: 3, x: 12, y: 12 }, 'cache is as big as the scaled object'); + assert.deepEqual(dims, { width: 26, height: 38, zoomX: 2, zoomY: 3, x: 24, y: 36 }, 'cache is as big as the scaled object'); + }); + + QUnit.test('_getCacheCanvasDimensions and strokeUniform', function(assert) { + var object = new fabric.Object({ width: 10, height: 10, strokeWidth: 2 }); + var dims = object._getCacheCanvasDimensions(); + assert.deepEqual(dims, { width: 14, height: 14, zoomX: 1, zoomY: 1, x: 12, y: 12 }, 'if no scaling is applied cache is as big as object + strokeWidth'); + object.strokeUniform = true; + var dims = object._getCacheCanvasDimensions(); + assert.deepEqual(dims, { width: 14, height: 14, zoomX: 1, zoomY: 1, x: 12, y: 12 }, 'if no scaling is applied strokeUniform makes no difference'); + object.scaleX = 2; + object.scaleY = 3; + dims = object._getCacheCanvasDimensions(); + assert.deepEqual(dims, { width: 24, height: 34, zoomX: 2, zoomY: 3, x: 22, y: 32 }, 'cache is as big as the scaled object'); }); QUnit.test('_updateCacheCanvas check if cache canvas should be updated', function(assert) {