refactor() BREAKING: align line to other classes for positioning

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ## [6.0.0-beta5]
 
+- refactor(fabric.Line): Line position is calculated from the center between the 2 points now [#8877](https://github.com/fabricjs/fabric.js/pull/8877)
 - bundle(): export `setEnv` for JEST interoperability [#8888](https://github.com/fabricjs/fabric.js/pull/8888)
 
 ## [6.0.0-beta4]

src/shapes/Line.ts

@@ -11,6 +11,7 @@ import type {
   TProps,
 } from './Object/types';
 import type { ObjectEvents } from '../EventTypeDefs';
+import { makeBoundingBoxFromPoints } from '../util';
 
 // @TODO this code is terrible and Line should be a special case of polyline.
 
@@ -70,24 +71,28 @@ export class Line<
    * @param {Object} [options] Options object
    * @return {Line} thisArg
    */
-  constructor(points = [0, 0, 0, 0], options: Props = {} as Props) {
-    super(options);
-    this.x1 = points[0];
-    this.y1 = points[1];
-    this.x2 = points[2];
-    this.y2 = points[3];
-    this._setWidthHeight(options);
+  constructor([x1, y1, x2, y2] = [0, 0, 0, 0], options: Props = {} as Props) {
+    super({ ...options, x1, y1, x2, y2 });
+    this._setWidthHeight();
+    const { left, top } = options;
+    typeof left === 'number' && this.set('left', left);
+    typeof top === 'number' && this.set('top', top);
   }
 
   /**
    * @private
    * @param {Object} [options] Options
    */
-  _setWidthHeight({ left, top }: Partial<Props> = {}) {
-    this.width = Math.abs(this.x2 - this.x1);
-    this.height = Math.abs(this.y2 - this.y1);
-    this.left = left ?? this._getLeftToOriginX();
-    this.top = top ?? this._getTopToOriginY();
+  _setWidthHeight() {
+    const { x1, y1, x2, y2 } = this;
+    this.width = Math.abs(x2 - x1);
+    this.height = Math.abs(y2 - y1);
+    const { left, top, width, height } = makeBoundingBoxFromPoints([
+      { x: x1, y: y1 },
+      { x: x2, y: y2 },
+    ]);
+    const position = new Point(left + width / 2, top + height / 2);
+    this.setPositionByOrigin(position, 'center', 'center');
   }
 
   /**
@@ -98,6 +103,12 @@ export class Line<
   _set(key: string, value: any) {
     super._set(key, value);
     if (coordProps.includes(key as keyof UniqueLineProps)) {
+      // this doesn't make sense very much, since setting x1 when top or left
+      // are already set, is just going to show a strange result since the
+      // line will move way more than the developer expect.
+      // in fabric5 it worked only when the line didn't have extra transformations,
+      // in fabric6 too. With extra transform they behave bad in different ways.
+      // This needs probably a good rework or a tutorial if you have to create a dynamic line
       this._setWidthHeight();
     }
     return this;
@@ -174,119 +185,44 @@ export class Line<
 
   /**
    * Recalculates line points given width and height
+   * Those points are simply placed around the center,
+   * This is not useful outside internal render functions and svg output
+   * Is not meant to be for the developer.
    * @private
    */
   calcLinePoints(): UniqueLineProps {
-    const xMult = this.x1 <= this.x2 ? -1 : 1,
-      yMult = this.y1 <= this.y2 ? -1 : 1,
-      x1 = xMult * this.width * 0.5,
-      y1 = yMult * this.height * 0.5,
-      x2 = xMult * this.width * -0.5,
-      y2 = yMult * this.height * -0.5;
+    const { x1: _x1, x2: _x2, y1: _y1, y2: _y2, width, height } = this;
+    const xMult = _x1 <= _x2 ? -1 : 1,
+      yMult = _y1 <= _y2 ? -1 : 1,
+      x1 = (xMult * width) / 2,
+      y1 = (yMult * height) / 2,
+      x2 = (xMult * -width) / 2,
+      y2 = (yMult * -height) / 2;
 
     return {
-      x1: x1,
-      x2: x2,
-      y1: y1,
-      y2: y2,
+      x1,
+      x2,
+      y1,
+      y2,
     };
   }
 
-  private makeEdgeToOriginGetter(
-    propertyNames: any,
-    originValues: any
-  ): number {
-    const origin = propertyNames.origin,
-      axis1 = propertyNames.axis1,
-      axis2 = propertyNames.axis2,
-      dimension = propertyNames.dimension,
-      nearest = originValues.nearest,
-      center = originValues.center,
-      farthest = originValues.farthest;
-
-    switch (this.get(origin)) {
-      case nearest:
-        return Math.min(this.get(axis1), this.get(axis2));
-      case center:
-        return (
-          Math.min(this.get(axis1), this.get(axis2)) + 0.5 * this.get(dimension)
-        );
-      case farthest:
-        return Math.max(this.get(axis1), this.get(axis2));
-      // this should never occurr, since origin is always either one of the 3 above
-      default:
-        return 0;
-    }
-  }
-
-  /**
-   * @private
-   * @return {Number} leftToOriginX Distance from left edge of canvas to originX of Line.
-   */
-  _getLeftToOriginX(): number {
-    return this.makeEdgeToOriginGetter(
-      {
-        // property names
-        origin: 'originX',
-        axis1: 'x1',
-        axis2: 'x2',
-        dimension: 'width',
-      },
-      {
-        // possible values of origin
-        nearest: 'left',
-        center: 'center',
-        farthest: 'right',
-      }
-    );
-  }
-
-  /**
-   * @private
-   * @return {Number} leftToOriginX Distance from left edge of canvas to originX of Line.
-   */
-  _getTopToOriginY(): number {
-    return this.makeEdgeToOriginGetter(
-      {
-        // property names
-        origin: 'originY',
-        axis1: 'y1',
-        axis2: 'y2',
-        dimension: 'height',
-      },
-      {
-        // possible values of origin
-        nearest: 'top',
-        center: 'center',
-        farthest: 'bottom',
-      }
-    );
-  }
+  /* _FROM_SVG_START_ */
 
   /**
    * Returns svg representation of an instance
    * @return {Array} an array of strings with the specific svg representation
    * of the instance
    */
   _toSVG() {
-    const p = this.calcLinePoints();
+    const { x1, x2, y1, y2 } = this.calcLinePoints();
     return [
       '<line ',
       'COMMON_PARTS',
-      'x1="',
-      p.x1,
-      '" y1="',
-      p.y1,
-      '" x2="',
-      p.x2,
-      '" y2="',
-      p.y2,
-      '" />\n',
+      `x1="${x1}" y1="${y1}" x2="${x2}" y2="${y2}" />\n`,
     ];
   }
 
-  /* _FROM_SVG_START_ */
-
   /**
    * List of attribute names to account for when parsing SVG element (used by {@link Line.fromElement})
    * @static
@@ -304,13 +240,14 @@ export class Line<
    * @param {Function} [callback] callback function invoked after parsing
    */
   static fromElement(element: SVGElement, callback: (line: Line) => any) {
-    const parsedAttributes = parseAttributes(element, this.ATTRIBUTE_NAMES),
-      points = [
-        parsedAttributes.x1 || 0,
-        parsedAttributes.y1 || 0,
-        parsedAttributes.x2 || 0,
-        parsedAttributes.y2 || 0,
-      ];
+    const {
+        x1 = 0,
+        y1 = 0,
+        x2 = 0,
+        y2 = 0,
+        ...parsedAttributes
+      } = parseAttributes(element, this.ATTRIBUTE_NAMES),
+      points: [number, number, number, number] = [x1, y1, x2, y2];
     callback(new this(points, parsedAttributes));
   }