From c8189b9b2ecaa05fe50416a2d2b7b87aca286f2a Mon Sep 17 00:00:00 2001 From: Igor Zinken Date: Tue, 22 Dec 2020 12:13:18 +0100 Subject: [PATCH] Fixed issue with document resizing, sprite functions can now be run for specific documents, added unit tets for sprite factory --- README.md | 10 +-- .../document-canvas/document-canvas.vue | 2 +- .../resize-document/resize-document.vue | 4 +- .../file-menu/export-image/export-image.vue | 4 +- src/components/ui/zcanvas/layer-sprite.js | 27 +++++- src/factories/sprite-factory.js | 15 ++-- src/store/modules/document-module.js | 6 +- src/utils/canvas-util.js | 10 ++- tests/unit/factories/sprite-factory.spec.js | 85 +++++++++++++++++++ .../store/modules/document-module.spec.js | 65 ++++++++++---- 10 files changed, 185 insertions(+), 43 deletions(-) create mode 100644 tests/unit/factories/sprite-factory.spec.js diff --git a/README.md b/README.md index ee9cb54..6ac1347 100644 --- a/README.md +++ b/README.md @@ -50,25 +50,19 @@ npm run lint # TODO / Roadmap * Something weird with best fit only fitting when window resize fires (also see weird non-cleared canvas areas) -* Mask cache invalidation on document resize/switch -* Layer and mask coordinates are not saved when saving a document? +* Layer bitmaps and masks must not be stored as Vue observables +* Layer and mask coordinates are not saved when saving / switching document? * Only draw visible content (hope to improve zoomed in performance) -* LayerSprite should show outline when brush is selected * Canvas clearRect() is not doing full width and height ? (might be related to drawable layer click color problem) -* Document resizing doesn't rescale sprites properly? * Layer view in options-panel: allow naming, repositioning, toggle visibility, opacity * Canvas util : store transparency of images * Restored base64 images should be treated as binary once more (see layer-factory) * scale logic should move from zoomable-canvas into zCanvas (as handleInteraction needs to transform offsets by zoom ratio, see LayerSprite!) * Zoom set original size isn't that accurate (check also on mobile views), needs calculateMaxScaling ? -* adjust scaling (on widescreen images scale in the width, rather than go for full height and zoomed out mode) -* Image position must be made persistent (now isn't on document switch) * Implement selections * Unload Blobs when images are no longer used in document (see sprite-factory disposeSprite, keep instance count of usages) * Load/save documents from/to Dropbox -* Use hand cursor when draggable * Text editing using Google fonts! -* Use paint brush cursor when painting * Add tools for layer rotation and scaling * Implement layer scale * Implement clone brush diff --git a/src/components/document-canvas/document-canvas.vue b/src/components/document-canvas/document-canvas.vue index 36c2ee5..dbd7639 100644 --- a/src/components/document-canvas/document-canvas.vue +++ b/src/components/document-canvas/document-canvas.vue @@ -48,7 +48,7 @@ import { MAX_ZOOM, calculateMaxScaling } from "@/definitions/tool-types"; import { scaleToRatio, scaleValue } from "@/utils/image-math"; import { getCanvasInstance, setCanvasInstance, - createSpriteForLayer, runSpriteFn, flushLayerSprites, flushCache, + createSpriteForLayer, flushLayerSprites, flushCache, } from "@/factories/sprite-factory"; /* internal non-reactive properties */ diff --git a/src/components/edit-menu/resize-document/resize-document.vue b/src/components/edit-menu/resize-document/resize-document.vue index fe1762e..fe7fa26 100644 --- a/src/components/edit-menu/resize-document/resize-document.vue +++ b/src/components/edit-menu/resize-document/resize-document.vue @@ -31,7 +31,7 @@ @@ -47,7 +47,7 @@
diff --git a/src/components/file-menu/export-image/export-image.vue b/src/components/file-menu/export-image/export-image.vue index 2ae2a84..0b9ba4e 100644 --- a/src/components/file-menu/export-image/export-image.vue +++ b/src/components/file-menu/export-image/export-image.vue @@ -79,7 +79,7 @@ import Slider from "@/components/ui/slider/slider"; import { mapSelectOptions } from "@/utils/search-select-util"; import { getSpriteForLayer, getCanvasInstance } from "@/factories/sprite-factory"; import { EXPORTABLE_FILE_TYPES, typeToExt, isCompressableFileType } from "@/definitions/image-types"; -import { createCanvas, resizeImage } from "@/utils/canvas-util"; +import { createCanvas, resizeToBase64 } from "@/utils/canvas-util"; import { saveBlobAsFile } from "@/utils/file-util"; import messages from "./messages.json"; @@ -127,7 +127,7 @@ export default { // zCanvas magnifies content by the pixel ratio for a crisper result, downscale // to actual dimensions of the document - const resizedImage = await resizeImage( + const resizedImage = await resizeToBase64( base64, width * ( window.devicePixelRatio || 1 ), height * ( window.devicePixelRatio || 1 ), diff --git a/src/components/ui/zcanvas/layer-sprite.js b/src/components/ui/zcanvas/layer-sprite.js index 8a4edc0..3b2830f 100644 --- a/src/components/ui/zcanvas/layer-sprite.js +++ b/src/components/ui/zcanvas/layer-sprite.js @@ -20,8 +20,8 @@ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -import { sprite } from "zcanvas"; -import { createCanvas } from "@/utils/canvas-util"; +import { sprite } from "zcanvas"; +import { createCanvas, resizeImage } from "@/utils/canvas-util"; import { LAYER_GRAPHIC, LAYER_MASK } from "@/definitions/layer-types"; import ToolTypes from "@/definitions/tool-types"; @@ -130,6 +130,24 @@ class LayerSprite extends sprite { } } + async resize( width, height ) { + const ratio = width / this.width; + + if ( this.layer.bitmap ) { + this.layer.bitmap = await resizeImage( + this.layer.bitmap, this._bounds.width, this._bounds.height, width, height + ); + } + if ( this.layer.mask ) { + this.layer.mask = await resizeImage( + this.layer.mask, this._bounds.width, this._bounds.height, width, height + ); + this.cacheMask(); + } + this.setBounds( this.getX() * ratio, this.getY() * ratio, width, height ); + this.invalidate(); + } + // cheap way to hook into zCanvas.handleMove() handler keep following the cursor in draw() forceDrag() { this.isDragging = true; @@ -178,6 +196,11 @@ class LayerSprite extends sprite { } } + invalidate() { + this._cacheMask = true; + super.invalidate(); + } + draw( documentContext ) { if ( !this.isMaskable() ) { // use base draw() logic when no mask is set diff --git a/src/factories/sprite-factory.js b/src/factories/sprite-factory.js index 14be9e4..738c345 100644 --- a/src/factories/sprite-factory.js +++ b/src/factories/sprite-factory.js @@ -23,7 +23,7 @@ import LayerSprite from "@/components/ui/zcanvas/layer-sprite"; import { LAYER_IMAGE, LAYER_GRAPHIC, LAYER_MASK } from "@/definitions/layer-types"; -let zCanvasInstance; // a non-Vue observable zCanvas instance +let zCanvasInstance = null; // a non-Vue observable zCanvas instance export const getCanvasInstance = () => zCanvasInstance; export const setCanvasInstance = zCanvas => zCanvasInstance = zCanvas; @@ -36,9 +36,15 @@ const spriteCache = new Map(); /** * Runs given fn on each Sprite in the cache + * You can also pass in the active document to only run operations on + * sprites rendering the layer of the current document */ -export const runSpriteFn = fn => { - spriteCache.forEach( fn ); +export const runSpriteFn = ( fn, optDocument ) => { + spriteCache.forEach(( sprite ) => { + if ( !optDocument || optDocument.layers.includes( sprite.layer )) { + fn( sprite ); + } + }); }; /** @@ -57,7 +63,7 @@ export const hasSpriteForLayer = ({ id }) => { return spriteCache.has( id ); }; -export const getSpriteForLayer = ({ id }) => spriteCache.get( id ); +export const getSpriteForLayer = ({ id }) => spriteCache.get( id ) || null; /** * Clears the entire cache and disposes all Sprites. @@ -92,7 +98,6 @@ export const createSpriteForLayer = ( zCanvasInstance, layer, isInteractive = fa /* internal methods */ function disposeSprite( sprite ) { - console.warn( "disposing sprite" ); sprite?.dispose(); // TODO: also free associated bitmap ? } diff --git a/src/store/modules/document-module.js b/src/store/modules/document-module.js index e3f3dc0..3356ee3 100644 --- a/src/store/modules/document-module.js +++ b/src/store/modules/document-module.js @@ -44,19 +44,17 @@ export default { mutations: { setActiveDocument( state, index ) { state.activeIndex = index; + runSpriteFn( sprite => sprite.invalidate(), state.documents[ index ] ); }, setActiveDocumentSize( state, { width, height }) { const document = state.documents[ state.activeIndex ]; - const ratio = width / document.width; document.width = width; document.height = height; document.layers?.forEach( layer => { layer.width = width; layer.height = height; }); - runSpriteFn( sprite => { - sprite.setBounds( sprite.getX() * ratio, sprite.getY() * ratio, width, height ); - }); + runSpriteFn( sprite => sprite.resize( width, height ), document ); }, addNewDocument( state, nameOrDocument ) { const document = typeof nameOrDocument === "object" ? nameOrDocument : DocumentFactory.create({ name: nameOrDocument }); diff --git a/src/utils/canvas-util.js b/src/utils/canvas-util.js index 1390ce7..93dd3cd 100644 --- a/src/utils/canvas-util.js +++ b/src/utils/canvas-util.js @@ -72,7 +72,8 @@ export const base64ToLayerImage = async( base64, type, width, height ) => { return null; }; -export const resizeImage = async ( image, srcWidth, srcHeight, targetWidth, targetHeight, mime, encoderOptions ) => { +export const resizeImage = async ( image, srcWidth, srcHeight, targetWidth, targetHeight ) => +{ if ( srcWidth === targetWidth && srcHeight === targetHeight ) { return image; } @@ -86,5 +87,10 @@ export const resizeImage = async ( image, srcWidth, srcHeight, targetWidth, targ ctx.drawImage( image, 0, 0, srcWidth, srcHeight, 0, 0, targetWidth, targetHeight ); - return cvs.toDataURL( mime, encoderOptions ); + return cvs; }; + +export const resizeToBase64 = async ( image, srcWidth, srcHeight, targetWidth, targetHeight, mime, encoderOptions ) => { + const cvs = await resizeImage( image, srcWidth, srcHeight, targetWidth, targetHeight ); + return cvs.toDataURL( mime, encoderOptions ); +} diff --git a/tests/unit/factories/sprite-factory.spec.js b/tests/unit/factories/sprite-factory.spec.js new file mode 100644 index 0000000..3ead64a --- /dev/null +++ b/tests/unit/factories/sprite-factory.spec.js @@ -0,0 +1,85 @@ +import { sprite } from "zcanvas"; +import { + getCanvasInstance, setCanvasInstance, + createSpriteForLayer, hasSpriteForLayer, getSpriteForLayer, flushLayerSprites, + flushCache +} from "@/factories/sprite-factory"; + +describe( "Sprite factory", () => { + describe( "when maintaining the active zCanvas instance", () => { + it( "should not have an instance by default", () => { + expect( getCanvasInstance() ).toBeNull(); + }); + + it( "should be able to set and retrieve the active zCanvas instance", () => { + const zCanvas = { name: "zcanvas" }; + setCanvasInstance( zCanvas ); + expect( getCanvasInstance() ).toEqual( zCanvas ); + }); + + it( "should be able to unset the active zCanvas instance", () => { + expect( getCanvasInstance() ).not.toBeNull(); // set in previous test + setCanvasInstance( null ); + expect( getCanvasInstance() ).toBeNull(); + }); + }); + + describe( "when lazily creating / caching sprites", () => { + let zCanvas, layer1sprite; + const layer1 = { id: "layer1", x: 0, y: 0, width: 10, height: 10 }; + const layer2 = { id: "layer2", x: 0, y: 0, width: 10, height: 10 }; + + beforeEach(() => { + zCanvas = { name: "zcanvas", addChild: jest.fn() }; + }); + + it( "should create a new Sprite on first request", () => { + layer1sprite = createSpriteForLayer( zCanvas, layer1 ); + expect( layer1sprite instanceof sprite ); + expect( zCanvas.addChild ).toHaveBeenCalledWith( layer1sprite ); + }); + + it( "should know when there is a Sprite cached for a layer", () => { + expect( hasSpriteForLayer( layer1 )).toBe( true ); + expect( hasSpriteForLayer( layer2 )).toBe( false ); + }); + + it( "should retrieve the cached Sprite on repeated invocation for the same layer", () => { + const newSprite = createSpriteForLayer( zCanvas, layer1 ); + expect( newSprite ).toEqual( layer1sprite ); + expect( zCanvas.addChild ).not.toHaveBeenCalledWith( layer1sprite ); + }); + + it( "should be able to retrieve the cached Sprite for a layer", () => { + expect( getSpriteForLayer( layer1 )).toEqual( layer1sprite ); + expect( getSpriteForLayer( layer2 )).toBeNull(); // no Sprite registered + }); + + it( "should be able to dispose all Sprites for a layer", () => { + jest.spyOn( layer1sprite, "dispose" ); + flushLayerSprites( layer1 ); + expect( getSpriteForLayer( layer1 )).toBeNull(); + expect( layer1sprite.dispose ).toHaveBeenCalled(); + }); + }); + + it( "should be able to flush its cache in its entierity", () => { + const zCanvas = { name: "zcanvas", addChild: jest.fn() }; + const layer1 = { id: "layer1", x: 0, y: 0, width: 10, height: 10 }; + const layer2 = { id: "layer2", x: 0, y: 0, width: 10, height: 10 }; + + const layer1sprite = createSpriteForLayer( zCanvas, layer1 ); + const layer2sprite = createSpriteForLayer( zCanvas, layer2 ); + + jest.spyOn( layer1sprite, "dispose" ); + jest.spyOn( layer2sprite, "dispose" ); + + flushCache(); + + expect( hasSpriteForLayer( layer1 )).toBe( false ); + expect( hasSpriteForLayer( layer2 )).toBe( false ); + + expect( layer1sprite.dispose ).toHaveBeenCalled(); + expect( layer2sprite.dispose ).toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/store/modules/document-module.spec.js b/tests/unit/store/modules/document-module.spec.js index e04e662..b1f3222 100644 --- a/tests/unit/store/modules/document-module.spec.js +++ b/tests/unit/store/modules/document-module.spec.js @@ -65,26 +65,57 @@ describe( "Vuex document module", () => { }); describe( "mutations", () => { - it( "should be able to set the active Document index", () => { - const state = { activeIndex: 0 }; - mutations.setActiveDocument( state, 2 ); - expect( state.activeIndex ).toEqual( 2 ); + describe( "when setting the active Document", () => { + it( "should be able to set the active Document index", () => { + const state = { + documents: [{ name: "foo" }, { name: "bar" }], + activeIndex: 0 + }; + mutations.setActiveDocument( state, 1 ); + expect( state.activeIndex ).toEqual( 1 ); + }); + + it( "should request the invalidate() method on each Sprite for the given Document", () => { + const state = { + documents: [{ name: "foo" }, { name: "bar" }], + activeIndex: 0 + }; + mockUpdateFn = jest.fn(); + mutations.setActiveDocument( state, 1 ); + expect( mockUpdateFn ).toHaveBeenCalledWith( "runSpriteFn", expect.any( Function ), state.documents[ 1 ]); + }); }); - it( "should be able to update the active Document size", () => { - const state = { - documents: [ + describe( "when setting the active Document size", () => { + it( "should be able to update the active Document size", () => { + const state = { + documents: [ + { name: "foo", width: 30, height: 30 }, + { name: "bar", width: 50, height: 50 } + ], + activeIndex : 1, + }; + const size = { width: 75, height: 40 }; + mutations.setActiveDocumentSize( state, size ); + expect( state.documents ).toEqual([ { name: "foo", width: 30, height: 30 }, - { name: "bar", width: 50, height: 50 } - ], - activeIndex : 1, - }; - const size = { width: 75, height: 40 }; - mutations.setActiveDocumentSize( state, size ); - expect( state.documents ).toEqual([ - { name: "foo", width: 30, height: 30 }, - { name: "bar", width: size.width, height: size.height }, - ]); + { name: "bar", width: size.width, height: size.height }, + ]); + }); + + it( "should request the invalidate() method on each Sprite for the given Document", () => { + const state = { + documents: [ + { name: "foo", width: 30, height: 30 }, + { name: "bar", width: 50, height: 50 } + ], + activeIndex : 1, + }; + const size = { width: 75, height: 40 }; + mockUpdateFn = jest.fn(); + mutations.setActiveDocumentSize( state, size ); + expect( mockUpdateFn ).toHaveBeenCalledWith( "runSpriteFn", expect.any( Function ), state.documents[ state.activeIndex ]); + }); }); it( "should be able to add a new Document to the list", () => {