fix(IText): **NO** clear context top during rendering

CHANGELOG.md

@@ -2,8 +2,9 @@
 
 ## [next]
 
-- fix(): `_initRetinaScaling`  initializaing the scaling regardless of settings in Canvas. [#8565](https://github.com/fabricjs/fabric.js/pull/8565)
-- fix(): regression of canvas migration with pointer and sendPointToPlane [#8563](https://github.com/fabricjs/fabric.js/pull/8563)
+- fix(IText): refactor clearing context top logic of itext to align with brush pattern, using the canvas rendering cycle in order to guard from edge cases #8560
+- fix(Canvas): `_initRetinaScaling` initializaing the scaling regardless of settings in Canvas. [#8565](https://github.com/fabricjs/fabric.js/pull/8565)
+- fix(Canvas): regression of canvas migration with pointer and sendPointToPlane [#8563](https://github.com/fabricjs/fabric.js/pull/8563)
 - chore(TS): Use exports from files to build fabricJS, get rid of HEADER.js [#8549](https://github.com/fabricjs/fabric.js/pull/8549)
 - chore(): rm `fabric.filterBackend` => `getFilterBackend` [#8487](https://github.com/fabricjs/fabric.js/pull/8487)
 - chore(TS): migrate text SVG export mixin [#8486](https://github.com/fabricjs/fabric.js/pull/8486)

src/canvas/canvas.class.ts

@@ -19,11 +19,10 @@ import {
   isActiveSelection,
   isCollection,
   isFabricObjectCached,
-  isInteractiveTextObject,
 } from '../util/types';
 import { invertTransform, transformPoint } from '../util/misc/matrix';
 import { isTransparent } from '../util/misc/isTransparent';
-import { TMat2D, TOriginX, TOriginY, TSize } from '../typedefs';
+import { TOriginX, TOriginY, TSize } from '../typedefs';
 import { degreesToRadians } from '../util/misc/radiansDegreesConversion';
 import { getPointer, isTouchEvent } from '../util/dom_event';
 import type { IText } from '../shapes/itext.class';
@@ -1631,17 +1630,6 @@ export class SelectableCanvas<
     super._setSVGObject(markup, instance, reviver);
     instance.set(originalProperties);
   }
-
-  setViewportTransform(vpt: TMat2D) {
-    if (
-      this.renderOnAddRemove &&
-      isInteractiveTextObject(this._activeObject) &&
-      this._activeObject.isEditing
-    ) {
-      this._activeObject.clearContextTop();
-    }
-    super.setViewportTransform(vpt);
-  }
 }
 
 Object.assign(SelectableCanvas.prototype, {

src/canvas/canvas_events.ts

@@ -342,10 +342,12 @@ export class Canvas extends SelectableCanvas {
     source?: FabricObject,
     target?: FabricObject
   ) {
+    let dirty = false;
     const ctx = this.contextTop;
     if (source) {
       source.clearContextTop(true);
       source.renderDragSourceEffect(e);
+      dirty = true;
     }
     if (target) {
       if (target !== source) {
@@ -354,8 +356,10 @@ export class Canvas extends SelectableCanvas {
         target.clearContextTop(true);
       }
       target.renderDropTargetEffect(e);
+      dirty = true;
     }
     ctx.restore();
+    dirty && (this.contextTopDirty = true);
   }
 
   /**

src/shapes/Object/InteractiveObject.ts

@@ -512,7 +512,7 @@ export class InteractiveFabricObject<
    * Clears the canvas.contextTop in a specific area that corresponds to the object's bounding box
    * that is in the canvas.contextContainer.
    * This function is used to clear pieces of contextTop where we render ephemeral effects on top of the object.
-   * Example: blinking cursror text selection, drag effects.
+   * Example: blinking cursor text selection, drag effects.
    * @todo discuss swapping restoreManually with a renderCallback, but think of async issues
    * @param {Boolean} [restoreManually] When true won't restore the context after clear, in order to draw something else.
    * @return {CanvasRenderingContext2D|undefined} canvas.contextTop that is either still transformed

src/shapes/itext.class.ts

@@ -221,7 +221,6 @@ export class IText extends ITextClickBehaviorMixin<ITextEvents> {
    */
   initDimensions() {
     this.isEditing && this.initDelayedCursor();
-    this.clearContextTop();
     super.initDimensions();
   }
 
@@ -272,22 +271,13 @@ export class IText extends ITextClickBehaviorMixin<ITextEvents> {
    * @param {CanvasRenderingContext2D} ctx Context to render on
    */
   render(ctx: CanvasRenderingContext2D) {
-    this.clearContextTop();
     super.render(ctx);
     // clear the cursorOffsetCache, so we ensure to calculate once per renderCursor
     // the correct position but not at every cursor animation.
     this.cursorOffsetCache = {};
     this.renderCursorOrSelection();
   }
 
-  /**
-   * @private
-   * @param {CanvasRenderingContext2D} ctx Context to render on
-   */
-  _render(ctx: CanvasRenderingContext2D) {
-    super._render(ctx);
-  }
-
   /**
    * Renders cursor or selection (depending on what exists)
    * it does on the contextTop. If contextTop is not available, do nothing.
@@ -306,19 +296,10 @@ export class IText extends ITextClickBehaviorMixin<ITextEvents> {
     } else {
       this.renderSelection(ctx, boundaries);
     }
+    this.canvas!.contextTopDirty = true;
     ctx.restore();
   }
 
-  /**
-   * Renders cursor on context Top, outside the animation cycle, on request
-   * Used for the drag/drop effect.
-   * If contextTop is not available, do nothing.
-   */
-  renderCursorAt(selectionStart) {
-    const boundaries = this._getCursorBoundaries(selectionStart, true);
-    this._renderCursor(this.canvas.contextTop, boundaries, selectionStart);
-  }
-
   /**
    * Returns cursor boundaries (left, top, leftOffset, topOffset)
    * left/top are left/top of entire text box
@@ -406,6 +387,16 @@ export class IText extends ITextClickBehaviorMixin<ITextEvents> {
     return boundaries;
   }
 
+  /**
+   * Renders cursor on context Top, outside the animation cycle, on request
+   * Used for the drag/drop effect.
+   * If contextTop is not available, do nothing.
+   */
+  renderCursorAt(selectionStart: number) {
+    const boundaries = this._getCursorBoundaries(selectionStart, true);
+    this._renderCursor(this.canvas.contextTop, boundaries, selectionStart);
+  }
+
   /**
    * Renders cursor
    * @param {Object} boundaries
@@ -468,11 +459,7 @@ export class IText extends ITextClickBehaviorMixin<ITextEvents> {
    * Renders drag start text selection
    */
   renderDragSourceEffect() {
-    if (
-      this.__isDragging &&
-      this.__dragStartSelection &&
-      this.__dragStartSelection
-    ) {
+    if (this.__isDragging && this.__dragStartSelection) {
       this._renderSelection(
         this.canvas.contextTop,
         this.__dragStartSelection,

src/shapes/textbox.class.ts

@@ -52,7 +52,6 @@ export class Textbox extends IText {
       return;
     }
     this.isEditing && this.initDelayedCursor();
-    this.clearContextTop();
     this._clearCache();
     // clear dynamicMinWidth as it will be different after we re-wrap line
     this.dynamicMinWidth = 0;

test/lib/visualTestLoop.js

@@ -188,7 +188,7 @@
           }
           await fabricCanvas.dispose();
           done();
-        });
+        }, assert);
       });
     }
   }

test/visual/freedraw.js

@@ -3,13 +3,14 @@
     canvas.freeDrawingBrush = brush;
   }
   var options = { e: { pointerId: 1 } };
-  function pointDrawer(points, brush, fireUp = false) {
+  function pointDrawer(points, brush, fireUp = false, onMove = undefined) {
     setBrush(brush.canvas, brush);
     brush.onMouseDown(points[0], options);
     for (var i = 1; i < points.length; i++) {
       points[i].x = parseFloat(points[i].x);
       points[i].y = parseFloat(points[i].y);
       brush.onMouseMove(points[i], options);
+      onMove && onMove(points[i], i, points);
     }
     if (fireUp) {
       brush.onMouseUp(options);
@@ -2255,6 +2256,31 @@ QUnit.module('Free Drawing', hooks => {
     }
   });
 
+  function withText(canvas) {
+    canvas.add(new fabric.IText('This textbox should NOT\nclear the brush during rendering'));
+    const brush = new fabric.PencilBrush(canvas);
+    brush.color = 'red';
+    brush.width = 25;
+    pointDrawer(points, brush, false, (point, index, points) => index === points.length - 1 && canvas.renderAll());
+  }
+
+  tests.push({
+    test: 'textbox should not clear brush',
+    build: withText,
+    golden: 'withText.png',
+    percentage: 0.02,
+    width: 200,
+    height: 250,
+    fabricClass: 'Canvas',
+    targets: {
+      top: true,
+      main: false,
+      mesh: true,
+      result: false,
+      compare: false
+    }
+  });
+
   tests.forEach(function (test) {
     var options = Object.assign({}, freeDrawingTestDefaults, test.targets);
     if (options.top) {

test/visual/text.js

@@ -491,5 +491,101 @@
     percentage: 0.092,
   });
 
+
+  // sinon could have spied this w/o effort and in one line
+  class TestTextbox extends fabric.Textbox {
+    __calledInitDimensions = 0;
+    initDimensions() {
+      super.initDimensions();
+      this.initialized && this.__calledInitDimensions++;
+    }
+  }
+
+  function selectionClearingEdgeCases(canvas, callback, assert) {
+    const text = new TestTextbox('lorem ipsum dolor sit Amet consectgetur', {
+      width: 200,
+      centeredRotation: true
+    });
+    canvas.add(text);
+    canvas.setActiveObject(text);
+    text.enterEditing();
+    text.selectAll();
+    canvas.renderAll();
+    text.rotate(90);
+    text.scale(0.8);
+    canvas.centerObject(text);
+    canvas.renderAll();
+    assert.equal(text.__calledInitDimensions, 0, 'initDimensions was not called');
+    canvas.getContext().drawImage(canvas.upperCanvasEl, 0, 0);
+    callback(canvas.lowerCanvasEl);
+  }
+
+  tests.push({
+    test: 'Text selection clearing edge cases: transform',
+    code: selectionClearingEdgeCases,
+    width: 200,
+    height: 200,
+    disabled: fabric.getEnv().isLikelyNode,
+    golden: 'textSelectionClearing.png',
+    percentage: 0.02,
+    fabricClass: 'Canvas'
+  });
+
+  function selectionClearingEdgeCases2(canvas, callback, assert) {   
+    const text = new TestTextbox('lorem ipsum dolor sit Amet sit Amet', {
+      width: 200,
+    });
+    canvas.add(text);
+    canvas.setActiveObject(text);
+    text.enterEditing();
+    text.selectAll();
+    assert.ok(canvas.contextTopDirty, 'flagged as dirty');
+    canvas.renderAll();
+    text.width = 150;
+    text._forceClearCache = true;
+    canvas.renderAll();
+    assert.equal(text.__calledInitDimensions, 1, 'initDimensions was called');
+    canvas.getContext().drawImage(canvas.upperCanvasEl, 0, 0);
+    callback(canvas.lowerCanvasEl);
+  }
+
+  tests.push({
+    test: 'Text selection clearing edge cases: changing width, `initDimensions`',
+    code: selectionClearingEdgeCases2,
+    width: 200,
+    height: 200,
+    disabled: fabric.getEnv().isLikelyNode,
+    golden: 'textSelectionClearing2.png',
+    percentage: 0.02,
+    fabricClass: 'Canvas'
+  });
+
+  function selectionClearingEdgeCases3(canvas, callback, assert) {
+    const text = new TestTextbox('lorem ipsum dolor sit Amet consectgetur', {
+      width: 200
+    });
+    canvas.add(text);
+    canvas.setActiveObject(text);
+    text.enterEditing();
+    text.selectAll();
+    canvas.renderAll();
+    canvas.setViewportTransform([0.8, 0, 0, 1, 0, 0]);
+    canvas.renderAll();
+    assert.equal(text.__calledInitDimensions, 0, 'initDimensions was not called');
+    canvas.getContext().drawImage(canvas.upperCanvasEl, 0, 0);
+    callback(canvas.lowerCanvasEl);
+  }
+
+  tests.push({
+    test: 'Text selection clearing edge cases: vpt',
+    code: selectionClearingEdgeCases3,
+    width: 200,
+    height: 200,
+    disabled: fabric.getEnv().isLikelyNode,
+    golden: 'textSelectionClearing3.png',
+    percentage: 0.03,
+    fabricClass: 'Canvas'
+  });
+
   tests.forEach(visualTestLoop(QUnit));
 })();