diff --git a/src/components/ui/zcanvas/layer-sprite.js b/src/components/ui/zcanvas/layer-sprite.js index 65db5c2..2569bff 100644 --- a/src/components/ui/zcanvas/layer-sprite.js +++ b/src/components/ui/zcanvas/layer-sprite.js @@ -22,7 +22,7 @@ */ import Vue from "vue"; import { sprite } from "zcanvas" -import { createCanvas, cloneCanvas, resizeImage, globalToLocal } from "@/utils/canvas-util"; +import { createCanvas, canvasToBlob, resizeImage, globalToLocal } from "@/utils/canvas-util"; import { renderCross, renderBrushStroke, renderClonedStroke } from "@/utils/render-util"; import { LAYER_GRAPHIC, LAYER_MASK, LAYER_TEXT } from "@/definitions/layer-types"; import { scaleRectangle } from "@/math/image-math"; @@ -290,7 +290,9 @@ class LayerSprite extends sprite { * not delay to history state UI from updating more than necessary. */ preparePendingPaintState() { - this._orgSourceToStore = cloneCanvas( this.layer.source ); + canvasToBlob( this.layer.source ).then( blob => { + this._orgSourceToStore = URL.createObjectURL( blob ); + }); this._pendingPaintState = setTimeout( this.storePaintState.bind( this ), 5000 ); } @@ -302,14 +304,17 @@ class LayerSprite extends sprite { this._pendingPaintState = null; const layer = this.layer; const orgState = this._orgSourceToStore; - const newState = cloneCanvas( layer.source ); - enqueueState( `spritePaint_${layer.id}`, { - undo() { - restorePaintFromHistory( layer, orgState ); - }, - redo() { - restorePaintFromHistory( layer, newState); - } + canvasToBlob( layer.source ).then( blob => { + const newState = URL.createObjectURL( blob ); + enqueueState( `spritePaint_${layer.id}`, { + undo() { + restorePaintFromHistory( layer, orgState ); + }, + redo() { + restorePaintFromHistory( layer, newState); + }, + resources: [ orgState, newState ], + }); }); this._orgSourceToStore = null; } @@ -539,7 +544,13 @@ function positionSpriteFromHistory( layer, x, y ) { } } -function restorePaintFromHistory( layer, orgState ) { - layer.source = orgState; - getSpriteForLayer( layer )?.resetFilterAndRecache(); +function restorePaintFromHistory( layer, state ) { + const ctx = layer.source.getContext( "2d" ); + ctx.clearRect( 0, 0, layer.source.width, layer.source.height ); + const image = new Image(); + image.onload = () => { + ctx.drawImage( image, 0, 0 ); + getSpriteForLayer( layer )?.resetFilterAndRecache(); + }; + image.src = state; } diff --git a/src/factories/history-state-factory.js b/src/factories/history-state-factory.js index b3d4380..5e01072 100644 --- a/src/factories/history-state-factory.js +++ b/src/factories/history-state-factory.js @@ -41,12 +41,25 @@ export const flushQueue = () => { // this ensures that an enqueued state that is reverted within the ENQUEUE_TIMEOUT is restored export const forceProcess = processQueue; +/** + * Enqueue a state for addition in the history module. By enqueing, duplicate + * calls for the same key with new state Objects are merged into a single state. + * + * @param {String} key unique identifier for this state + * @param {{ undo: Function, redo: Function, resources: Array }} Object with + * undo and redo functions and optional list of resources (blob URLs) + * associated with the undo / redo actions. Blob URLs will be revoked when + * state is popped from the history stack to free memory. + */ export const enqueueState = ( key, undoRedoState ) => { // new state is for the same property as the previously enqueued state // we can discard the previously enqueued states.redo in favour of this more actual one if ( stateQueue.has( key )) { const existing = stateQueue.get( key ); existing.redo = undoRedoState.redo; + if ( existing.resources && undoRedoState.resources ) { + existing.resources.push( ...undoRedoState.resources ); + } return; } // there is an existing queue for a different property diff --git a/src/store/modules/history-module.js b/src/store/modules/history-module.js index a3508cd..fcf7439 100644 --- a/src/store/modules/history-module.js +++ b/src/store/modules/history-module.js @@ -23,19 +23,27 @@ import UndoManager from "undo-manager"; import { forceProcess, flushQueue } from "@/factories/history-state-factory"; -const STATES_TO_SAVE = 99; +export const STATES_TO_SAVE = 99; // a module to store states so the application can undo/redo changes -// made to the song. We do this by applying save and restore functions for -// individual changes made to a song. This is preferred over cloning -// entire song structures as this will consume a large amount of memory! +// made to a document. We do this by applying save and restore functions for +// individual changes made to a document. This is preferred over cloning +// entire document structures as this will consume a large amount of memory! // by using Vue.set() and Vue.delete() in the undo/redo functions reactivity // will be retained. const module = { state: { undoManager: new UndoManager(), - historyIndex: -1 // used for reactivity (as undo manager isn"t bound to Vue) + historyIndex: -1, // used for reactivity (as undo manager isn't bound to Vue) + // states can specify an optional list of Blob URLs associated with their undo/redo + // operation. When such a state is popped from the avilable undo stack (because + // new states have been added beyond the STATES_TO_SAVE limit), these Blob URLs + // are revoked to free up memory. + blobUrls: new Map(), + // the amount of states registered, this is not equal to the amount of states + // available for undo (which is capped at STATES_TO_SAVE). + stored: 0, }, getters: { canUndo( state ) { @@ -52,7 +60,7 @@ const module = { /** * Store a state change inside the history. */ - saveState( state, { undo, redo }) { + saveState( state, { undo, redo, resources = null }) { if ( process.env.NODE_ENV === "development" ) { if ( typeof undo !== "function" || typeof redo !== "function" ) { throw new Error( "cannot store a state without specifying valid undo and redo actions" ); @@ -60,6 +68,24 @@ const module = { } state.undoManager.add({ undo, redo }); state.historyIndex = state.undoManager.getIndex(); + ++state.stored; + + const storedIndex = state.stored; + + if ( storedIndex > STATES_TO_SAVE ) { + // the minimum index that should still be available in the undo stack + const minIndex = storedIndex - STATES_TO_SAVE; + [ ...state.blobUrls.entries()].forEach(([ index, urls ]) => { + if ( index < minIndex ) { + urls.forEach( url => window.URL.revokeObjectURL( url )); + state.blobUrls.delete( index ); + } + }); + } + + if ( Array.isArray( resources )) { + state.blobUrls.set( storedIndex, resources ); + } }, setHistoryIndex( state, value ) { state.historyIndex = value; @@ -71,6 +97,9 @@ const module = { flushQueue(); state.undoManager.clear(); state.historyIndex = -1; + state.stored = 0; + state.blobUrls.forEach( urlList => urlList.forEach( url => window.URL.revokeObjectURL( url ))); + state.blobUrls.clear(); } }, actions: { diff --git a/src/utils/resource-manager.js b/src/utils/resource-manager.js index 429df82..ac1df54 100644 --- a/src/utils/resource-manager.js +++ b/src/utils/resource-manager.js @@ -1,7 +1,7 @@ /** * The MIT License (MIT) * - * Igor Zinken 2020 - https://www.igorski.nl + * Igor Zinken 2020-2021 - https://www.igorski.nl * * Permission is hereby granted, free of charge, to any person obtaining a copy of * this software and associated documentation files (the "Software"), to deal in @@ -22,6 +22,9 @@ */ import { createCanvas, canvasToBlob } from "@/utils/canvas-util"; +// for debugging and spotting memory leaks, in Chrome you can access chrome://blob-internals/ +// to view all allocated object URLs + const { URL } = window; /** diff --git a/tests/unit/store/modules/history-module.spec.js b/tests/unit/store/modules/history-module.spec.js index 6e6f6f0..63b9a25 100644 --- a/tests/unit/store/modules/history-module.spec.js +++ b/tests/unit/store/modules/history-module.spec.js @@ -1,5 +1,5 @@ import UndoManager from "undo-manager"; -import store from "@/store/modules/history-module"; +import store, { STATES_TO_SAVE } from "@/store/modules/history-module"; const { getters, mutations, actions } = store; let mockUpdateFn; @@ -9,13 +9,22 @@ jest.mock( "@/factories/history-state-factory", () => ({ })); describe( "History State module", () => { - const noop = () => {}, AMOUNT_OF_STATES = 5; + const noop = () => {}; let commit = jest.fn(); let state; beforeEach( () => { - state = { undoManager: new UndoManager(), historyIndex: -1 }; - state.undoManager.setLimit(AMOUNT_OF_STATES); + state = { + undoManager: new UndoManager(), + historyIndex: -1, + blobUrls: new Map(), + stored: 0, + }; + state.undoManager.setLimit( STATES_TO_SAVE ); + }); + + afterAll(() => { + jest.clearAllMocks(); }); describe("getters", () => { @@ -37,63 +46,122 @@ describe( "History State module", () => { expect(getters.canRedo(state)).toBe(false); // expected no redo to be available after having redone all actions }); - it("should know the amount of states it has stored", async () => { + it("should know the amount of states it can restore", async () => { commit = (action, value) => state.historyIndex = value; expect(0).toEqual(getters.amountOfStates(state)); // expected no states to be present after construction - for ( let i = 0; i < AMOUNT_OF_STATES; ++i ) { + for ( let i = 0; i < STATES_TO_SAVE; ++i ) { mutations.saveState(state, { undo: noop, redo: noop }); expect(i + 1).toEqual(getters.amountOfStates(state)); // expected amount of states to increase when storing new states } - for ( let i = AMOUNT_OF_STATES - 1; i >= 0; --i ) { + for ( let i = STATES_TO_SAVE - 1; i >= 0; --i ) { await actions.undo({ state, commit, getters: { canUndo: state.undoManager.hasUndo() }}); expect(i).toEqual(getters.amountOfStates(state)); // expected amount of states to decrease when performing undo } - for ( let i = 0; i < AMOUNT_OF_STATES; ++i ) { + for ( let i = 0; i < STATES_TO_SAVE; ++i ) { await actions.redo({ state, commit, getters: { canRedo: state.undoManager.hasRedo() }}); expect(i + 1).toEqual(getters.amountOfStates(state)); // expected amount of states to increase when performing redo } }); - }); - describe("mutations", () => { - it("should be able to store a state and increment the history index", () => { - mutations.saveState(state, { undo: noop, redo: noop }); - expect(state.historyIndex).toEqual(0); - mutations.saveState(state, { undo: noop, redo: noop }); - expect(state.historyIndex).toEqual(1); - }); - - it("should not store more states than are allowed", () => { - for ( let i = 0; i < AMOUNT_OF_STATES * 2; ++i ) { + it( "should keep track of the total amount of states registered, even when popped from the stack", () => { + for ( let i = 0; i < STATES_TO_SAVE * 2; ++i ) { mutations.saveState(state, { undo: noop, redo: noop }); } - expect(getters.amountOfStates(state)).toEqual(AMOUNT_OF_STATES); // expected model to not have recorded more states than the defined maximum + expect( state.stored ).toEqual( STATES_TO_SAVE * 2 ); // total counts the cumulatives + expect( getters.amountOfStates( state )).toEqual( STATES_TO_SAVE ); // states never exceed the total + }); + }); + + describe( "mutations", () => { + describe( "when saving states", () => { + it( "should be able to store a state and increment the history index", () => { + mutations.saveState( state, { undo: noop, redo: noop }); + expect( state.historyIndex ).toEqual( 0 ); + mutations.saveState( state, { undo: noop, redo: noop }); + expect( state.historyIndex ).toEqual( 1 ); + }); + + it( "should not store more states than are allowed", () => { + for ( let i = 0; i < STATES_TO_SAVE * 2; ++i ) { + mutations.saveState( state, { undo: noop, redo: noop }); + } + // expected model to not have recorded more states than the defined maximum + expect( getters.amountOfStates( state )).toEqual( STATES_TO_SAVE ); + }); + + it( "should be able to store a list of optional Blob resources within a state", () => { + expect( state.blobUrls.size ).toEqual( 0 ); + + mutations.saveState( state, { undo: noop, redo: noop, resources: [ "foo" ] }); + mutations.saveState( state, { undo: noop, redo: noop }); + mutations.saveState( state, { undo: noop, redo: noop, resources: [ "bar", "baz" ]}); + + expect( state.blobUrls.size ).toEqual( 2 ); + + expect( state.blobUrls.get( 1 )).toEqual([ "foo" ]); + expect( state.blobUrls.get( 3 )).toEqual([ "bar", "baz" ]); + }); + + it( "should free memory associated with optional Blob resources of popped states", () => { + window.URL = { revokeObjectURL: jest.fn() }; + + // add enough states to exceed the limit to pop old states + for ( let i = 0; i < STATES_TO_SAVE + 2; ++i ) { + // only add resources to first two states + const resources = ( i < 2 ) ? [ `resource_${i}`, `resource__${i}` ] : null; + mutations.saveState( state, { undo: noop, redo: noop, resources }); + } + // assert the first state's resources have been freed while the second still exist + expect( state.blobUrls.size ).toEqual( 1 ); + expect( state.blobUrls.has( 2 )).toBe( true ); // index of second, still available state + expect( window.URL.revokeObjectURL ).toHaveBeenNthCalledWith( 1, "resource_0" ); + expect( window.URL.revokeObjectURL ).toHaveBeenNthCalledWith( 2, "resource__0" ); + + // push one more state to pop the second added state from the available undo stack + mutations.saveState( state, { undo: noop, redo: noop }); + + // assert the second state's resource have also been freed and no further + // resources are listed (as no other states were stored with associated resources) + expect( state.blobUrls.size ).toEqual( 0 ); + expect( state.blobUrls.has( 2 )).toBe( false ); + expect( window.URL.revokeObjectURL ).toHaveBeenNthCalledWith( 3, "resource_1" ); + expect( window.URL.revokeObjectURL ).toHaveBeenNthCalledWith( 4, "resource__1" ); + }); }); - it("should be able to set the history index", () => { + it( "should be able to set the history index", () => { mutations.setHistoryIndex(state, 2); expect(state.historyIndex).toEqual(2); }); - it("should be able to clear its history", () => { + it("should be able to clear its history and allocated state resources", () => { mockUpdateFn = jest.fn(); function shouldntRun() { - throw new Error("undo/redo callback should not have fired after clearing the undo history"); + throw new Error( "undo/redo callback should not have fired after clearing the undo history" ); } - mutations.saveState(state, { undo: shouldntRun, redo: shouldntRun }); - mutations.saveState(state, { undo: shouldntRun, redo: shouldntRun }); + mutations.saveState(state, { undo: shouldntRun, redo: shouldntRun, resources: [ "foo", "bar" ] }); + mutations.saveState(state, { undo: shouldntRun, redo: shouldntRun, resources: [ "baz" ] }); - mutations.resetHistory(state); + window.URL = { revokeObjectURL: jest.fn() }; - expect(mockUpdateFn).toHaveBeenCalledWith( "flushQueue" ); - expect(state.historyIndex).toEqual(-1); - expect(getters.canUndo(state)).toBe(false); // expected no undo to be available after flushing of history - expect(getters.canRedo(state)).toBe(false); // expected no redo to be available after flushing of history - expect(0).toEqual(getters.amountOfStates(state)); // expected no states to be present in history + mutations.resetHistory( state ); + + expect( mockUpdateFn ).toHaveBeenCalledWith( "flushQueue" ); + expect( state.historyIndex ).toEqual( -1 ); + expect( getters.canUndo( state )).toBe( false ); // expected no undo to be available after flushing of history + expect( getters.canRedo( state )).toBe( false ); // expected no redo to be available after flushing of history + expect( getters.amountOfStates( state )).toEqual( 0 ); // expected no states to be present in history + expect( state.stored ).toEqual( 0 ); + + expect( window.URL.revokeObjectURL ).toHaveBeenNthCalledWith( 1, "foo" ); + expect( window.URL.revokeObjectURL ).toHaveBeenNthCalledWith( 2, "bar" ); + expect( window.URL.revokeObjectURL ).toHaveBeenNthCalledWith( 3, "baz" ); + + expect( state.blobUrls.size ).toEqual( 0 ); }); });