Better use of filter isNeutral (#5129)

* done so far

* done so far

* added tests for isNeutral

* mini test for apply filter

* test lint

src/filters/base_filter.class.js

@@ -190,11 +190,31 @@ fabric.Image.filters.BaseFilter = fabric.util.createClass(/** @lends fabric.Imag
   },
 
   /**
-   * Intentionally left blank, to be overridden in custom filters
+   * Generic isNeutral implementation for one parameter based filters.
+   * Used only in image applyFilters to discard filters that will not have an effect
+   * on the image
+   * Other filters may need their own verison ( ColorMatrix, HueRotation, gamma, ComposedFilter )
    * @param {Object} options
    **/
   isNeutralState: function(/* options */) {
-    return false;
+    var main = this.mainParameter,
+        _class = fabric.Image.filters[this.type].prototype;
+    if (main) {
+      if (Array.isArray(_class[main])) {
+        for (var i = _class[main].length; i--;) {
+          if (this[main][i] !== _class[main][i]) {
+            return false;
+          }
+        }
+        return true;
+      }
+      else {
+        return _class[main] === this[main];
+      }
+    }
+    else {
+      return false;
+    }
   },
 
   /**
@@ -212,15 +232,11 @@ fabric.Image.filters.BaseFilter = fabric.util.createClass(/** @lends fabric.Imag
    */
   applyTo: function(options) {
     if (options.webgl) {
-      if (options.passes > 1 && this.isNeutralState(options)) {
-        // avoid doing something that we do not need
-        return;
-      }
       this._setupFrameBuffer(options);
       this.applyToWebGL(options);
       this._swapTextures(options);
     }
-    else if (!this.isNeutralState()) {
+    else {
       this.applyTo2d(options);
     }
   },

src/filters/colormatrix_filter.class.js

@@ -81,20 +81,6 @@
       this.matrix = this.matrix.slice(0);
     },
 
-    /**
-     * Intentionally left blank, to be overridden in custom filters
-     * @param {Object} options
-     **/
-    isNeutralState: function(/* options */) {
-      var _class = filters.ColorMatrix;
-      for (var i = 20; i--;) {
-        if (this.matrix[i] !== _class.prototype.matrix[i]) {
-          return false;
-        }
-      }
-      return true;
-    },
-
     /**
      * Apply the ColorMatrix operation to a Uint8Array representing the pixels of an image.
      *

src/filters/composed_filter.class.js

@@ -51,6 +51,10 @@
         subFilters: this.subFilters.map(function(filter) { return filter.toObject(); }),
       });
     },
+
+    isNeutralState: function() {
+      return !this.subFilters.some(function(filter) { return !filter.isNeutralState(); });
+    }
   });
 
   /**

src/filters/gamma_filter.class.js

@@ -57,6 +57,15 @@
      */
     mainParameter: 'gamma',
 
+    /**
+     * Constructor
+     * @param {Object} [options] Options object
+     */
+    initialize: function(options) {
+      this.gamma = [1, 1, 1];
+      filters.BaseFilter.prototype.initialize.call(this, options);
+    },
+
     /**
      * Apply the Gamma operation to a Uint8Array representing the pixels of an image.
      *

src/filters/grayscale_filter.class.js

@@ -131,6 +131,15 @@
       var mode = 1;
       gl.uniform1i(uniformLocations.uMode, mode);
     },
+
+    /**
+     * Grayscale filter isNeutralState implementation
+     * The filter is never neutral
+     * on the image
+     **/
+    isNeutralState: function() {
+      return false;
+    },
   });
 
   /**

src/filters/hue_rotation.class.js

@@ -64,6 +64,17 @@
       this.matrix[12] = cos + aThird * OneMinusCos;
     },
 
+    /**
+     * HueRotation isNeutralState implementation
+     * Used only in image applyFilters to discard filters that will not have an effect
+     * on the image
+     * @param {Object} options
+     **/
+    isNeutralState: function(options) {
+      this.calculateMatrix();
+      return filters.BaseFilter.prototype.isNeutralState.call(this, options);
+    },
+
     /**
      * Apply this filter to the input image data provided.
      *
@@ -79,7 +90,7 @@
      */
     applyTo: function(options) {
       this.calculateMatrix();
-      fabric.Image.filters.BaseFilter.prototype.applyTo.call(this, options);
+      filters.BaseFilter.prototype.applyTo.call(this, options);
     },
 
   });

