diff --git a/CHANGELOG.md b/CHANGELOG.md index 64609a04276..b3aabe0ffd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- BREAKING: remove calculate true/false from the api. [#9483](https://github.com/fabricjs/fabric.js/pull/9483) - chore(): remove some Type assertions [#8950](https://github.com/fabricjs/fabric.js/pull/8950) - chore(): expose `sendVectorToPlane` [#9479](https://github.com/fabricjs/fabric.js/pull/9479) - BREAKING: remove absolute true/false from the api. [#9395](https://github.com/fabricjs/fabric.js/pull/9395) diff --git a/src/benchmarks/raycasting.mjs b/src/benchmarks/raycasting.mjs index 6b535201d1d..d96914143f1 100644 --- a/src/benchmarks/raycasting.mjs +++ b/src/benchmarks/raycasting.mjs @@ -67,7 +67,7 @@ const findCrossPoints = (point, lines) => { /** * Method that returns an object with the object edges in it, given the coordinates of the corners * @private - * @param {Object} lineCoords or aCoords Coordinates of the object corners + * @param {Object} aCoords Coordinates of the object corners */ const getImageLines = ({ tl, tr, bl, br }) => { const lines = { @@ -101,8 +101,9 @@ export const cornerPointContainsPoint = (point, cornerPoint) => { // END OF OLD CODE class Test1 extends FabricObject { - containsPoint(point, absolute, calculate) { - return cornerPointContainsPoint(point, this._getCoords(calculate)); + containsPoint(point) { + const [tl, tr, br, bl] = this.getCoords(); + return cornerPointContainsPoint(point, { tl, tr, br, bl }); } } diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index a7a0a2bf2c5..94af4847b75 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -786,7 +786,7 @@ export class SelectableCanvas ]; // in case of padding we calculate the new coords on the fly. // otherwise we have to maintain 2 sets of coordinates for everything. - // we can reiterate on storing those on something similar to lineCoords + // we can reiterate on storing them. // if this is slow, for now the semplification is large and doesn't impact // rendering. // the idea behind this is that outside target check we don't need ot know diff --git a/src/canvas/StaticCanvas.ts b/src/canvas/StaticCanvas.ts index c91e3c457b5..ae9d295c109 100644 --- a/src/canvas/StaticCanvas.ts +++ b/src/canvas/StaticCanvas.ts @@ -112,13 +112,9 @@ export class StaticCanvas< declare allowTouchScrolling: boolean; declare viewportTransform: TMat2D; + /** - * Describe canvas element extension over design - * properties are tl,tr,bl,br. - * if canvas is not zoomed/panned those points are the four corner of canvas - * if canvas is viewportTransformed you those points indicate the extension - * of canvas element in plain untrasformed coordinates - * The coordinates get updated with @method calcViewportBoundaries. + * The viewport bounding box in scene plane coordinates, see {@link calcViewportBoundaries} */ declare vptCoords: TCornerPoint; @@ -504,10 +500,7 @@ export class StaticCanvas< /** * Calculate the position of the 4 corner of canvas with current viewportTransform. - * helps to determinate when an object is in the current rendering viewport using - * object absolute coordinates ( aCoords ) - * @return {Object} points.tl - * @chainable + * helps to determinate when an object is in the current rendering viewport */ calcViewportBoundaries(): TCornerPoint { const width = this.width, diff --git a/src/shapes/Object/InteractiveObject.ts b/src/shapes/Object/InteractiveObject.ts index 7d6cd4d83d2..6beda9e31d7 100644 --- a/src/shapes/Object/InteractiveObject.ts +++ b/src/shapes/Object/InteractiveObject.ts @@ -87,13 +87,10 @@ export class InteractiveFabricObject< declare moveCursor: CSSStyleDeclaration['cursor'] | null; /** - * Describe object's corner position in canvas element coordinates. - * properties are depending on control keys and padding the main controls. - * each property is an object with x, y and corner. - * The `corner` property contains in a similar manner the 4 points of the - * interactive area of the corner. - * The coordinates depends from the controls positionHandler and are used - * to draw and locate controls + * The object's controls' position in viewport coordinates + * Calculated by {@link Control#positionHandler} and {@link Control#calcCornerCoords}, depending on {@link padding}. + * `corner/touchCorner` describe the 4 points forming the interactive area of the corner. + * Used to draw and locate controls. */ declare oCoords: Record; @@ -292,16 +289,12 @@ export class InteractiveFabricObject< } /** - * Sets corner and controls position coordinates based on current angle, width and height, left and top. - * oCoords are used to find the corners - * aCoords are used to quickly find an object on the canvas - * lineCoords are used to quickly find object during pointer events. + * @override set controls' coordinates as well * See {@link https://github.com/fabricjs/fabric.js/wiki/When-to-call-setCoords} and {@link http://fabricjs.com/fabric-gotchas} * @return {void} */ setCoords(): void { super.setCoords(); - // set coordinates of the draggable boxes in the corners used to scale/rotate the image this.canvas && (this.oCoords = this.calcOCoords()); } diff --git a/src/shapes/Object/Object.ts b/src/shapes/Object/Object.ts index e71656e9742..384b85f985a 100644 --- a/src/shapes/Object/Object.ts +++ b/src/shapes/Object/Object.ts @@ -1376,9 +1376,9 @@ export class FabricObject< sendObjectToPlane(this, this.getViewportTransform()); } + this.setCoords(); const el = createCanvasElement(), - // calculate with setCoords now. - boundingRect = this.getBoundingRect(true), + boundingRect = this.getBoundingRect(), shadow = this.shadow, shadowOffset = new Point(); diff --git a/src/shapes/Object/ObjectGeometry.ts b/src/shapes/Object/ObjectGeometry.ts index fe42ae85afe..cbf754b95ec 100644 --- a/src/shapes/Object/ObjectGeometry.ts +++ b/src/shapes/Object/ObjectGeometry.ts @@ -19,11 +19,7 @@ import { transformPoint, calcPlaneRotation, } from '../../util/misc/matrix'; -import { - degreesToRadians, - radiansToDegrees, -} from '../../util/misc/radiansDegreesConversion'; -import { sin } from '../../util/misc/sin'; +import { radiansToDegrees } from '../../util/misc/radiansDegreesConversion'; import type { Canvas } from '../../canvas/Canvas'; import type { StaticCanvas } from '../../canvas/StaticCanvas'; import { ObjectOrigin } from './ObjectOrigin'; @@ -44,27 +40,15 @@ export class ObjectGeometry declare padding: number; /** - * Describe object's corner position in canvas object absolute coordinates - * properties are tl,tr,bl,br and describe the four main corner. - * each property is an object with x, y, instance of Fabric.Point. - * The coordinates depends from this properties: width, height, scaleX, scaleY - * skewX, skewY, angle, strokeWidth, top, left. - * Those coordinates are useful to understand where an object is. They get updated - * with lineCoords or oCoords in interactive cases but they do not need to be updated when zoom or panning change. - * The coordinates get updated with @method setCoords. - * You can calculate them without updating with @method calcACoords(); + * Describe object's corner position in scene coordinates. + * The coordinates are derived from the following: + * left, top, width, height, scaleX, scaleY, skewX, skewY, angle, strokeWidth. + * The coordinates do not depend on viewport changes. + * The coordinates get updated with {@link setCoords}. + * You can calculate them without updating with {@link calcACoords()} */ declare aCoords: TACoords; - /** - * Describe object's corner position in canvas element coordinates. - * includes padding. Used of object detection. - * set and refreshed with setCoords. - * Those could go away - * @todo investigate how to get rid of those - */ - declare lineCoords: TCornerPoint; - /** * storage cache for object transform matrix */ @@ -195,32 +179,11 @@ export class ObjectGeometry } /** - * return correct set of coordinates for intersection - * this will return either aCoords or lineCoords. - * @param {boolean} calculate will calculate the coords or use the one - * that are attached to the object instance - * @return {Object} {tl, tr, br, bl} points - */ - private _getCoords(calculate = false): TCornerPoint { - if (calculate) { - return this.calcACoords(); - } - // swapped this double if in place of setCoords(); - if (!this.aCoords) { - this.aCoords = this.calcACoords(); - } - return this.aCoords; - } - - /** - * return correct set of coordinates for intersection - * this will return either aCoords or lineCoords. - * The coords are returned in an array. - * @param {boolean} calculate will return aCoords if true or lineCoords - * @return {Array} [tl, tr, br, bl] of points + * @return {Point[]} [tl, tr, br, bl] in the scene plane */ - getCoords(calculate = false): Point[] { - const { tl, tr, br, bl } = this._getCoords(calculate); + getCoords(): Point[] { + const { tl, tr, br, bl } = + this.aCoords || (this.aCoords = this.calcACoords()); const coords = [tl, tr, br, bl]; if (this.group) { const t = this.group.calcTransformMatrix(); @@ -230,78 +193,56 @@ export class ObjectGeometry } /** - * Checks if object intersects with an area formed by 2 points - * @param {Object} pointTL top-left point of area - * @param {Object} pointBR bottom-right point of area - * @param {Boolean} [calculate] use coordinates of current position instead of stored one - * @return {Boolean} true if object intersects with an area formed by 2 points + * Checks if object intersects with the scene rect formed by {@link tl} and {@link br} */ - intersectsWithRect( - pointTL: Point, - pointBR: Point, - calculate?: boolean - ): boolean { - const coords = this.getCoords(calculate), - intersection = Intersection.intersectPolygonRectangle( - coords, - pointTL, - pointBR - ); + intersectsWithRect(tl: Point, br: Point): boolean { + const intersection = Intersection.intersectPolygonRectangle( + this.getCoords(), + tl, + br + ); return intersection.status === 'Intersection'; } /** * Checks if object intersects with another object * @param {Object} other Object to test - * @param {Boolean} [absolute] use coordinates without viewportTransform - * @param {Boolean} [calculate] use coordinates of current position instead of calculating them * @return {Boolean} true if object intersects with another object */ - intersectsWithObject(other: ObjectGeometry, calculate = false): boolean { + intersectsWithObject(other: ObjectGeometry): boolean { const intersection = Intersection.intersectPolygonPolygon( - this.getCoords(calculate), - other.getCoords(calculate) + this.getCoords(), + other.getCoords() ); return ( intersection.status === 'Intersection' || intersection.status === 'Coincident' || - other.isContainedWithinObject(this, calculate) || - this.isContainedWithinObject(other, calculate) + other.isContainedWithinObject(this) || + this.isContainedWithinObject(other) ); } /** * Checks if object is fully contained within area of another object * @param {Object} other Object to test - * @param {Boolean} [absolute] use coordinates without viewportTransform - * @param {Boolean} [calculate] use coordinates of current position instead of stored ones * @return {Boolean} true if object is fully contained within area of another object */ - isContainedWithinObject(other: ObjectGeometry, calculate = false): boolean { - const points = this.getCoords(calculate); - calculate && other.getCoords(true); + isContainedWithinObject(other: ObjectGeometry): boolean { + const points = this.getCoords(); return points.every((point) => other.containsPoint(point)); } /** - * Checks if object is fully contained within area formed by 2 points, aligned with scene axis. - * @param {Object} pointTL top-left point of area - * @param {Object} pointBR bottom-right point of area - * @param {Boolean} [calculate] use coordinates of current position instead of stored one - * @return {Boolean} true if object is fully contained within area formed by 2 points + * Checks if object is fully contained within the scene rect formed by {@link tl} and {@link br} */ - isContainedWithinRect( - pointTL: Point, - pointBR: Point, - calculate?: boolean - ): boolean { - const boundingRect = this.getBoundingRect(calculate); + isContainedWithinRect(tl: Point, br: Point): boolean { + const { left, top, width, height } = this.getBoundingRect(); return ( - boundingRect.left >= pointTL.x && - boundingRect.left + boundingRect.width <= pointBR.x && - boundingRect.top >= pointTL.y && - boundingRect.top + boundingRect.height <= pointBR.y + left >= tl.x && + left + width <= br.x && + top >= tl.y && + top + height <= br.y ); } @@ -316,25 +257,23 @@ export class ObjectGeometry /** * Checks if point is inside the object * @param {Point} point Point to check against - * @param {Boolean} [calculate] use coordinates of current position instead of stored ones * @return {Boolean} true if point is inside the object */ - containsPoint(point: Point, calculate = false): boolean { - return Intersection.isPointInPolygon(point, this.getCoords(calculate)); + containsPoint(point: Point): boolean { + return Intersection.isPointInPolygon(point, this.getCoords()); } /** * Checks if object is contained within the canvas with current viewportTransform * the check is done stopping at first point that appears on screen - * @param {Boolean} [calculate] use coordinates of current position instead of .aCoords * @return {Boolean} true if object is fully or partially contained within canvas */ - isOnScreen(calculate = false): boolean { + isOnScreen(): boolean { if (!this.canvas) { return false; } const { tl, br } = this.canvas.vptCoords; - const points = this.getCoords(calculate); + const points = this.getCoords(); // if some point is on screen, the object is on screen. if ( points.some( @@ -348,62 +287,41 @@ export class ObjectGeometry return true; } // no points on screen, check intersection with absolute coordinates - if (this.intersectsWithRect(tl, br, calculate)) { + if (this.intersectsWithRect(tl, br)) { return true; } - return this._containsCenterOfCanvas(tl, br, calculate); - } - - /** - * Checks if the object contains the midpoint between canvas extremities - * Does not make sense outside the context of isOnScreen and isPartiallyOnScreen - * @private - * @param {Point} pointTL Top Left point - * @param {Point} pointBR Top Right point - * @param {Boolean} calculate use coordinates of current position instead of stored ones - * @return {Boolean} true if the object contains the point - */ - private _containsCenterOfCanvas( - pointTL: Point, - pointBR: Point, - calculate?: boolean - ): boolean { - // worst case scenario the object is so big that contains the screen - const centerPoint = pointTL.midPointFrom(pointBR); - return this.containsPoint(centerPoint, calculate); + // check if the object is so big that it contains the entire viewport + return this.containsPoint(tl.midPointFrom(br)); } /** * Checks if object is partially contained within the canvas with current viewportTransform - * @param {Boolean} [calculate] use coordinates of current position instead of stored ones * @return {Boolean} true if object is partially contained within canvas */ - isPartiallyOnScreen(calculate?: boolean): boolean { + isPartiallyOnScreen(): boolean { if (!this.canvas) { return false; } const { tl, br } = this.canvas.vptCoords; - if (this.intersectsWithRect(tl, br, calculate)) { + if (this.intersectsWithRect(tl, br)) { return true; } - const allPointsAreOutside = this.getCoords(calculate).every( + const allPointsAreOutside = this.getCoords().every( (point) => (point.x >= br.x || point.x <= tl.x) && (point.y >= br.y || point.y <= tl.y) ); - return ( - allPointsAreOutside && this._containsCenterOfCanvas(tl, br, calculate) - ); + // check if the object is so big that it contains the entire viewport + return allPointsAreOutside && this.containsPoint(tl.midPointFrom(br)); } /** * Returns coordinates of object's bounding rectangle (left, top, width, height) * the box is intended as aligned to axis of canvas. - * @param {Boolean} [calculate] use coordinates of current position instead of .lineCoords / .aCoords * @return {Object} Object with left, top, width, height properties */ - getBoundingRect(calculate?: boolean): TBBox { - return makeBoundingBoxFromPoints(this.getCoords(calculate)); + getBoundingRect(): TBBox { + return makeBoundingBoxFromPoints(this.getCoords()); } /** @@ -474,9 +392,7 @@ export class ObjectGeometry } /** - * Retrieves viewportTransform from Object's canvas if possible - * @method getViewportTransform - * @memberOf FabricObject.prototype + * Retrieves viewportTransform from Object's canvas if available * @return {TMat2D} */ getViewportTransform(): TMat2D { @@ -507,11 +423,8 @@ export class ObjectGeometry /** * Sets corner and controls position coordinates based on current angle, width and height, left and top. - * aCoords are used to quickly find an object on the canvas - * lineCoords are used to quickly find object during pointer events. + * aCoords are used to quickly find an object on the canvas. * See {@link https://github.com/fabricjs/fabric.js/wiki/When-to-call-setCoords} and {@link http://fabricjs.com/fabric-gotchas} - * @param {Boolean} [skipCorners] skip calculation of aCoord, lineCoords. - * @return {void} */ setCoords(): void { this.aCoords = this.calcACoords(); diff --git a/test/unit/object_geometry.js b/test/unit/object_geometry.js index 4fbb5dd7910..77b860e8cab 100644 --- a/test/unit/object_geometry.js +++ b/test/unit/object_geometry.js @@ -310,7 +310,6 @@ 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); @@ -327,7 +326,6 @@ 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); @@ -630,13 +628,13 @@ cObj.left += 5; coords = cObj.getCoords(); - assert.deepEqual(coords[0], new fabric.Point(40, 30), 'return top left corner cached oCoords'); - assert.deepEqual(coords[1], new fabric.Point(52, 30), 'return top right corner cached oCoords'); - assert.deepEqual(coords[2], new fabric.Point(52, 47), 'return bottom right corner cached oCoords'); - assert.deepEqual(coords[3], new fabric.Point(40, 47), 'return bottom left corner cached oCoords'); + assert.deepEqual(coords[0], new fabric.Point(40, 30), 'return top left corner cached aCoords'); + assert.deepEqual(coords[1], new fabric.Point(52, 30), 'return top right corner cached aCoords'); + assert.deepEqual(coords[2], new fabric.Point(52, 47), 'return bottom right corner cached aCoords'); + assert.deepEqual(coords[3], new fabric.Point(40, 47), 'return bottom left corner cached aCoords'); - // testing calculate here - coords = cObj.getCoords(true); + cObj.setCoords(); + coords = cObj.getCoords(); assert.deepEqual(coords[0], new fabric.Point(45, 30), 'return top left corner recalculated'); assert.deepEqual(coords[1], new fabric.Point(57, 30), 'return top right corner recalculated'); assert.deepEqual(coords[2], new fabric.Point(57, 47), 'return bottom right corner recalculated');