Fix rounding issues for getClientRect() for some shapes. fix #879

konvajs · Mar 26, 2020 · 3d00d1a · 3d00d1a
1 parent 65e06f2
commit 3d00d1a
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ## Not released:
 
 * Fix some issues with `mouseenter` and `mouseleave` events.
+* Deprecate `hitStrokeEnabled` property
+* Fix rounding issues for `getClientRect()` for some shapes
 
 ## 4.2.0 - 2020-03-14
 

diff --git a/konva.js b/konva.js
@@ -8,7 +8,7 @@
    * Konva JavaScript Framework v4.2.0
    * http://konvajs.org/
    * Licensed under the MIT
-   * Date: Wed Mar 18 2020
+   * Date: Thu Mar 26 2020
    *
    * Original work Copyright (C) 2011 - 2013 by Eric Rowell (KineticJS)
    * Modified work Copyright (C) 2014 - present by Anton Lavrenov (Konva)
@@ -7146,11 +7146,11 @@
       };
       Shape.prototype._hasShadow = function () {
           return (this.shadowEnabled() &&
-              (this.shadowOpacity() !== 0 &&
-                  !!(this.shadowColor() ||
-                      this.shadowBlur() ||
-                      this.shadowOffsetX() ||
-                      this.shadowOffsetY())));
+              this.shadowOpacity() !== 0 &&
+              !!(this.shadowColor() ||
+                  this.shadowBlur() ||
+                  this.shadowOffsetX() ||
+                  this.shadowOffsetY()));
       };
       Shape.prototype._getFillPattern = function () {
           return this._getCache(patternImage, this.__getFillPattern);
@@ -7229,10 +7229,11 @@
        * @returns {Boolean}
        */
       Shape.prototype.hasFill = function () {
-          return this.fillEnabled() && !!(this.fill() ||
-              this.fillPatternImage() ||
-              this.fillLinearGradientColorStops() ||
-              this.fillRadialGradientColorStops());
+          return (this.fillEnabled() &&
+              !!(this.fill() ||
+                  this.fillPatternImage() ||
+                  this.fillLinearGradientColorStops() ||
+                  this.fillRadialGradientColorStops()));
       };
       /**
        * returns whether or not the shape will be stroked
@@ -7252,7 +7253,7 @@
           // we should enable hit stroke we stroke is enabled
           // and we have some value from width
           return (this.strokeEnabled() &&
-              (width || this.strokeWidth() && width === 'auto'));
+              (width || (this.strokeWidth() && width === 'auto')));
       };
       /**
        * determines if point is in the shape, regardless if other shapes are on top of it.  Note: because
@@ -7291,6 +7292,7 @@
               this.getStage());
       };
       Shape.prototype.setStrokeHitEnabled = function (val) {
+          Util.warn('strokeHitEnabled property is deprecated. Please use hitStrokeWidth instead.');
           if (val) {
               this.hitStrokeWidth('auto');
           }
@@ -7321,8 +7323,8 @@
       Shape.prototype.getSelfRect = function () {
           var size = this.size();
           return {
-              x: this._centroid ? Math.round(-size.width / 2) : 0,
-              y: this._centroid ? Math.round(-size.height / 2) : 0,
+              x: this._centroid ? -size.width / 2 : 0,
+              y: this._centroid ? -size.height / 2 : 0,
               width: size.width,
               height: size.height
           };
@@ -7619,7 +7621,7 @@
   // TODO: probably we should deprecate it
   Factory.addGetterSetter(Shape, 'strokeHitEnabled', true, getBooleanValidator());
   /**
-   * get/set strokeHitEnabled property. Useful for performance optimization.
+   * **deprecated, use hitStrokeWidth instead!** get/set strokeHitEnabled property. Useful for performance optimization.
    * You may set `shape.strokeHitEnabled(false)`. In this case stroke will be no draw on hit canvas, so hit area
    * of shape will be decreased (by lineWidth / 2). Remember that non closed line with `strokeHitEnabled = false`
    * will be not drawn on hit canvas, that is mean line will no trigger pointer events (like mouseover)
@@ -10259,10 +10261,10 @@
               maxY = Math.max(maxY, y);
           }
           return {
-              x: Math.round(minX),
-              y: Math.round(minY),
-              width: Math.round(maxX - minX),
-              height: Math.round(maxY - minY)
+              x: minX,
+              y: minY,
+              width: maxX - minX,
+              height: maxY - minY
           };
       };
       return Line;
@@ -13528,7 +13530,11 @@
           context.fillStrokeShape(this);
       };
       Text.prototype.setText = function (text) {
-          var str = Util._isString(text) ? text : (text === null || text === undefined) ? '' : text + '';
+          var str = Util._isString(text)
+              ? text
+              : text === null || text === undefined
+                  ? ''
+                  : text + '';
           this._setAttr(TEXT, str);
           return this;
       };
@@ -14428,10 +14434,10 @@
           }
           var fontSize = this.fontSize();
           return {
-              x: Math.round(minX) - fontSize / 2,
-              y: Math.round(minY) - fontSize / 2,
-              width: Math.round(maxX - minX) + fontSize,
-              height: Math.round(maxY - minY) + fontSize
+              x: minX - fontSize / 2,
+              y: minY - fontSize / 2,
+              width: maxX - minX + fontSize,
+              height: maxY - minY + fontSize
           };
       };
       return TextPath;

diff --git a/konva.min.js b/konva.min.js
diff --git a/src/Shape.ts b/src/Shape.ts
@@ -246,13 +246,13 @@ export class Shape<Config extends ShapeConfig = ShapeConfig> extends Node<
   _hasShadow() {
     return (
       this.shadowEnabled() &&
-      (this.shadowOpacity() !== 0 &&
-        !!(
-          this.shadowColor() ||
-          this.shadowBlur() ||
-          this.shadowOffsetX() ||
-          this.shadowOffsetY()
-        ))
+      this.shadowOpacity() !== 0 &&
+      !!(
+        this.shadowColor() ||
+        this.shadowBlur() ||
+        this.shadowOffsetX() ||
+        this.shadowOffsetY()
+      )
     );
   }
   _getFillPattern() {
@@ -349,11 +349,14 @@ export class Shape<Config extends ShapeConfig = ShapeConfig> extends Node<
    * @returns {Boolean}
    */
   hasFill() {
-    return this.fillEnabled() && !!(
-      this.fill() ||
-      this.fillPatternImage() ||
-      this.fillLinearGradientColorStops() ||
-      this.fillRadialGradientColorStops()
+    return (
+      this.fillEnabled() &&
+      !!(
+        this.fill() ||
+        this.fillPatternImage() ||
+        this.fillLinearGradientColorStops() ||
+        this.fillRadialGradientColorStops()
+      )
     );
   }
   /**
@@ -377,7 +380,7 @@ export class Shape<Config extends ShapeConfig = ShapeConfig> extends Node<
     // and we have some value from width
     return (
       this.strokeEnabled() &&
-      (width || this.strokeWidth() && width === 'auto')
+      (width || (this.strokeWidth() && width === 'auto'))
     );
   }
   /**
@@ -428,6 +431,9 @@ export class Shape<Config extends ShapeConfig = ShapeConfig> extends Node<
     );
   }
   setStrokeHitEnabled(val: number) {
+    Util.warn(
+      'strokeHitEnabled property is deprecated. Please use hitStrokeWidth instead.'
+    );
     if (val) {
       this.hitStrokeWidth('auto');
     } else {
@@ -456,8 +462,8 @@ export class Shape<Config extends ShapeConfig = ShapeConfig> extends Node<
   getSelfRect() {
     var size = this.size();
     return {
-      x: this._centroid ? Math.round(-size.width / 2) : 0,
-      y: this._centroid ? Math.round(-size.height / 2) : 0,
+      x: this._centroid ? -size.width / 2 : 0,
+      y: this._centroid ? -size.height / 2 : 0,
       width: size.width,
       height: size.height
     };
@@ -888,7 +894,7 @@ Factory.addGetterSetter(
 Factory.addGetterSetter(Shape, 'strokeHitEnabled', true, getBooleanValidator());
 
 /**
- * get/set strokeHitEnabled property. Useful for performance optimization.
+ * **deprecated, use hitStrokeWidth instead!** get/set strokeHitEnabled property. Useful for performance optimization.
  * You may set `shape.strokeHitEnabled(false)`. In this case stroke will be no draw on hit canvas, so hit area
  * of shape will be decreased (by lineWidth / 2). Remember that non closed line with `strokeHitEnabled = false`
  * will be not drawn on hit canvas, that is mean line will no trigger pointer events (like mouseover)

diff --git a/src/shapes/Line.ts b/src/shapes/Line.ts
@@ -219,10 +219,10 @@ export class Line<Config extends LineConfig = LineConfig> extends Shape<
       maxY = Math.max(maxY, y);
     }
     return {
-      x: Math.round(minX),
-      y: Math.round(minY),
-      width: Math.round(maxX - minX),
-      height: Math.round(maxY - minY)
+      x: minX,
+      y: minY,
+      width: maxX - minX,
+      height: maxY - minY
     };
   }
 

diff --git a/src/shapes/Text.ts b/src/shapes/Text.ts
@@ -95,9 +95,11 @@ function checkDefaultFill(config) {
 }
 
 // polyfill for IE11
-const trimRight = String.prototype.trimRight || function polyfill() {
-  return this.replace(/[\s\xa0]+$/, '');
-}
+const trimRight =
+  String.prototype.trimRight ||
+  function polyfill() {
+    return this.replace(/[\s\xa0]+$/, '');
+  };
 
 /**
  * Text constructor
@@ -294,7 +296,11 @@ export class Text extends Shape<TextConfig> {
     context.fillStrokeShape(this);
   }
   setText(text) {
-    var str = Util._isString(text) ? text : (text === null || text === undefined) ? '' : text + '';
+    var str = Util._isString(text)
+      ? text
+      : text === null || text === undefined
+      ? ''
+      : text + '';
     this._setAttr(TEXT, str);
     return this;
   }

diff --git a/src/shapes/TextPath.ts b/src/shapes/TextPath.ts
@@ -531,10 +531,10 @@ export class TextPath extends Shape<TextPathConfig> {
     }
     var fontSize = this.fontSize();
     return {
-      x: Math.round(minX) - fontSize / 2,
-      y: Math.round(minY) - fontSize / 2,
-      width: Math.round(maxX - minX) + fontSize,
-      height: Math.round(maxY - minY) + fontSize
+      x: minX - fontSize / 2,
+      y: minY - fontSize / 2,
+      width: maxX - minX + fontSize,
+      height: maxY - minY + fontSize
     };
   }
 

diff --git a/test/unit/shapes/Line-test.js b/test/unit/shapes/Line-test.js
@@ -309,10 +309,10 @@ suite('Line', function() {
 
     stage.add(layer);
 
-    assert.equal(client.x, 56, 'check x');
-    assert.equal(client.y, 74, 'check y');
-    assert.equal(client.width, 245, 'check width');
-    assert.equal(client.height, 147, 'check height');
+    assert.equal(Math.round(client.x), 56, 'check x');
+    assert.equal(Math.round(client.y), 74, 'check y');
+    assert.equal(Math.round(client.width), 245, 'check width');
+    assert.equal(Math.round(client.height), 147, 'check height');
   });
 
   test('getClientRect with low number of points', function() {
@@ -401,4 +401,30 @@ suite('Line', function() {
       'calculated points should change'
     );
   });
+
+  test('getClientRect with scaling', function() {
+    var stage = addStage();
+    var scale = 42;
+    stage.scaleX(scale);
+    stage.scaleY(scale);
+    var layer = new Konva.Layer();
+    stage.add(layer);
+
+    var points = [1, 1, 7, 2, 8, 7, 2, 6];
+    var line = new Konva.Line({
+      points: points.map(function(v) {
+        return (v * 20) / scale;
+      }),
+      closed: true,
+      fill: 'green'
+    });
+    layer.add(line);
+
+    var client = line.getClientRect();
+
+    assert.equal(client.x, 20, 'check x');
+    assert.equal(client.y, 20, 'check y');
+    assert.equal(client.width, 140, 'check width');
+    assert.equal(client.height, 120, 'check height');
+  });
 });
diff --git a/test/unit/shapes/TextPath-test.js b/test/unit/shapes/TextPath-test.js
@@ -260,7 +260,7 @@ suite('TextPath', function() {
     layer.add(textpath);
     stage.add(layer);
 
-    cloneAndCompareLayer(layer, 50);
+    cloneAndCompareLayer(layer, 200);
     showHit(layer);
   });
 
@@ -286,7 +286,7 @@ suite('TextPath', function() {
 
     layer.add(textpath);
     stage.add(layer);
-    cloneAndCompareLayer(layer, 50);
+    cloneAndCompareLayer(layer, 200);
     showHit(layer);
   });