fix(FabricObject): Render clipPath as sharp as the object (#9774)

.codesandbox/templates/vanilla/src/index.ts

@@ -1,6 +1,6 @@
 import * as fabric from 'fabric';
 import './styles.css';
-import { testCase } from './testcases/responsive';
+import { testCase } from './testcases/loadingSvgs';
 
 const el = document.getElementById('canvas');
 const canvas = (window.canvas = new fabric.Canvas(el));

.codesandbox/templates/vanilla/src/testcases/loadingSvgs.ts

@@ -0,0 +1,24 @@
+import * as fabric from 'fabric';
+
+const svgString = `<svg xmlns="http://www.w3.org/2000/svg" xmlns:svg="http://www.w3.org/2000/svg" viewBox="5 3 10 10" version="1.1" width="400" height="400">
+<g>
+    <g>
+    <clipPath id="a">
+        <circle transform="scale(1.2, 1.2) translate(-1, -1)"  r="4"  cx="8" cy="8" />
+    </clipPath>
+    <clipPath id="t" clip-path="url(#a)">
+        <circle r="6" transform="scale(1.3, 0.8) translate(1, 1)"  cx="7" cy="7" />
+    </clipPath>
+    <clipPath id="c" clip-path="url(#t)" >
+        <circle  transform="translate(12, 10) scale(14, 14)"  r="0.5"  cx="0.01" cy="0.01" />
+    </clipPath>
+<path clip-path="url(#c)" d="M15.422,18.129l-5.264-2.768l-5.265,2.768l1.006-5.863L1.64,8.114l5.887-0.855
+	l2.632-5.334l2.633,5.334l5.885,0.855l-4.258,4.152L15.422,18.129z" fill="red"/>
+    </g>
+</g>
+</svg>`;
+
+export async function testCase(canvas: fabric.Canvas) {
+  const svg = await fabric.loadSVGFromString(svgString);
+  canvas.add(...(svg.objects.filter((obj) => !!obj) as fabric.FabricObject[]));
+}

.github/workflows/tests.yml

@@ -40,7 +40,7 @@ jobs:
         uses: ./.github/actions/build-fabric-cached
       - name: Run ${{ matrix.suite }} tests with coverage
         if: matrix.suite == 'unit'
-        run: npm run test:coverage
+        run: npm run test:coverage && sleep 5
       - name: Run ${{ matrix.suite }} tests with coverage
         if: matrix.suite == 'visual'
         run: npm run test:visual:coverage
@@ -49,6 +49,7 @@ jobs:
         with:
           name: coverage-${{ matrix.suite }}
           path: .nyc_output/*.json
+
   browser:
     needs: [prime-build]
     name: ${{ matrix.target }} ${{ matrix.suite }} tests

CHANGELOG.md

@@ -1,7 +1,12 @@
 # Changelog
 
+## [next]
+
+- fix(FabricObject): Fix clipPath blurryness with scale [#9774](https://github.com/fabricjs/fabric.js/pull/9774)
+
 ## [6.4.3]
 
+- fix(FabricObject): Render clipPath as sharp as the object [#9774](https://github.com/fabricjs/fabric.js/pull/9774)
 - fix(Controls): changeWidth can change width with decimals [#10186](https://github.com/fabricjs/fabric.js/pull/10186)
 - ci(): Add some prebuilt fabric in the dist folder [#10178](https://github.com/fabricjs/fabric.js/pull/10178)
 - chore(): Add more generic font families to FabricText.genericFonts [#10167](https://github.com/fabricjs/fabric.js/pull/10167)

src/canvas/StaticCanvas.ts

@@ -26,7 +26,7 @@ import {
 } from '../util/animation/AnimationFrameProvider';
 import { runningAnimations } from '../util/animation/AnimationRegistry';
 import { uid } from '../util/internals/uid';
-import { createCanvasElement, toDataURL } from '../util/misc/dom';
+import { createCanvasElementFor, toDataURL } from '../util/misc/dom';
 import { invertTransform, transformPoint } from '../util/misc/matrix';
 import type { EnlivenObjectOptions } from '../util/misc/objectEnlive';
 import {
@@ -585,9 +585,10 @@ export class StaticCanvas<
     if (path) {
       path._set('canvas', this);
       // needed to setup a couple of variables
+      // todo migrate to the newer one
       path.shouldCache();
       path._transformDone = true;
-      path.renderCache({ forClipping: true });
+      (path as TCachedFabricObject).renderCache({ forClipping: true });
       this.drawClipPathOnCanvas(ctx, path as TCachedFabricObject);
     }
     this._renderOverlay(ctx);
@@ -1334,9 +1335,7 @@ export class StaticCanvas<
    * This essentially copies canvas dimensions since loadFromJSON does not affect canvas size.
    */
   cloneWithoutData() {
-    const el = createCanvasElement();
-    el.width = this.width;
-    el.height = this.height;
+    const el = createCanvasElementFor(this);
     return new (this.constructor as Constructor<this>)(el);
   }
 
@@ -1425,12 +1424,13 @@ export class StaticCanvas<
       translateY = (vp[5] - (top || 0)) * multiplier,
       newVp = [newZoom, 0, 0, newZoom, translateX, translateY] as TMat2D,
       originalRetina = this.enableRetinaScaling,
-      canvasEl = createCanvasElement(),
+      canvasEl = createCanvasElementFor({
+        width: scaledWidth,
+        height: scaledHeight,
+      }),
       objectsToRender = filter
         ? this._objects.filter((obj) => filter(obj))
         : this._objects;
-    canvasEl.width = scaledWidth;
-    canvasEl.height = scaledHeight;
     this.enableRetinaScaling = false;
     this.viewportTransform = newVp;
     this.width = scaledWidth;

src/filters/BaseFilter.ts

@@ -1,5 +1,4 @@
 import { getEnv } from '../env';
-import { createCanvasElement } from '../util/misc/dom';
 import type {
   T2DPipelineState,
   TWebGLAttributeLocationMap,
@@ -15,6 +14,7 @@ import {
 } from './shaders/baseFilter';
 import type { Abortable } from '../typedefs';
 import { FabricError } from '../util/internals/console';
+import { createCanvasElementFor } from '../util/misc/dom';
 
 const regex = new RegExp(highPsourceCode, 'g');
 
@@ -368,9 +368,11 @@ export class BaseFilter<
    */
   createHelpLayer(options: T2DPipelineState) {
     if (!options.helpLayer) {
-      const helpLayer = createCanvasElement();
-      helpLayer.width = options.sourceWidth;
-      helpLayer.height = options.sourceHeight;
+      const { sourceWidth, sourceHeight } = options;
+      const helpLayer = createCanvasElementFor({
+        width: sourceWidth,
+        height: sourceHeight,
+      });
       options.helpLayer = helpLayer;
     }
   }

src/filters/WebGLFilterBackend.ts

@@ -1,5 +1,5 @@
 import { config } from '../config';
-import { createCanvasElement } from '../util/misc/dom';
+import { createCanvasElementFor } from '../util/misc/dom';
 import type {
   TWebGLPipelineState,
   TProgramCache,
@@ -72,9 +72,7 @@ export class WebGLFilterBackend {
    * class properties to the GLFilterBackend class.
    */
   createWebGLCanvas(width: number, height: number): void {
-    const canvas = createCanvasElement();
-    canvas.width = width;
-    canvas.height = height;
+    const canvas = createCanvasElementFor({ width, height });
     const glOptions = {
         alpha: true,
         premultipliedAlpha: false,

src/filters/utils.ts

@@ -1,5 +1,5 @@
 import { getFabricWindow } from '../env';
-import { createCanvasElement } from '../util/misc/dom';
+import { createCanvasElement, createCanvasElementFor } from '../util/misc/dom';
 import { WebGLFilterBackend } from './WebGLFilterBackend';
 import type { TWebGLPipelineState, T2DPipelineState } from './typedefs';
 
@@ -16,7 +16,7 @@ export const isWebGLPipelineState = (
  * putImageData is faster than drawImage for that specific operation.
  */
 export const isPutImageFaster = (width: number, height: number): boolean => {
-  const targetCanvas = createCanvasElement();
+  const targetCanvas = createCanvasElementFor({ width, height });
   const sourceCanvas = createCanvasElement();
   const gl = sourceCanvas.getContext('webgl')!;
   // eslint-disable-next-line no-undef
@@ -31,8 +31,6 @@ export const isPutImageFaster = (width: number, height: number): boolean => {
     targetCanvas: targetCanvas,
   } as unknown as TWebGLPipelineState;
   let startTime;
-  targetCanvas.width = width;
-  targetCanvas.height = height;
 
   startTime = getFabricWindow().performance.now();
   WebGLFilterBackend.prototype.copyGLTo2D.call(

src/parser/elements_parser.ts

@@ -135,7 +135,11 @@ export class ElementsParser {
 
   // TODO: resolveClipPath could be run once per clippath with minor work per object.
   // is a refactor that i m not sure is worth on this code
-  async resolveClipPath(obj: NotParsedFabricObject, usingElement: Element) {
+  async resolveClipPath(
+    obj: NotParsedFabricObject,
+    usingElement: Element,
+    exactOwner?: Element,
+  ) {
     const clipPathElements = this.extractPropertyDefinition(
       obj,
       'clipPath',
@@ -146,6 +150,7 @@ export class ElementsParser {
       const clipPathTag = clipPathElements[0].parentElement!;
       let clipPathOwner = usingElement;
       while (
+        !exactOwner &&
         clipPathOwner.parentElement &&
         clipPathOwner.getAttribute('clip-path') !== obj.clipPath
       ) {
@@ -188,7 +193,14 @@ export class ElementsParser {
         clipPath.calcTransformMatrix(),
       );
       if (clipPath.clipPath) {
-        await this.resolveClipPath(clipPath, clipPathOwner);
+        await this.resolveClipPath(
+          clipPath,
+          clipPathOwner,
+          // this is tricky.
+          // it tries to differentiate from when clipPaths are inherited by outside groups
+          // or when are really clipPaths referencing other clipPaths
+          clipPathTag.getAttribute('clip-path') ? clipPathOwner : undefined,
+        );
       }
       const { scaleX, scaleY, angle, skewX, translateX, translateY } =
         qrDecompose(gTransform);

src/parser/parseSVGDocument.ts

@@ -54,7 +54,6 @@ export async function parseSVGDocument(
       crossOrigin,
       signal,
     };
-
   const elements = descendants.filter((el) => {
     applyViewboxTransform(el);
     return isValidSvgTag(el) && !hasInvalidAncestor(el); // http://www.w3.org/TR/SVG/struct.html#DefsElement

src/shapes/Group.ts

@@ -34,6 +34,7 @@ import {
 } from '../LayoutManager/constants';
 import type { SerializedLayoutManager } from '../LayoutManager/LayoutManager';
 import type { FitContentLayout } from '../LayoutManager';
+import type { DrawContext } from './Object/Object';
 
 /**
  * This class handles the specific case of creating a group using {@link Group#fromObject} and is not meant to be used in any other case.
@@ -42,7 +43,6 @@ import type { FitContentLayout } from '../LayoutManager';
  * This layout manager doesn't do anything and therefore keeps the exact layout the group had when {@link Group#toObject} was called.
  */
 class NoopLayoutManager extends LayoutManager {
-  // eslint-disable-next-line @typescript-eslint/no-empty-function
   performLayout() {}
 }
 
@@ -493,23 +493,25 @@ export class Group
    * Execute the drawing operation for an object on a specified context
    * @param {CanvasRenderingContext2D} ctx Context to render on
    */
-  drawObject(ctx: CanvasRenderingContext2D) {
+  drawObject(
+    ctx: CanvasRenderingContext2D,
+    forClipping: boolean | undefined,
+    context: DrawContext,
+  ) {
     this._renderBackground(ctx);
     for (let i = 0; i < this._objects.length; i++) {
+      const obj = this._objects[i];
       // TODO: handle rendering edge case somehow
-      if (
-        this.canvas?.preserveObjectStacking &&
-        this._objects[i].group !== this
-      ) {
+      if (this.canvas?.preserveObjectStacking && obj.group !== this) {
         ctx.save();
         ctx.transform(...invertTransform(this.calcTransformMatrix()));
-        this._objects[i].render(ctx);
+        obj.render(ctx);
         ctx.restore();
-      } else if (this._objects[i].group === this) {
-        this._objects[i].render(ctx);
+      } else if (obj.group === this) {
+        obj.render(ctx);
       }
     }
-    this._drawClipPath(ctx, this.clipPath);
+    this._drawClipPath(ctx, this.clipPath, context);
   }
 
   /**

src/shapes/Image.ts

@@ -11,7 +11,7 @@ import type {
   TOptions,
 } from '../typedefs';
 import { uid } from '../util/internals/uid';
-import { createCanvasElement } from '../util/misc/dom';
+import { createCanvasElementFor } from '../util/misc/dom';
 import { findScaleToCover, findScaleToFit } from '../util/misc/findScaleTo';
 import type { LoadImageOptions } from '../util/misc/objectEnlive';
 import {
@@ -497,19 +497,16 @@ export class FabricImage<
       this._lastScaleY = scaleY;
       return;
     }
-    const canvasEl = createCanvasElement(),
-      sourceWidth = elementToFilter.width,
-      sourceHeight = elementToFilter.height;
-    canvasEl.width = sourceWidth;
-    canvasEl.height = sourceHeight;
+    const canvasEl = createCanvasElementFor(elementToFilter),
+      { width, height } = elementToFilter;
     this._element = canvasEl;
     this._lastScaleX = filter.scaleX = scaleX;
     this._lastScaleY = filter.scaleY = scaleY;
     getFilterBackend().applyFilters(
       [filter],
       elementToFilter,
-      sourceWidth,
-      sourceHeight,
+      width,
+      height,
       this._element,
     );
     this._filterScalingX = canvasEl.width / this._originalElement.width;
@@ -549,9 +546,10 @@ export class FabricImage<
     if (this._element === this._originalElement) {
       // if the _element a reference to _originalElement
       // we need to create a new element to host the filtered pixels
-      const canvasEl = createCanvasElement();
-      canvasEl.width = sourceWidth;
-      canvasEl.height = sourceHeight;
+      const canvasEl = createCanvasElementFor({
+        width: sourceWidth,
+        height: sourceHeight,
+      });
       this._element = canvasEl;
       this._filteredEl = canvasEl;
     } else if (this._filteredEl) {