Address issues when filling or stroking out-of-visual-bounds Selections (#99)

When zooming in and panning the Viewport while a Selection is active, if a bucket fill or stroke operation is used, only the visible area of the Selection is filled.

This should be updated to fill the full area, even the out of visual bounds area.

This changeset also fixes an issue where stroke width would be applied differently at different zoom levels.

src/definitions/editor.ts

@@ -100,6 +100,13 @@ export type BrushAction = {
     selection: Selection;
 };
 
+export type PaintProps = {
+    paintCanvas?: CanvasContextPairing; // temporary canvas used during drawing
+    useViewport?: boolean; // whether to consider Viewport coordinates when applying paint canvas onto source
+    orgSource?: HTMLCanvasElement; // source content before painting started (used for history states)
+    pendingSave?: ReturnType<typeof setTimeout>;
+};
+
 export type ZoomToolOptions = {
     level: number;
 };

src/rendering/actors/layer-renderer.ts

@@ -29,7 +29,7 @@ import { BlendModes } from "@/definitions/blend-modes";
 import type { Document } from "@/model/types/document";
 import type { Layer } from "@/model/types/layer";
 import type { Selection } from "@/model/types/selection";
-import type { CanvasContextPairing, CanvasDrawable, Brush, BrushToolOptions, BrushAction } from "@/definitions/editor";
+import type { Brush, BrushAction, BrushToolOptions, CanvasContextPairing, CanvasDrawable, PaintProps } from "@/definitions/editor";
 import { isPixelArt } from "@/definitions/editor-properties";
 import { LayerTypes } from "@/definitions/layer-types";
 import ToolTypes, { canDragMask, TOOL_SRC_MERGED } from "@/definitions/tool-types";
@@ -87,15 +87,13 @@ export default class LayerRenderer extends ZoomableSprite {
     protected _pointer: Point;
     protected _brush: Brush;
     protected _lastBrushIndex: number;
-    protected _paintCanvas: CanvasContextPairing; // temporary canvas used during drawing
+    protected _paintProps: PaintProps = {};
     protected _isPaintMode: boolean;
     protected _isDragMode: boolean;
     protected _isColorPicker: boolean;
     protected _selection: Selection;
     protected _invertSelection: boolean;
     protected _toolType: ToolTypes;
-    protected _orgSourceToStore: HTMLCanvasElement | undefined;
-    protected _pendingPaintState: ReturnType<typeof setTimeout>;
     protected _pendingEffectsRender: boolean;
     protected _unmaskedBitmap: HTMLCanvasElement | undefined; // a reference to the effected source w/out mask applied
     protected _draggingMask: Point | undefined;
@@ -307,34 +305,38 @@ export default class LayerRenderer extends ZoomableSprite {
     // draw onto the source Bitmap (e.g. brushing / fill tool / eraser)
 
     paint( optAction: BrushAction = null ): void {
-        // most drawing operations operate directly onto a temporary Canvas
-        const usePaintCanvas = this.usePaintCanvas() || optAction?.type === "stroke";
-        
-        if ( !this._pendingPaintState || ( usePaintCanvas && !this._paintCanvas )) {
-            this.preparePendingPaintState( usePaintCanvas );
-        }
         const isCloneStamp = this._toolType === ToolTypes.CLONE;
         const isFillMode   = this._toolType === ToolTypes.FILL;
+        const isStroking   = optAction?.type === "stroke";
         const isDrawing    = this.isDrawing();
+
+        // most drawing operations operate directly onto a temporary Canvas
+        const usePaintCanvas = this.usePaintCanvas() || isStroking;
         
+        if ( !this._paintProps.pendingSave || ( usePaintCanvas && !this._paintProps.paintCanvas )) {
+            this.preparePendingPaintState( usePaintCanvas );
+        }
+
         // get the drawing context
         let ctx = this.getPaintSource().getContext( "2d" ) as CanvasRenderingContext2D;
         const { width, height } = ctx.canvas;
 
         // if there is an active selection, painting will be constrained within
         let selection: Selection = optAction?.selection || this._selection;
-
+        // selection fill/stroke operations should ignore viewport offset (allow filling of out-of-visual-bounds selection)
+        this._paintProps.useViewport = !( usePaintCanvas && ( isFillMode || isStroking ) && !!selection );
+   
         // get the enqueued pointers which are to be rendered in this paint cycle
         const pointers  = isDrawing ? sliceBrushPointers( this._brush ) : undefined;
-        const overrides = createOverrideConfig( this.canvas, pointers );
+        const overrides = createOverrideConfig( this.canvas, this._paintProps, pointers );
         
-        const doSaveRestore  = !!selection; // selections will apply context clipping which needs to be restored
+        const restoreContext = !!selection; // selections will apply context clipping which needs to be restored
 
         if ( usePaintCanvas ) {
-            ({ ctx } = this._paintCanvas );
+            ({ ctx } = this._paintProps.paintCanvas );
             if ( selection ) {
                 ctx.save();
-                // note no offset is required when drawing on the full-size _paintCanvas
+                // note no offset is required when drawing on the full-size paintCanvas
                 clipContextToSelection( ctx, selection, 0, 0, this._invertSelection, overrides );
             }
         } else if ( selection ) {
@@ -343,9 +345,9 @@ export default class LayerRenderer extends ZoomableSprite {
         }
 
         if ( optAction ) {
-            if ( optAction.type === "stroke" ) {
+            if ( isStroking ) {
                 ctx.strokeStyle = optAction.color;
-                ctx.lineWidth   = ( optAction.size ?? 1 ) / this.canvas.documentScale;
+                ctx.lineWidth = optAction.size ?? 1;
                 ctx.stroke();
             }
             this.handleRelease( 0, 0 ); // supplied outside actions are instantly completed actions
@@ -373,7 +375,7 @@ export default class LayerRenderer extends ZoomableSprite {
             }
         }
         
-        if ( doSaveRestore ) {
+        if ( restoreContext ) {
             ctx.restore();
         }
 
@@ -391,19 +393,19 @@ export default class LayerRenderer extends ZoomableSprite {
      */
     preparePendingPaintState( preparePaintCanvas = false ): void {
         // nullish coalescing check because in lowMemory mode we have less states saved
-        this._orgSourceToStore = this._orgSourceToStore ?? cloneCanvas( this.getPaintSource() ); // must be sync (otherwise single click paints lead to race conditions)
+        this._paintProps.orgSource ??= cloneCanvas( this.getPaintSource() ); // must be sync (otherwise single click paints lead to race conditions)
         this._pixelArt = isPixelArt( this.canvas.getActiveDocument() );
 
         // drawing is handled on a temporary, drawable Canvas
         if ( preparePaintCanvas ) {
-            this._paintCanvas = this._paintCanvas || getDrawableCanvas( this.getPaintSize() );
-            setSmoothing( this._paintCanvas.cvs, this._pixelArt );
+            this._paintProps.paintCanvas ??= getDrawableCanvas( this.getPaintSize() );
+            setSmoothing( this._paintProps.paintCanvas.cvs, this._pixelArt );
         }
         this.debouncePaintStore();
     }
 
     debouncePaintStore( timeout: number = 5000 ): void {
-        this._pendingPaintState = setTimeout( this.storePaintState.bind( this ), timeout );
+        this._paintProps.pendingSave = setTimeout( this.storePaintState.bind( this ), timeout );
     }
 
     usePaintCanvas(): boolean {
@@ -414,7 +416,7 @@ export default class LayerRenderer extends ZoomableSprite {
     }
     
     isPainting(): boolean {
-        return !!this._paintCanvas; // while instance is declared, some painting operation is taking place
+        return !!this._paintProps.paintCanvas; // while instance is declared, some painting operation is taking place
     }
 
     getPaintSource(): HTMLCanvasElement {
@@ -435,19 +437,19 @@ export default class LayerRenderer extends ZoomableSprite {
     }
 
     async storePaintState(): Promise<boolean> {
-        if ( !this._pendingPaintState ) {
+        if ( !this._paintProps.pendingSave ) {
             return true;
         }
-        window.clearTimeout( this._pendingPaintState );
+        window.clearTimeout( this._paintProps.pendingSave );
         if ( this.isDrawing() ) {
             // still drawing, debounce again (layer.source only updated on handleRelease())
             this.debouncePaintStore( 1000 );
             return false;
         }
-        this._pendingPaintState = undefined;
+        this._paintProps.pendingSave = undefined;
 
-        const original = this._orgSourceToStore; // grab reference to avoid race conditions while creating Blobs during continued painting
-        this._orgSourceToStore = undefined;
+        const original = this._paintProps.orgSource; // grab reference to avoid race conditions while creating Blobs during continued painting
+        this._paintProps.orgSource = undefined;
 
         const newBlob  = await canvasToBlob( this.getPaintSource() );
         const newState = blobToResource( newBlob );
@@ -600,14 +602,14 @@ export default class LayerRenderer extends ZoomableSprite {
         if ( this.isPainting() ) {
             // commit the drawable canvas content onto the destination source
             renderDrawableCanvas(
-                this.getPaintSource().getContext( "2d" ), this.getPaintSize(), this.canvas, this._brush.options.opacity,
+                this._paintProps, this.getPaintSource().getContext( "2d" ), this.getPaintSize(), this.canvas, this._brush.options.opacity,
                 this._toolType === ToolTypes.ERASER ? "destination-out" : undefined, this.layer, this._pixelArt,
             );
             disposeMaskComposite();
             disposeDrawableCanvas();
             this.resetFilterAndRecache();
 
-            this._paintCanvas = null;
+            this._paintProps.paintCanvas = null;
             this._brush.down  = false;
             this._brush.last  = 0;
             this._brush.pointers.length = 0;
@@ -736,7 +738,7 @@ export default class LayerRenderer extends ZoomableSprite {
             if ( isErasing ) {
                 const maskedSource = cloneCanvas( isDrawingOnMask ? this._unmaskedBitmap : this._bitmap as HTMLCanvasElement );
                 renderDrawableCanvas(
-                    maskedSource.getContext( "2d" )!, this.getPaintSize(), this.canvas,
+                    this._paintProps, maskedSource.getContext( "2d" )!, this.getPaintSize(), this.canvas,
                     this._brush.options.opacity, "destination-out", this.layer, this._pixelArt,
                 );
                 if ( isDrawingOnMask ) {
@@ -763,7 +765,7 @@ export default class LayerRenderer extends ZoomableSprite {
                     clipLayer( documentContext, this.layer, this._bounds, viewport, false );
                 }
                 renderDrawableCanvas(
-                    isDrawingOnMask ? drawContext : documentContext, this.getPaintSize(), this.canvas, this._brush.options.opacity,
+                    this._paintProps, isDrawingOnMask ? drawContext : documentContext, this.getPaintSize(), this.canvas, this._brush.options.opacity,
                     this._toolType === ToolTypes.ERASER || isMaskable( this.layer, this.getStore() ) ? "destination-out" : undefined,
                 );
                 if ( isDrawingOnMask ) {

src/rendering/utils/drawable-canvas-utils.ts

@@ -22,7 +22,7 @@
  */
 import type { Point, Size } from "zcanvas";
 import type { Layer } from "@/model/types/layer";
-import type { CanvasContextPairing, Brush } from "@/definitions/editor";
+import type { Brush, CanvasContextPairing, PaintProps } from "@/definitions/editor";
 import { reverseTransformation } from "@/rendering/operations/transforming";
 import type ZoomableCanvas from "@/rendering/actors/zoomable-canvas";
 import { fastRound } from "@/math/unit-math";
@@ -62,7 +62,7 @@ export const getDrawableCanvas = ( size: Size ): CanvasContextPairing => {
  * effects of the Layer, to be used when committing the effects permanently when drawing has completed.
  */
 export const renderDrawableCanvas = (
-    destinationContext: CanvasRenderingContext2D, destinationSize: Size, zoomableCanvas: ZoomableCanvas,
+    paintProps: PaintProps, destinationContext: CanvasRenderingContext2D, destinationSize: Size, zoomableCanvas: ZoomableCanvas,
     alpha = 1, compositeOperation?: GlobalCompositeOperation, layer?: Layer, roundValues = false,
 ): void => {
     const source = drawableCanvas.cvs;
@@ -83,7 +83,7 @@ export const renderDrawableCanvas = (
         };
         reverseTransformation( destinationContext, layer );
     }
-    const viewport = offset ? zoomableCanvas.getViewport() : undefined;
+    const viewport = paintProps.useViewport && offset ? zoomableCanvas.getViewport() : undefined;
 
     destinationContext.globalAlpha = alpha;
     if ( compositeOperation !== undefined ) {
@@ -127,13 +127,16 @@ export const sliceBrushPointers = ( brush: Brush ): Point[] => {
  * Create override configuration for a render operation, wrapping its source input (e.g. pointers list) with scaling
  * and coordinate space of the drawableCanvas.
  */
-export const createOverrideConfig = ( zoomableCanvas: ZoomableCanvas, pointers: Point[] ): OverrideConfig => ({
-    scale : 1 / zoomableCanvas.documentScale,
-    zoom  : zoomableCanvas.zoomFactor,
-    vpX   : zoomableCanvas.getViewport().left,
-    vpY   : zoomableCanvas.getViewport().top,
-    pointers,
-});
+export const createOverrideConfig = ( zoomableCanvas: ZoomableCanvas, paintProps: PaintProps, pointers: Point[] ): OverrideConfig => {
+    const viewport = paintProps.useViewport ? zoomableCanvas.getViewport() : undefined;
+    return {
+        scale : 1 / zoomableCanvas.documentScale,
+        zoom  : zoomableCanvas.zoomFactor,
+        vpX   : viewport?.left ?? 0,
+        vpY   : viewport?.top ?? 0,
+        pointers,
+    };
+};
 
 /**
  * Apply a override configuration to use given pointers on a drawableCanvas. NOTE: This will mutate the original

tests/unit/mocks.ts

@@ -60,6 +60,8 @@ export function mockZCanvas() {
 export function createMockZoomableCanvas(): ZoomableCanvas {
     const canvas = createMockCanvasElement();
     return {
+        documentScale: 1,
+        zoomFactor: 1,
         draggingSprite: null,
         width: 300,
         height: 200,
@@ -69,7 +71,7 @@ export function createMockZoomableCanvas(): ZoomableCanvas {
         getActiveDocument: vi.fn(),
         getElement: vi.fn().mockReturnValue( canvas ),
         getStore: vi.fn(),
-        getViewport: vi.fn(() => ({ left: 0, top: 0, width: 300, height: 300 })),
+        getViewport: vi.fn().mockReturnValue({ left: 0, top: 0, width: 300, height: 300 }),
         refreshFn: vi.fn(),
         rescaleFn: vi.fn(),
         setDimensions: vi.fn(),

tests/unit/rendering/utils/drawable-canvas-utils.spec.ts

@@ -0,0 +1,69 @@
+import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "vitest";
+import type { Point } from "zcanvas";
+import { createMockZoomableCanvas, mockZCanvas } from "../../mocks";
+
+mockZCanvas();
+
+import { applyOverrideConfig, createOverrideConfig, } from "@/rendering/utils/drawable-canvas-utils";
+
+describe( "Drawable canvas utils", () => {
+    const zoomableCanvas = createMockZoomableCanvas();
+    let pointers: Point[];
+
+    beforeEach(() => {
+        zoomableCanvas.documentScale = 3;
+        zoomableCanvas.zoomFactor = 2;
+        ( zoomableCanvas.getViewport as Mock ).mockReturnValue({ left: 10, top: 5 });
+
+        pointers = [
+            { x: 0, y: 0 }, { x: 10, y: 0 }, { x: 10, y: 10 }, { x: 0, y: 10 }, { x: 0, y: 0 },
+        ];
+    });
+    
+    afterEach(() => {
+        vi.resetAllMocks();
+    });
+
+    describe( "When creating a draw override configuration", () => {
+        it( "should store the current Document scale and zoom factor", () => {
+            const { scale, zoom } = createOverrideConfig( zoomableCanvas, { useViewport: true }, pointers )
+            
+            expect( scale ).toEqual( 1 / zoomableCanvas.documentScale );
+            expect( zoom ).toEqual( zoomableCanvas.zoomFactor );
+        });
+
+        it( "should take the Viewport offset into account", () => {
+            const { vpX, vpY } = createOverrideConfig( zoomableCanvas, { useViewport: true }, pointers );
+
+            expect( vpX ).toEqual( 10 );
+            expect( vpY ).toEqual( 5 );
+        });
+
+        it( "should ignore the Viewport offset when requested", () => {
+            const { vpX, vpY } = createOverrideConfig( zoomableCanvas, { useViewport: false }, pointers );
+
+            expect( vpX ).toEqual( 0 );
+            expect( vpY ).toEqual( 0 );
+        });
+    });
+
+    describe( "When applying a draw override configuration", () => {
+        it( "should translate the pointer coordinates to reflect the Viewport position and Document scale in place", () => {
+            const overrideConfig = createOverrideConfig( zoomableCanvas, { useViewport: true }, pointers );
+            applyOverrideConfig( overrideConfig, pointers );
+
+            expect( pointers ).toEqual([
+                { x: -30, y: -15 }, { x: -20, y: -15 }, { x: -20, y: -5 }, { x: -30, y: -5 }, { x: -30, y: -15 }, 
+            ]);
+        });
+
+        it( "should not translate the pointer coordinates when the Viewport position should be ignored", () => {
+            const overrideConfig = createOverrideConfig( zoomableCanvas, { useViewport: false }, pointers );
+            applyOverrideConfig( overrideConfig, pointers );
+
+            expect( pointers ).toEqual([
+                { x: 0, y: 0 }, { x: 10, y: 0 }, { x: 10, y: 10 }, { x: 0, y: 10 }, { x: 0, y: 0 },
+            ]);
+        });
+    });
+});