Skip to content

Commit

Permalink
fix(Controls): forbid scaling to avoid NaN issues on scaling zero siz…
Browse files Browse the repository at this point in the history
…ed objects. Issue #9475
  • Loading branch information
gloriousjob authored Jan 11, 2024
1 parent 33b92d9 commit 1bebff0
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

## [next]

- BREAKING BETA(LayoutManager): rm `shouldResetTransform` [#9581](https://github.com/fabricjs/fabric.js/pull/9581)
- fix(Controls): forbid scaling to avoid NaN issues on scaling zero sized objects. #9475 [#9563](https://github.com/fabricjs/fabric.js/pull/9563)
- feat(LayoutManager): BREAKING remove `shouldResetTransform` handling from LayoutManager [#9581](https://github.com/fabricjs/fabric.js/pull/9581)
- refactor(): rm active selection ref [#9561](https://github.com/fabricjs/fabric.js/pull/9561)
- feat(Next.js sandbox): simpler canvas hook [#9577](https://github.com/fabricjs/fabric.js/pull/9577)
- fix(): fix modify polygon points with zero sized polygons ( particular case of axis oriented lines ) [#9575](https://github.com/fabricjs/fabric.js/pull/9575)
Expand Down
135 changes: 135 additions & 0 deletions src/controls/scale.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { Point } from '../Point';
import { Canvas } from '../canvas/Canvas';
import { Rect } from '../shapes/Rect';
import type { RectProps } from '../shapes/Rect';
import { scalingX, scalingY } from './scale';

//
// prep for tests
//
const createZeroThickRectangleScalingItems = (
rectOptions: { width: number; height: number } & Partial<RectProps>,
usedCorner: keyof Rect['oCoords'],
pointDiff: Point
) => {
const extraMargin = 100;
const { width, height } = rectOptions;

// create canvas as large as the rect + margin
const canvas = new Canvas('canvas', {
width: width + extraMargin,
height: height + extraMargin,
});

// create rect in the center of rect
const target = new Rect({
strokeWidth: 0,
stroke: 'black',
...rectOptions,
left: (width + extraMargin) / 2,
top: (height + extraMargin) / 2,
originX: 'center',
originY: 'center',
});

// add rect to canvas or we get undefined issues
canvas.add(target);

// create mouse event near center of rect, as the 0 size will put it on the middle scaler
const canvasOffset = canvas.calcOffset();

const mouseDown = new MouseEvent('mousedown', {
clientX: canvasOffset.left + target.oCoords[usedCorner].x,
clientY: canvasOffset.top + target.oCoords[usedCorner].y,
});

const moveEvent = new MouseEvent('mousemove', {
clientX: canvasOffset.left + target.oCoords[usedCorner].x + pointDiff.x,
clientY: canvasOffset.top + target.oCoords[usedCorner].y + pointDiff.y,
});

canvas.setActiveObject(target);
target.__corner = usedCorner;
canvas._setupCurrentTransform(mouseDown, target, false);
const transform = canvas._currentTransform!;
const pointer = canvas.getScenePoint(moveEvent);

// return items used by our action handler
return { moveEvent, transform, pointer, target };
};

//
// actual tests
//
describe('Scale', () => {
it('adjusts a 0 width rect for polyActionhandler without it returning Infinity/NaN side scale', () => {
// when NaN wasn't handled correctly, the polygon would disappear. Now, it stays
const { moveEvent, transform, pointer, target } =
createZeroThickRectangleScalingItems(
{ width: 0, height: 60 },
'mr',
new Point(20, 0) // moving right
);
const currentScale = target.scaleX;
const didScale = scalingX(moveEvent, transform, pointer.x, pointer.y);
expect(currentScale).toEqual(transform.target.scaleX);
expect((transform.target as Rect).scaleX).not.toBeNaN();
expect((transform.target as Rect).scaleX).not.toBe(
Number.POSITIVE_INFINITY
);
expect(didScale).toBe(false);
});
it('adjusts a 0 width rect for polyActionhandler without it returning Infinity/NaN corner scale', () => {
// when NaN wasn't handled correctly, the polygon would disappear. Now, it stays
const { moveEvent, transform, pointer, target } =
createZeroThickRectangleScalingItems(
{ width: 0, height: 60 },
'br',
new Point(20, 20) // moving right
);
const currentScaleX = target.scaleX;
const currentScaleY = target.scaleY;
const didScale = scalingX(moveEvent, transform, pointer.x, pointer.y);
expect(currentScaleX).toEqual(transform.target.scaleX);
expect(currentScaleY).toEqual(transform.target.scaleY);
expect((transform.target as Rect).scaleX).not.toBeNaN();
expect((transform.target as Rect).scaleX).not.toBe(
Number.POSITIVE_INFINITY
);
expect(didScale).toBe(false);
});
it('adjusts a 0 height rect for polyActionhandler without it returning Infinity/NaN side scale', () => {
// when NaN wasn't handled correctly, the polygon would disappear. Now, it stays
const { moveEvent, transform, pointer } =
createZeroThickRectangleScalingItems(
{ width: 60, height: 0 },
'mb',
new Point(0, 20)
);

const didScale = scalingY(moveEvent, transform, pointer.x, pointer.y);

expect((transform.target as Rect).scaleY).not.toBeNaN();
expect((transform.target as Rect).scaleY).not.toBe(
Number.POSITIVE_INFINITY
);
expect(didScale).toBe(false);
});
it('adjusts a 0 height rect for polyActionhandler without it returning Infinity/NaN corner scale', () => {
// when NaN wasn't handled correctly, the polygon would disappear. Now, it stays
const { moveEvent, transform, pointer } =
createZeroThickRectangleScalingItems(
{ width: 60, height: 0 },
'br',
new Point(20, 20)
);

const didScale = scalingY(moveEvent, transform, pointer.x, pointer.y);

expect((transform.target as Rect).scaleY).not.toBeNaN();
expect((transform.target as Rect).scaleY).not.toBe(
Number.POSITIVE_INFINITY
);
expect(didScale).toBe(false);
});
});
9 changes: 9 additions & 0 deletions src/controls/scale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ export function scalingIsForbidden(
if (lockY && by === 'y') {
return true;
}
// code crashes because of a division by 0 if a 0 sized object is scaled
// forbid to prevent scaling to happen. ISSUE-9475
const { width, height, strokeWidth } = fabricObject;
if (width === 0 && strokeWidth === 0 && by !== 'y') {
return true;
}
if (height === 0 && strokeWidth === 0 && by !== 'x') {
return true;
}
return false;
}

Expand Down

0 comments on commit 1bebff0

Please sign in to comment.