fix: keep descendant caches on clearCache() and release the previous canvases on re-cache

clearCache() on any container wiped the canvas cache of every descendant,
even when the container itself was never cached, leaving filtered
children unfiltered and their canvases unreleased. Re-caching a node also
dropped its previous scene, filter and hit canvases without releasing
them. The canvas cache now lives in its own field instead of the generic
cache map, and one _releaseCanvasCache() serves clearCache() and cache().

Author: lavrton

src/Node.ts

@@ -167,7 +167,6 @@ export type NodeConfig = {
 // CONSTANTS
 const ABSOLUTE_OPACITY = 'absoluteOpacity',
   ABSOLUTE_TRANSFORM = 'absoluteTransform',
-  CANVAS = 'canvas',
   CHANGE = 'Change',
   CHILDREN = 'children',
   KONVA = 'konva',
@@ -274,6 +273,15 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
   index = 0;
   parent: Container | null = null;
   _cache: Map<string, any> = new Map<string, any>();
+  // canvases of cache(), see _getCanvasCache()
+  _canvasCache: {
+    scene: SceneCanvas;
+    filter: SceneCanvas;
+    hit: HitCanvas | null;
+    hitConfig: { pixelRatio: number; width: number; height: number };
+    x: number;
+    y: number;
+  } | null = null;
   _attachedDepsListeners: Map<string, boolean> = new Map<string, boolean>();
   _lastPos: Vector2d | null = null;
   _attrsAffectingSize!: string[];
@@ -317,6 +325,18 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
       this._cache.clear();
     }
   }
