From 83d8e7af4dae5b5da76dd7d2bb7e22414747576a Mon Sep 17 00:00:00 2001 From: Igor Zinken <730069+igorski@users.noreply.github.com> Date: Sat, 15 Mar 2025 10:19:27 +0100 Subject: [PATCH] Address issue where drawing operations on offset, cropped Layers would not use correct coordinate space --- src/definitions/tool-types.ts | 4 ++-- src/rendering/canvas-elements/interaction-pane.ts | 11 ++++++++--- src/rendering/canvas-elements/layer-sprite.ts | 3 +-- src/store/modules/document-module.ts | 4 ++-- tests/unit/definitions/tool-types.spec.ts | 6 +++--- tests/unit/store/modules/document-module.spec.ts | 4 ++-- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/definitions/tool-types.ts b/src/definitions/tool-types.ts index d501087..debee2d 100644 --- a/src/definitions/tool-types.ts +++ b/src/definitions/tool-types.ts @@ -52,9 +52,9 @@ export const usesInteractionPane = ( tool: ToolTypes ): boolean => PANE_TYPES.in export const SELECTION_TOOLS = [ ToolTypes.SELECTION, ToolTypes.LASSO, ToolTypes.WAND ]; -export const canDraw = ( activeDocument: Document, activeLayer: Layer, activeLayerMask: HTMLCanvasElement | null ): boolean => { +export const canDraw = ( activeDocument: Document, activeLayer: Layer, activeLayerMask?: HTMLCanvasElement ): boolean => { return activeDocument && - (( activeLayer?.mask !== null && activeLayer.mask === activeLayerMask ) || activeLayer?.type === LayerTypes.LAYER_GRAPHIC ); + (( !!activeLayer?.mask && activeLayer.mask === activeLayerMask ) || activeLayer?.type === LayerTypes.LAYER_GRAPHIC ); }; // we cannot draw in selection if a layer is mirrored (see https://github.com/igorski/bitmappery/issues/5) diff --git a/src/rendering/canvas-elements/interaction-pane.ts b/src/rendering/canvas-elements/interaction-pane.ts index 8ef14f4..270c564 100644 --- a/src/rendering/canvas-elements/interaction-pane.ts +++ b/src/rendering/canvas-elements/interaction-pane.ts @@ -396,9 +396,12 @@ class InteractionPane extends sprite { } draw( ctx: CanvasRenderingContext2D, viewport: Viewport ): void { + const document = this.getActiveDocument(); + if ( !document ) { + return; // pane was active prior to Document closing + } + let { activeSelection, invertSelection, width, height } = document; // render selection outline - let { invertSelection, width, height } = this.getActiveDocument(); - const { activeSelection } = this.getActiveDocument(); if ( /*this.mode === InteractionModes.MODE_SELECTION && */ activeSelection?.length > 0 ) { for ( let shape of activeSelection ) { const connectToPointer = shape === activeSelection.at( -1 ); @@ -523,7 +526,9 @@ function calculateSelectionSize( firstPoint: Point, destX: number, destY: number function syncSelection(): void { const { getters } = getCanvasInstance().store; - getSpriteForLayer( getters.activeLayer )?.setSelection( getters.activeDocument ); + if ( getters.activeLayer ) { + getSpriteForLayer( getters.activeLayer )?.setSelection( getters.activeDocument ); + } } function storeSelectionHistory( document: Document, optPreviousSelection: Selection = [], optType = "" ): void { diff --git a/src/rendering/canvas-elements/layer-sprite.ts b/src/rendering/canvas-elements/layer-sprite.ts index 877e669..fb330fb 100644 --- a/src/rendering/canvas-elements/layer-sprite.ts +++ b/src/rendering/canvas-elements/layer-sprite.ts @@ -418,8 +418,7 @@ export default class LayerSprite extends ZoomableSprite { } getPaintSize(): Size { - const source = this.getPaintSource(); - return { width: source.width, height: source.height }; + return this.canvas.getActiveDocument(); // always use the Document size to allow drawing on offset, cropped Layers /* // depending on zoom level, the interpolation when committing the drawableCanvas content onto the source // may benefit from a higher resolution when drawing on a zoomed in canvas. But maybe negligible and not worth memory overhead diff --git a/src/store/modules/document-module.ts b/src/store/modules/document-module.ts index 23fc1cd..13ce843 100644 --- a/src/store/modules/document-module.ts +++ b/src/store/modules/document-module.ts @@ -62,8 +62,8 @@ const DocumentModule: Module = { activeLayer: ( state: DocumentState, getters: any ): Layer => { return getters.layers?.[ state.activeLayerIndex ]; }, - activeLayerMask: ( state: DocumentState, getters: any ): HTMLCanvasElement | null => { - return ( state.maskActive && getters.activeLayer.mask ) || null; + activeLayerMask: ( state: DocumentState, getters: any ): HTMLCanvasElement | undefined => { + return ( state.maskActive && getters.activeLayer?.mask ) || undefined; }, activeLayerEffects: ( _: DocumentState, getters: any ): Effects => { return getters.activeLayer?.effects || {}; diff --git a/tests/unit/definitions/tool-types.spec.ts b/tests/unit/definitions/tool-types.spec.ts index 055b06f..5ffd2b3 100644 --- a/tests/unit/definitions/tool-types.spec.ts +++ b/tests/unit/definitions/tool-types.spec.ts @@ -22,14 +22,14 @@ describe( "tool types", () => { LayerTypes.LAYER_IMAGE, LayerTypes.LAYER_TEXT, ])( `should not consider a "%s"-layer to be drawable`, ( type: LayerTypes ) => { const layer = LayerFactory.create({ type }); - expect( canDraw( document, layer, null )).toBe( false ); + expect( canDraw( document, layer, undefined )).toBe( false ); }); it.each([ LayerTypes.LAYER_IMAGE, LayerTypes.LAYER_TEXT, ])( `should not consider a "%s"-layer with an unselected mask to be drawable`, ( type: LayerTypes ) => { const layer = LayerFactory.create({ type, mask: createMockCanvasElement() }); - expect( canDraw( document, layer, null )).toBe( false ); + expect( canDraw( document, layer, undefined )).toBe( false ); }); it.each([ @@ -41,7 +41,7 @@ describe( "tool types", () => { it( `should consider a "${LayerTypes.LAYER_GRAPHIC}"-layer to be drawable`, () => { const layer = LayerFactory.create({ type: LayerTypes.LAYER_GRAPHIC }); - expect( canDraw( document, layer, null )).toBe( true ); + expect( canDraw( document, layer, undefined )).toBe( true ); }); }); }); \ No newline at end of file diff --git a/tests/unit/store/modules/document-module.spec.ts b/tests/unit/store/modules/document-module.spec.ts index 9522943..70ac944 100644 --- a/tests/unit/store/modules/document-module.spec.ts +++ b/tests/unit/store/modules/document-module.spec.ts @@ -98,10 +98,10 @@ describe( "Vuex document module", () => { activeLayer: LayerFactory.create({ name: "layer1" }), }; // null because mask is not active - expect( getters.activeLayerMask( state, mockedGetters, {}, {} )).toBeNull(); + expect( getters.activeLayerMask( state, mockedGetters, {}, {} )).toBeUndefined(); state.maskActive = true; // null because layer has no mask drawable - expect( getters.activeLayerMask( state, mockedGetters, {}, {} )).toBeNull(); + expect( getters.activeLayerMask( state, mockedGetters, {}, {} )).toBeUndefined(); mockedGetters.activeLayer.mask = createMockCanvasElement(); expect( getters.activeLayerMask( state, mockedGetters, {}, {} )).toEqual( mockedGetters.activeLayer.mask ); });