src/filters/invert_filter.class.js

@@ -55,9 +55,6 @@
      * @param {ImageData} options.imageData The Uint8Array to be filtered.
      */
     applyTo2d: function(options) {
-      if (!this.invert) {
-        return;
-      }
       var imageData = options.imageData,
           data = imageData.data, i,
           len = data.length;
@@ -68,6 +65,16 @@
       }
     },
 
+    /**
+     * Invert filter isNeutralState implementation
+     * Used only in image applyFilters to discard filters that will not have an effect
+     * on the image
+     * @param {Object} options
+     **/
+    isNeutralState: function() {
+      return !this.invert;
+    },
+
     /**
      * Return WebGL uniform locations for this filter's shader.
      *

src/filters/pixelate_filter.class.js

@@ -61,9 +61,6 @@
      * @param {ImageData} options.imageData The Uint8ClampedArray to be filtered.
      */
     applyTo2d: function(options) {
-      if (this.blocksize === 1) {
-        return;
-      }
       var imageData = options.imageData,
           data = imageData.data,
           iLen = imageData.height,

src/filters/resize_filter.class.js

@@ -42,14 +42,14 @@
      * @param {Number} scaleX
      * @default
      */
-    scaleX: 0,
+    scaleX: 1,
 
     /**
      * Scale factor for resizing, y axis
      * @param {Number} scaleY
      * @default
      */
-    scaleY: 0,
+    scaleY: 1,
 
     /**
      * LanczosLobes parameter for lanczos filter, valid for resizeType lanczos
@@ -158,10 +158,6 @@
      */
     applyTo: function(options) {
       if (options.webgl) {
-        if (options.passes > 1 && this.isNeutralState(options)) {
-          // avoid doing something that we do not need
-          return;
-        }
         options.passes++;
         this.width = options.sourceWidth;
         this.horizontal = true;
@@ -186,15 +182,13 @@
         this._swapTextures(options);
         options.sourceHeight = options.destinationHeight;
       }
-      else if (!this.isNeutralState(options)) {
+      else {
         this.applyTo2d(options);
       }
     },
 
-    isNeutralState: function(options) {
-      var scaleX = options.scaleX || this.scaleX,
-          scaleY = options.scaleY || this.scaleY;
-      return scaleX === 1 && scaleY === 1;
+    isNeutralState: function() {
+      return this.scaleX === 1 && this.scaleY === 1;
     },
 
     lanczosCreate: function(lobes) {

src/shapes/image.class.js

@@ -397,8 +397,8 @@
         this._element = elementToFilter;
         this._filterScalingX = 1;
         this._filterScalingY = 1;
-        this._lastScaleX = 1;
-        this._lastScaleY = 1;
+        this._lastScaleX = scaleX;
+        this._lastScaleY = scaleY;
         return;
       }
       if (!fabric.filterBackend) {
@@ -429,7 +429,7 @@
     applyFilters: function(filters) {
 
       filters = filters || this.filters || [];
-      filters = filters.filter(function(filter) { return filter; });
+      filters = filters.filter(function(filter) { return filter && !filter.isNeutralState(); });
       if (this.group) {
         this.set('dirty', true);
       }

test/unit/image.js

@@ -662,6 +662,23 @@
     });
   });
 
+  QUnit.test('apply filters run isNeutralState implementation of filters', function(assert) {
+    var done = assert.async();
+    createImageObject(function(image) {
+      var run = false;
+      image.dirty = false;
+      var filter = new fabric.Image.filters.Brightness();
+      image.filters = [filter];
+      filter.isNeutralState = function() {
+        run = true;
+      };
+      assert.equal(run, false, 'isNeutralState did not run yet');
+      image.applyFilters();
+      assert.equal(run, true, 'isNeutralState did run');
+      done();
+    });
+  });
+
   QUnit.test('apply filters do not set the image dirty if not in group', function(assert) {
     var done = assert.async();
     createImageObject(function(image) {