+  // drop the cached canvases and give their memory back
+  _releaseCanvasCache() {
+    const cache = this._canvasCache;
+    if (cache) {
+      Util.releaseCanvas(
+        cache.scene._canvas,
+        cache.filter._canvas,
+        ...(cache.hit ? [cache.hit._canvas] : [])
+      );
+      this._canvasCache = null;
+    }
+  }
   _getCache(attr: string, privateGetter: Function) {
     let cache = this._cache.get(attr);
 
@@ -350,7 +370,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
   }
 
   _getCanvasCache() {
-    return this._cache.get(CANVAS);
+    return this._canvasCache;
   }
   /*
    * when the logic for a cached result depends on ancestor propagation, use this
@@ -386,16 +406,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
    * node.clearCache();
    */
   clearCache() {
-    if (this._cache.has(CANVAS)) {
-      const { scene, filter, hit } = this._cache.get(CANVAS);
-      Util.releaseCanvas(
-        scene._canvas,
-        filter._canvas,
-        ...(hit ? [hit._canvas] : [])
-      );
-      this._cache.delete(CANVAS);
-    }
-
+    this._releaseCanvasCache();
     this._clearSelfAndDescendantCache();
     this._requestDraw();
     return this;
@@ -522,7 +533,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
 
     cachedSceneCanvas.isCache = true;
 
-    this._cache.delete(CANVAS);
+    this._releaseCanvasCache();
     this._filterUpToDate = false;
 
     if (conf.imageSmoothingEnabled === false) {
@@ -558,7 +569,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
     // the buffer is only needed while drawing
     Util.releaseCanvas(bufferCanvas._canvas);
 
-    this._cache.set(CANVAS, {
+    this._canvasCache = {
       scene: cachedSceneCanvas,
       filter: cachedFilterCanvas,
       // the hit canvas is built on demand (see _getCachedHitCanvas)
@@ -570,7 +581,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
       },
       x: x,
       y: y,
-    });
+    };
 
     this._requestDraw();
 
@@ -611,7 +622,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
    * @returns {Boolean}
    */
   isCached() {
-    return this._cache.has(CANVAS);
+    return !!this._canvasCache;
   }
 
   abstract drawScene(canvas?: Canvas, top?: Node, bufferCanvas?: Canvas): void;
@@ -697,7 +708,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
     context._applyOpacity(this);
     context._applyGlobalCompositeOperation(this);
 
-    const canvasCache = this._getCanvasCache();
+    const canvasCache = this._getCanvasCache()!;
     context.translate(canvasCache.x, canvasCache.y);
 
     const cacheCanvas = this._getCachedSceneCanvas();
@@ -713,7 +724,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
     context.restore();
   }
   _drawCachedHitCanvas(context: Context, hitCanvas: HitCanvas) {
-    const canvasCache = this._getCanvasCache();
+    const canvasCache = this._getCanvasCache()!;
     context.save();
     context.translate(canvasCache.x, canvasCache.y);
     context.drawImage(
@@ -727,7 +738,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
   }
   _getCachedSceneCanvas() {
     let filters = this.filters(),
-      cachedCanvas = this._getCanvasCache(),
+      cachedCanvas = this._getCanvasCache()!,
       sceneCanvas = cachedCanvas.scene as Canvas,
       filterCanvas = cachedCanvas.filter as Canvas,
       filterContext = filterCanvas.getContext(),

test/unit/Node-cache-test.ts

@@ -9,6 +9,7 @@ import {
   createCanvasAndContext,
   loadImage,
   getPixelRatio,
+  collectCanvasAllocations,
 } from './test-utils.ts';
 
 describe('Caching', function () {
@@ -820,7 +821,7 @@ describe('Caching', function () {
     group.add(circle);
     group.cache();
 
-    const canvas = group._cache.get('canvas').scene;
+    const canvas = group._getCanvasCache().scene;
     assert.equal(canvas.width, 106 * canvas.pixelRatio);
   });
 
@@ -1279,13 +1280,18 @@ describe('Caching', function () {
     maskgroup.add(mask);
 
     maskgroup.cache();
-    var canvasBefore = maskgroup._cache.get('canvas').scene._canvas;
+    // re-caching releases the previous canvas, so keep a copy of it
+    var canvasBefore = maskgroup._getCanvasCache().scene._canvas;
+    const copyBefore = Konva.Util.createCanvasElement();
+    copyBefore.width = canvasBefore.width;
+    copyBefore.height = canvasBefore.height;
+    copyBefore.getContext('2d')!.drawImage(canvasBefore, 0, 0);
 
     maskgroup.globalCompositeOperation('destination-in');
     maskgroup.cache();
-    var canvasAfter = maskgroup._cache.get('canvas').scene._canvas;
+    var canvasAfter = maskgroup._getCanvasCache().scene._canvas;
 
-    compareCanvases(canvasBefore, canvasAfter);
+    compareCanvases(copyBefore, canvasAfter);
 
     maskgroup.clearCache();
 
@@ -1395,7 +1401,7 @@ describe('Caching', function () {
 
     layer.draw();
     assert.equal(
-      bigCircle._cache.get('canvas').scene.getContext()._context
+      bigCircle._getCanvasCache().scene.getContext()._context
         .imageSmoothingEnabled,
       false
     );
@@ -1449,16 +1455,16 @@ describe('Caching', function () {
     layer.add(circle);
     circle.cache();
 
-    assert.equal(circle._cache.get('canvas').filter.width, 0);
+    assert.equal(circle._getCanvasCache().filter.width, 0);
     circle.filters([Konva.Filters.Blur]);
     layer.draw();
     assert.equal(
-      circle._cache.get('canvas').filter.width,
-      20 * circle._cache.get('canvas').filter.pixelRatio
+      circle._getCanvasCache().filter.width,
+      20 * circle._getCanvasCache().filter.pixelRatio
     );
     circle.filters([]);
     // TODO: should we clear cache canvas?
-    // assert.equal(circle._cache.get('canvas').filter.width, 0);
+    // assert.equal(circle._getCanvasCache().filter.width, 0);
   });
 
   it('hit from cache + global composite', function (done) {
@@ -1516,7 +1522,7 @@ describe('Caching', function () {
     });
     layer.draw();
 
-    var hitCanvas = rect._cache.get('canvas').hit;
+    var hitCanvas = rect._getCanvasCache().hit;
     assert.equal(hitCanvas._canvas.width, rect.width() * 0.2);
     assert.equal(hitCanvas._canvas.height, rect.height() * 0.2);
     assert.equal(hitCanvas.pixelRatio, 0.2);
@@ -1710,27 +1716,6 @@ describe('Caching', function () {
     assert.equal(stage.getIntersection({ x: 20, y: 20 }), null);
   });
 
-  // records the bitmap size of every canvas allocation made while fn runs
-  function collectCanvasAllocations(fn: () => void) {
-    const allocations: Array<{ canvas: any; width: number; height: number }> =
-      [];
-    const originalSetSize = Konva.Canvas.prototype.setSize;
-    Konva.Canvas.prototype.setSize = function (width, height) {
-      originalSetSize.call(this, width, height);
-      allocations.push({
-        canvas: this,
-        width: this.width,
-        height: this.height,
-      });
-    };
-    try {
-      fn();
-    } finally {
-      Konva.Canvas.prototype.setSize = originalSetSize;
-    }
-    return allocations.filter(({ width }) => width > 0);
-  }
-
   it('cache() allocates only the cache canvas when no shape needs the buffer', function () {
     // geometry far from the node's own origin used to inflate the buffer
     // canvas by the offset (5100x5100 for a 102x102 cache)
@@ -1807,4 +1792,34 @@ describe('Caching', function () {
     rect.position({ x: 0, y: 0 });
     compareCanvases(exported, rect.toCanvas(), 10);
   });
+
+  it('clearCache() on a parent keeps the caches of its children', function () {
+    var stage = addStage();
+    var layer = new Konva.Layer();
+    var group = new Konva.Group();
+    var rect = new Konva.Rect({
+      width: 50,
+      height: 50,
+      fill: 'red',
+      filters: [Konva.Filters.Invert],
+    });
+    group.add(rect);
+    layer.add(group);
+    stage.add(layer);
+    rect.cache();
+
+    group.clearCache();
+
+    assert.equal(rect.isCached(), true);
+  });
+
+  it('cache() releases the canvases of the previous cache', function () {
+    var rect = new Konva.Rect({ width: 50, height: 50, fill: 'red' });
+    rect.cache();
+    const previous = rect._getCanvasCache().scene;
+
+    rect.cache();
+
+    assert.equal(previous._canvas.width, 0, 'previous scene canvas released');
+  });
 });

test/unit/test-utils.ts

@@ -169,6 +169,22 @@ export function compareLayers(layer1: Layer, layer2: Layer, tol?, secondTol?) {
   );
 }
 
+// records the bitmap size of every canvas allocation made while fn runs
+export function collectCanvasAllocations(fn: () => void) {
+  const allocations: Array<{ canvas: any; width: number; height: number }> = [];
+  const originalSetSize = Konva.Canvas.prototype.setSize;
+  Konva.Canvas.prototype.setSize = function (width, height) {
+    originalSetSize.call(this, width, height);
+    allocations.push({ canvas: this, width: this.width, height: this.height });
+  };
+  try {
+    fn();
+  } finally {
+    Konva.Canvas.prototype.setSize = originalSetSize;
+  }
+  return allocations.filter(({ width }) => width > 0);
+}
+
 export function createCanvasAndContext() {
   const canvas = Konva.Util.createCanvasElement();
   canvas.width = 578 * Konva.pixelRatio;
@@ -405,8 +421,8 @@ export const assertAlmostEqual = function (val1, val2, tol = 0.000001) {
   }
 };
 
-export const assertAlmostDeepEqual = function (obj1, obj2) {
+export const assertAlmostDeepEqual = function (obj1, obj2, tol?) {
   for (var key1 in obj1) {
-    assertAlmostEqual(obj1[key1], obj2[key1]);
+    assertAlmostEqual(obj1[key1], obj2[key1], tol);
   }
 };