add-clipPath-to-cache-checks (#5384)

* add-clipPath-to-cache-checks

* added comment to skip canvas

* added a test

* more test

* avoid infinite recursion

* avoid using objects if some render will fire

* avoid rendering at all

src/mixins/stateful.mixin.js

@@ -40,6 +40,11 @@
       }
       for (var i = 0, len = keys.length; i < len; i++) {
         key = keys[i];
+        // since clipPath is in the statefull cache list and the clipPath objects
+        // would be iterated as an object, this would lead to possible infinite recursion
+        if (key === 'canvas') {
+          continue;
+        }
         if (!_isEqual(origValue[key], currentValue[key])) {
           return false;
         }

src/shapes/object.class.js

@@ -595,7 +595,7 @@
       'top left width height scaleX scaleY flipX flipY originX originY transformMatrix ' +
       'stroke strokeWidth strokeDashArray strokeLineCap strokeDashOffset strokeLineJoin strokeMiterLimit ' +
       'angle opacity fill globalCompositeOperation shadow clipTo visible backgroundColor ' +
-      'skewX skewY fillRule paintFirst'
+      'skewX skewY fillRule paintFirst clipPath'
     ).split(' '),
 
     /**
@@ -607,7 +607,7 @@
      */
     cacheProperties: (
       'fill stroke strokeWidth strokeDashArray width height paintFirst' +
-      ' strokeLineCap strokeDashOffset strokeLineJoin strokeMiterLimit backgroundColor'
+      ' strokeLineCap strokeDashOffset strokeLineJoin strokeMiterLimit backgroundColor clipPath'
     ).split(' '),
 
     /**

src/util/lang_object.js

@@ -1,7 +1,10 @@
 (function() {
   /**
    * Copies all enumerable properties of one js object to another
+   * this does not and cannot compete with generic utils.
    * Does not clone or extend fabric.Object subclasses.
+   * This is mostly for internal use and has extra handling for fabricJS objects
+   * it skips the canvas property in deep cloning.
    * @memberOf fabric.util.object
    * @param {Object} destination Where to copy to
    * @param {Object} source Where to copy from
@@ -25,7 +28,10 @@
       }
       else if (source && typeof source === 'object') {
         for (var property in source) {
-          if (source.hasOwnProperty(property)) {
+          if (property === 'canvas') {
+            destination[property] = extend({ }, source[property]);
+          }
+          else if (source.hasOwnProperty(property)) {
             destination[property] = extend({ }, source[property], deep);
           }
         }

test/unit/object_clipPath.js

@@ -133,4 +133,51 @@
     assert.equal(new fabric.Color(canvas.contextContainer.strokeStyle).getAlpha(), 0, 'stroke style is reset');
     assert.equal(canvas.contextContainer.globalAlpha, 1, 'globalAlpha is reset');
   });
+
+  QUnit.test('clipPath caching detection', function(assert) {
+    var cObj = new fabric.Object();
+    var clipPath = new fabric.Object();
+    cObj.statefullCache = true;
+    cObj.saveState({ propertySet: 'cacheProperties' });
+    var change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, false, 'cache is clean');
+
+    cObj.clipPath = clipPath;
+    change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, true, 'cache is dirty');
+
+    cObj.saveState({ propertySet: 'cacheProperties' });
+
+    change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, false, 'cache is clean again');
+
+    cObj.clipPath.fill = 'red';
+    change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, true, 'cache change in clipPath is detected');
+  });
+
+  QUnit.test('clipPath caching detection with canvas object', function(assert) {
+    var canvas = new fabric.StaticCanvas(null, { renderOnAddRemove: false });
+    var cObj = new fabric.Rect();
+    var clipPath = new fabric.Rect();
+    canvas.add(cObj);
+    clipPath.canvas = canvas;
+    cObj.statefullCache = true;
+    cObj.saveState({ propertySet: 'cacheProperties' });
+    var change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, false, 'cache is clean - canvas');
+
+    cObj.clipPath = clipPath;
+    change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, true, 'cache is dirty - canvas');
+
+    cObj.saveState({ propertySet: 'cacheProperties' });
+
+    change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, false, 'cache is clean again - canvas');
+
+    cObj.clipPath.fill = 'red';
+    change = cObj.hasStateChanged('cacheProperties');
+    assert.equal(change, true, 'cache change in clipPath is detected - canvas');
+  });
 })();