fix(Canvas): flipped viewport transform coords (#7515)

fabricjs · Jun 12, 2022 · 27e89ce · 27e89ce
1 parent 405c0e0
commit 27e89ce
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 10 deletions.
diff --git a/src/static_canvas.class.js b/src/static_canvas.class.js
@@ -719,14 +719,20 @@
      * @chainable
      */
     calcViewportBoundaries: function() {
-      var points = { }, width = this.width, height = this.height,
-          iVpt = invertTransform(this.viewportTransform);
-      points.tl = transformPoint({ x: 0, y: 0 }, iVpt);
-      points.br = transformPoint({ x: width, y: height }, iVpt);
-      points.tr = new fabric.Point(points.br.x, points.tl.y);
-      points.bl = new fabric.Point(points.tl.x, points.br.y);
-      this.vptCoords = points;
-      return points;
+      var width = this.width, height = this.height,
+          iVpt = invertTransform(this.viewportTransform),
+          a = transformPoint({ x: 0, y: 0 }, iVpt),
+          b = transformPoint({ x: width, y: height }, iVpt),
+          // we don't support vpt flipping
+          // but the code is robust enough to mostly work with flipping
+          min = a.min(b),
+          max = a.max(b);
+      return this.vptCoords = {
+        tl: min,
+        tr: new fabric.Point(max.x, min.y),
+        bl: new fabric.Point(min.x, max.y),
+        br: max,
+      };
     },
 
     cancelRequestedRender: function() {

diff --git a/test/unit/canvas_static.js b/test/unit/canvas_static.js
@@ -1849,6 +1849,16 @@
     assert.deepEqual(canvas.vptCoords.br, new fabric.Point(30 + canvas.getWidth() / 2, canvas.getHeight() / 2 - 30), 'tl is 0,0');
   });
 
+  QUnit.test('calcViewportBoundaries with flipped zoom and translation', function (assert) {
+    assert.ok(typeof canvas.calcViewportBoundaries === 'function');
+    canvas.setViewportTransform([2, 0, 0, -2, -60, 60]);
+    canvas.calcViewportBoundaries();
+    assert.deepEqual({ x: canvas.vptCoords.tl.x, y: canvas.vptCoords.tl.y }, { x: 30, y: -145 }, 'tl is 30, -145');
+    assert.deepEqual({ x: canvas.vptCoords.tr.x, y: canvas.vptCoords.tr.y }, { x: 130, y: -145 }, 'tr is 130, -145');
+    assert.deepEqual({ x: canvas.vptCoords.bl.x, y: canvas.vptCoords.bl.y }, { x: 30,  y: 30 }, 'bl is 30,-70');
+    assert.deepEqual({ x: canvas.vptCoords.br.x, y: canvas.vptCoords.br.y }, { x: 130,  y: 30 }, 'br is 130,-70');
+  });
+
   QUnit.test('_isRetinaScaling', function(assert) {
     canvas.enableRetinaScaling = true;
     fabric.devicePixelRatio = 2;

diff --git a/test/unit/object_geometry.js b/test/unit/object_geometry.js
@@ -334,6 +334,24 @@
   QUnit.test('isOnScreen', function(assert) {
     var cObj = new fabric.Object({ left: 50, top: 50, width: 100, height: 100, strokeWidth: 0});
     canvas.viewportTransform = [1, 0, 0, 1, 0, 0];
+    canvas.calcViewportBoundaries();
+    cObj.canvas = canvas;
+    cObj.setCoords();
+    assert.ok(cObj.isOnScreen(), 'object is onScreen');
+    cObj.top = 1000;
+    assert.ok(cObj.isOnScreen(), 'object is still wrongly on screen since setCoords is not called and calculate is not set, even when top is already at 1000');
+    assert.ok(!cObj.isOnScreen(true), 'object is not onScreen with top 1000 with calculate true and no setCoords call');
+    cObj.setCoords();
+    assert.ok(!cObj.isOnScreen(), 'object is not onScreen with top 1000');
+    canvas.setZoom(0.1);
+    cObj.setCoords();
+    assert.ok(cObj.isOnScreen(), 'zooming out the object is again on screen');
+  });
+
+  QUnit.test('isOnScreen flipped vpt', function (assert) {
+    var cObj = new fabric.Object({ left: -50, top: -50, width: 100, height: 100, strokeWidth: 0 });
+    canvas.viewportTransform = [-1, 0, 0, -1, 0, 0];
+    canvas.calcViewportBoundaries();
     cObj.canvas = canvas;
     cObj.setCoords();
     assert.ok(cObj.isOnScreen(), 'object is onScreen');
@@ -381,6 +399,7 @@
     var cObj = new fabric.Object(
       { left: -10, top: -10, width: canvas.getWidth() + 100, height: canvas.getHeight(), strokeWidth: 0});
     canvas.viewportTransform = [1, 0, 0, 1, 0, 0];
+    canvas.calcViewportBoundaries();
     cObj.canvas = canvas;
     cObj.setCoords();
     assert.equal(cObj.isOnScreen(), true, 'object is onScreen because it include the canvas');
@@ -393,6 +412,7 @@
   QUnit.test('isOnScreen with object that is in top left corner of canvas', function(assert) {
     var cObj = new fabric.Rect({left: -46.56, top: -9.23, width: 50,height: 50, angle: 314.57});
     canvas.viewportTransform = [1, 0, 0, 1, 0, 0];
+    canvas.calcViewportBoundaries();
     cObj.canvas = canvas;
     cObj.setCoords();
     assert.ok(cObj.isOnScreen(), 'object is onScreen because it intersect a canvas line');
@@ -809,6 +829,7 @@
   QUnit.test('isPartiallyOnScreen', function(assert) {
     var cObj = new fabric.Object({ left: 50, top: 50, width: 100, height: 100, strokeWidth: 0});
     canvas.viewportTransform = [1, 0, 0, 1, 0, 0];
+    canvas.calcViewportBoundaries();
     cObj.canvas = canvas;
     cObj.left = -60;
     cObj.top = -60;
@@ -818,8 +839,8 @@
     cObj.top = -110;
     cObj.setCoords();
     assert.equal(cObj.isPartiallyOnScreen(true), false,'object is completely offScreen and not partial');
-    cObj.left = 50;
-    cObj.top = 50;
+    cObj.left = 45;
+    cObj.top = 45;
     cObj.setCoords();
     assert.equal(cObj.isPartiallyOnScreen(true), false, 'object is completely on screen and not partial');
     canvas.setZoom(2);