Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(): gradient transform + rm workarounds #9359

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## [next]

- fix(): gradient transform + rm workarounds [#9359](https://github.com/fabricjs/fabric.js/pull/9359)
**BREAKING**:
- `toLive(ctx)` => `toLive(ctx, target)`
- rm(): `_applyPatternGradientTransformText`, `_applyPatternGradientTransform`
- fix(IText): cursor width under group [#9341](https://github.com/fabricjs/fabric.js/pull/9341)
- TS(Canvas): constructor optional el [#9348](https://github.com/fabricjs/fabric.js/pull/9348)

Expand Down
7 changes: 5 additions & 2 deletions e2e/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ import setupApp from './setupApp';
import setupCoverage from './setupCoverage';
import setupSelectors from './setupSelectors';

export default () => {
/**
* @param {Function} [testConfig] pass data/config from the test to the browser
*/
export default (testConfig?: () => any) => {
// call first
setupSelectors();
// call before using fabric
setupCoverage();
// call at the end - navigates the page
setupApp();
setupApp(testConfig);
};
6 changes: 5 additions & 1 deletion e2e/setup/setupApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@ import path from 'path';
import imports from '../imports';
import { JSDOM } from 'jsdom';

export default () => {
/**
* @param {Function} [testConfig] pass data/config from the test to the browser
*/
export default (testConfig: () => any = () => {}) => {
test.beforeEach(async ({ page }, { file }) => {
await page.exposeFunction('testConfig', testConfig);
await page.goto('/e2e/site');
// expose imports for consumption
page.addScriptTag({
Expand Down
61 changes: 61 additions & 0 deletions e2e/tests/gradient-transform/common.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Runs from both the browser and node
*/

export function render(
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
fabric: typeof import('fabric'),
{ type, el }: { type: 'text' | 'rect'; el?: HTMLCanvasElement }
) {
const targets = new Array(8).fill(0).map((_, index) => {
const angle = index * 45;
const gradient = new fabric.Gradient({
coords: {
x1: 0,
y1: 0,
x2: 1,
y2: 1,
},
gradientUnits: 'percentage',
// offsetX: 150,
gradientTransform: fabric.util.createRotateMatrix({ angle }),
colorStops: [
{
offset: 0,
color: 'red',
},
{
offset: 1,
color: 'blue',
},
],
});

return type === 'text'
? new fabric.Text(`Gradient\n${angle}°`, {
fontSize: 50,
fontWeight: 'bold',
fill: gradient,
left: (index % 4) * 250,
top: Math.floor(index / 4) * 250,
})
: new fabric.Rect({
width: 150,
height: 150,
fill: gradient,
left: (index % 4) * 250,
top: Math.floor(index / 4) * 250,
});
});

const canvas = new fabric.StaticCanvas(el, {
width: 1000,
height: 400,
backgroundColor: 'white',
enableRetinaScaling: false,
});
canvas.add(...targets);
canvas.renderAll();

return { canvas };
}
36 changes: 36 additions & 0 deletions e2e/tests/gradient-transform/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { expect, test } from '@playwright/test';
import type { StaticCanvas } from 'fabric/node';
import setup from '../../setup';
import { CanvasUtil } from '../../utils/CanvasUtil';
import { render } from './common';

for (const type of ['rect', 'text'] as const) {
test.describe(`Gradient transform on ${type}`, () => {
setup(() => ({ type }));
test('Gradient transform', async ({ page }, config) => {
await test.step('browser', async () => {
expect(
await new CanvasUtil(page).screenshot({ element: 'main' }),
'browser snapshot'
).toMatchSnapshot({
name: `${type}.png`,
maxDiffPixelRatio: 0.05,
});
});

await test.step('node', async () => {
// we want the browser snapshot of a test to be committed and not the node snapshot
config.config.updateSnapshots = 'none';
expect(
(
(await render(await import('../../..'), { type }))
.canvas as StaticCanvas
)
.getNodeCanvas()
.toBuffer(),
'node snapshot should match browser snapshot'
).toMatchSnapshot({ name: `${type}.png`, maxDiffPixelRatio: 0.05 });
});
});
});
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 11 additions & 0 deletions e2e/tests/gradient-transform/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Runs in the **BROWSER**
*/

import * as fabric from 'fabric';
import { before } from '../test';
import { render } from './common';

before('#canvas', async (el) =>
render(fabric, { ...(await testConfig()), el })
);
18 changes: 13 additions & 5 deletions e2e/tests/template-with-node/common.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved canas init to the common method

Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,24 @@
* Runs from both the browser and node
*/

import type { StaticCanvas } from 'fabric';

// eslint-disable-next-line @typescript-eslint/consistent-type-imports
export function render(canvas: StaticCanvas, fabric: typeof import('fabric')) {
canvas.setDimensions({ width: 200, height: 70 });
export function render(
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
fabric: typeof import('fabric'),
{ el }: { el?: HTMLCanvasElement } = {}
) {
const canvas = new fabric.StaticCanvas(el, {
width: 200,
height: 70,
backgroundColor: 'white',
enableRetinaScaling: false,
});
const textbox = new fabric.Textbox('fabric.js test', {
width: 200,
top: 20,
});
canvas.add(textbox);
canvas.centerObjectH(textbox);
canvas.renderAll();

return { canvas, objects: { textbox } };
}
8 changes: 5 additions & 3 deletions e2e/tests/template-with-node/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { expect, test } from '@playwright/test';
import type { StaticCanvas } from 'fabric/node';
import setup from '../../setup';
import { CanvasUtil } from '../../utils/CanvasUtil';
import { createNodeSnapshot } from '../../utils/createNodeSnapshot';
import { render } from './common';

setup();

test('TEST NAME', async ({ page }, config) => {
await test.step('browser', async () => {
expect(
await new CanvasUtil(page).screenshot(),
await new CanvasUtil(page).screenshot({ element: 'main' }),
'browser snapshot'
).toMatchSnapshot({
name: 'textbox.png',
Expand All @@ -21,7 +21,9 @@ test('TEST NAME', async ({ page }, config) => {
// we want the browser snapshot of a test to be committed and not the node snapshot
config.config.updateSnapshots = 'none';
expect(
await createNodeSnapshot(render),
((await render(await import('../../..'))).canvas as StaticCanvas)
.getNodeCanvas()
.toBuffer(),
'node snapshot should match browser snapshot'
).toMatchSnapshot({ name: 'textbox.png', maxDiffPixelRatio: 0.05 });
});
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/template-with-node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

import * as fabric from 'fabric';
import { beforeAll } from '../test';
import { before } from '../test';
import { render } from './common';

beforeAll((canvas) => render(canvas, fabric), { enableRetinaScaling: false });
before('#canvas', (el) => render(fabric, { el }));
11 changes: 7 additions & 4 deletions e2e/tests/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
*/

import type { Object as FabricObject } from 'fabric';
import { Canvas } from 'fabric';
import { Canvas, type StaticCanvas } from 'fabric';
import * as fabric from 'fabric';

const canvasMap = (window.canvasMap = new Map<HTMLCanvasElement, Canvas>());
const canvasMap = (window.canvasMap = new Map<
HTMLCanvasElement,
Canvas | StaticCanvas
>());
const objectMap = (window.objectMap = new Map<string, FabricObject>());

type AsyncReturnValue<T> = T | Promise<T>;
Expand All @@ -33,7 +36,7 @@ export function before(
* @returns a map of objects for playwright to access during tests
*/
cb: (canvas: HTMLCanvasElement) => AsyncReturnValue<{
canvas: Canvas;
canvas: Canvas | StaticCanvas;
objects?: Record<string, FabricObject>;
}>
) {
Expand Down Expand Up @@ -72,7 +75,7 @@ export function beforeAll(

export function after(
selector: string,
cb: (canvas: Canvas) => AsyncReturnValue<void>
cb: (canvas: Canvas | StaticCanvas) => AsyncReturnValue<void>
) {
teardownTasks.push(() => {
const el = document.querySelector<HTMLCanvasElement>(selector);
Expand Down
12 changes: 10 additions & 2 deletions e2e/utils/CanvasUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,17 @@ export class CanvasUtil {
return this.page.keyboard.press(`${modifier}+KeyV`);
}

screenshot(options: LocatorScreenshotOptions = {}) {
screenshot({
element = 'wrapper',
...options
}: LocatorScreenshotOptions & { element?: 'wrapper' | 'top' | 'main' } = {}) {
const selector = {
wrapper: `canvas_wrapper=${this.selector}`,
top: `canvas_top=${this.selector}`,
main: this.selector,
}[element];
return this.page
.locator(`canvas_wrapper=${this.selector}`)
.locator(selector)
.screenshot({ omitBackground: true, ...options });
}

Expand Down
27 changes: 25 additions & 2 deletions src/Pattern/Pattern.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import type {
PatternOptions,
SerializedPatternOptions,
} from './types';
import { createTranslateMatrix, multiplyTransformMatrixArray } from '../util';
import type { StaticCanvas } from '../canvas/StaticCanvas';
import { FabricObject } from '../shapes/Object/Object';

/**
* @see {@link http://fabricjs.com/patterns demo}
Expand Down Expand Up @@ -125,7 +128,10 @@ export class Pattern {
* @param {CanvasRenderingContext2D} ctx Context to create pattern
* @return {CanvasPattern}
*/
toLive(ctx: CanvasRenderingContext2D): CanvasPattern | null {
toLive(
ctx: CanvasRenderingContext2D,
target: StaticCanvas | FabricObject
): CanvasPattern | null {
if (
// if the image failed to load, return, and allow rest to continue loading
!this.source ||
Expand All @@ -138,7 +144,24 @@ export class Pattern {
return null;
}

return ctx.createPattern(this.source, this.repeat)!;
const pattern = ctx.createPattern(this.source, this.repeat)!;
const { patternTransform, offsetX = 0, offsetY = 0 } = this;
const { x, y } =
// correct rendering position from object rendering origin (center) to tl
target instanceof FabricObject
? { x: -target.width / 2, y: -target.height / 2 }
: { x: 0, y: 0 };
if (patternTransform || offsetX || offsetY || x || y) {
pattern.setTransform(
new DOMMatrix(
multiplyTransformMatrixArray([
patternTransform,
createTranslateMatrix(offsetX + x, offsetY + y),
])
)
);
}
return pattern;
}

/**
Expand Down
9 changes: 1 addition & 8 deletions src/canvas/StaticCanvas.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { config } from '../config';
import { CENTER, VERSION } from '../constants';
import type { CanvasEvents, StaticCanvasEvents } from '../EventTypeDefs';
import type { Gradient } from '../gradient/Gradient';
import { createCollectionMixin } from '../Collection';
import { CommonMethods } from '../CommonMethods';
import type { Pattern } from '../Pattern';
Expand Down Expand Up @@ -650,16 +649,10 @@ export class StaticCanvas<
ctx.lineTo(this.width, this.height);
ctx.lineTo(0, this.height);
ctx.closePath();
ctx.fillStyle = isAFiller ? fill.toLive(ctx /* this */)! : fill;
ctx.fillStyle = isAFiller ? fill.toLive(ctx, this)! : fill;
if (needsVpt) {
ctx.transform(...v);
}
if (isAFiller) {
ctx.transform(1, 0, 0, 1, fill.offsetX || 0, fill.offsetY || 0);
const m = ((fill as Gradient<'linear'>).gradientTransform ||
(fill as Pattern).patternTransform) as TMat2D;
m && ctx.transform(...m);
}
ctx.fill();
ctx.restore();
}
Expand Down
Loading