From 5ee89354a6efd7df0a7aec20c3b85218c6698dcd Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sun, 5 Aug 2018 11:25:53 +0200 Subject: [PATCH] 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 | 30 ++++-- src/filters/colormatrix_filter.class.js | 14 --- src/filters/composed_filter.class.js | 4 + src/filters/gamma_filter.class.js | 9 ++ src/filters/grayscale_filter.class.js | 9 ++ src/filters/hue_rotation.class.js | 13 ++- src/filters/invert_filter.class.js | 13 ++- src/filters/pixelate_filter.class.js | 3 - src/filters/resize_filter.class.js | 16 +-- src/shapes/image.class.js | 6 +- test/unit/image.js | 17 +++ test/unit/image_filters.js | 135 +++++++++++++++++++++--- 12 files changed, 212 insertions(+), 57 deletions(-) diff --git a/src/filters/base_filter.class.js b/src/filters/base_filter.class.js index f9e60a1dcec..10281394911 100644 --- a/src/filters/base_filter.class.js +++ b/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); } }, diff --git a/src/filters/colormatrix_filter.class.js b/src/filters/colormatrix_filter.class.js index d65d6c461d0..a3295c35c57 100644 --- a/src/filters/colormatrix_filter.class.js +++ b/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. * diff --git a/src/filters/composed_filter.class.js b/src/filters/composed_filter.class.js index c1f8097a514..c03d962aa4d 100644 --- a/src/filters/composed_filter.class.js +++ b/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(); }); + } }); /** diff --git a/src/filters/gamma_filter.class.js b/src/filters/gamma_filter.class.js index 2ac7f495d80..bf5f2366aa7 100644 --- a/src/filters/gamma_filter.class.js +++ b/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. * diff --git a/src/filters/grayscale_filter.class.js b/src/filters/grayscale_filter.class.js index 2f16637becd..414b96d880f 100644 --- a/src/filters/grayscale_filter.class.js +++ b/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; + }, }); /** diff --git a/src/filters/hue_rotation.class.js b/src/filters/hue_rotation.class.js index 01053be5c33..5c3e6ec4dc4 100644 --- a/src/filters/hue_rotation.class.js +++ b/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); }, }); diff --git a/src/filters/invert_filter.class.js b/src/filters/invert_filter.class.js index fe6d2346bae..6490b2248d1 100644 --- a/src/filters/invert_filter.class.js +++ b/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. * diff --git a/src/filters/pixelate_filter.class.js b/src/filters/pixelate_filter.class.js index a9c7dbab612..ee7d04bf8c7 100644 --- a/src/filters/pixelate_filter.class.js +++ b/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, diff --git a/src/filters/resize_filter.class.js b/src/filters/resize_filter.class.js index f494c0e33bb..6fd6aaa7be5 100644 --- a/src/filters/resize_filter.class.js +++ b/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) { diff --git a/src/shapes/image.class.js b/src/shapes/image.class.js index c725bdcd8e1..cc56c1362b1 100644 --- a/src/shapes/image.class.js +++ b/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); } diff --git a/test/unit/image.js b/test/unit/image.js index 9e7f8af42df..5751944d3d0 100644 --- a/test/unit/image.js +++ b/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) { diff --git a/test/unit/image_filters.js b/test/unit/image_filters.js index 0265b5e5003..8869e257f98 100644 --- a/test/unit/image_filters.js +++ b/test/unit/image_filters.js @@ -165,6 +165,14 @@ assert.deepEqual(fabric.Image.filters.Brightness.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Brightness(); + + assert.ok(filter.isNeutralState(), 'Is neutral when brightness is 0'); + filter.brightness = 0.15; + assert.notOk(filter.isNeutralState(), 'Is not neutral when brightness change'); + }); + QUnit.module('fabric.Image.filters.Composed'); QUnit.test('constructor', function(assert) { @@ -238,6 +246,18 @@ assert.ok(newFilter.subFilters[1] instanceof fabric.Image.filters.Contrast, 'should inherit from fabric.Image.filters.Contrast'); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Composed(); + var brightness = new fabric.Image.filters.Brightness(); + var contrast = new fabric.Image.filters.Contrast(); + filter.subFilters.push(brightness); + filter.subFilters.push(contrast); + + assert.ok(filter.isNeutralState(), 'Is neutral when all filters are neutral'); + filter.subFilters[0].brightness = 0.15; + assert.notOk(filter.isNeutralState(), 'Is not neutral when one subfilter changes'); + }); + QUnit.module('fabric.Image.filters.ColorMatrix'); @@ -338,6 +358,19 @@ assert.deepEqual(fabric.Image.filters.ColorMatrix.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.ColorMatrix(); + + assert.ok(filter.isNeutralState(), 'Is neutral when matrix is identity'); + filter.matrix = [ + 0, 1, 0, 0, 0.2, + 0, 0, 1, 0, 0.1, + 1, 0, 0, 0, 0.3, + 0, 0, 0, 1, 0 + ]; + assert.notOk(filter.isNeutralState(), 'Is not neutral when matrix changes'); + }); + QUnit.module('fabric.Image.filters.HueRotation'); QUnit.test('constructor', function(assert) { @@ -409,6 +442,14 @@ assert.deepEqual(fabric.Image.filters.HueRotation.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.HueRotation(); + + assert.ok(filter.isNeutralState(), 'Is neutral when rotation is 0'); + filter.rotation = 0.6; + assert.notOk(filter.isNeutralState(), 'Is not neutral when rotation changes'); + }); + QUnit.module('fabric.Image.filters.Contrast'); QUnit.test('constructor', function(assert) { @@ -478,6 +519,14 @@ assert.deepEqual(fabric.Image.filters.Contrast.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Contrast(); + + assert.ok(filter.isNeutralState(), 'Is neutral when contrast is 0'); + filter.contrast = 0.6; + assert.notOk(filter.isNeutralState(), 'Is not neutral when contrast changes'); + }); + QUnit.module('fabric.Image.filters.Saturation'); QUnit.test('constructor', function(assert) { @@ -547,6 +596,14 @@ assert.deepEqual(fabric.Image.filters.Saturation.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Saturation(); + + assert.ok(filter.isNeutralState(), 'Is neutral when saturation is 0'); + filter.saturation = 0.6; + assert.notOk(filter.isNeutralState(), 'Is not neutral when saturation changes'); + }); + QUnit.module('fabric.Image.filters.Gamma'); QUnit.test('constructor', function(assert) { @@ -616,6 +673,14 @@ assert.deepEqual(fabric.Image.filters.Gamma.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Gamma(); + + assert.ok(filter.isNeutralState(), 'Is neutral when gamma is 1'); + filter.gamma = [1.5, 1.5, 1.5]; + assert.notOk(filter.isNeutralState(), 'Is not neutral when gamma changes'); + }); + QUnit.module('fabric.Image.filters.Convolute'); QUnit.test('constructor', function(assert) { @@ -666,6 +731,11 @@ assert.deepEqual(fabric.Image.filters.Convolute.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Convolute(); + assert.notOk(filter.isNeutralState(), 'Is not neutral'); + }); + QUnit.module('fabric.Image.filters.Grayscale'); QUnit.test('constructor', function(assert) { @@ -742,6 +812,10 @@ assert.deepEqual(fabric.Image.filters.Grayscale.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Grayscale(); + assert.notOk(filter.isNeutralState(), 'Is never neutral'); + }); QUnit.module('fabric.Image.filters.Invert'); @@ -798,6 +872,12 @@ assert.deepEqual(fabric.Image.filters.Invert.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Invert(); + assert.notOk(filter.isNeutralState(), 'Is not neutral when default'); + filter.invert = false; + assert.ok(filter.isNeutralState(), 'Is not neutral when default'); + }); QUnit.module('fabric.Image.filters.Noise'); @@ -857,6 +937,12 @@ assert.deepEqual(fabric.Image.filters.Noise.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Noise(); + assert.ok(filter.isNeutralState(), 'Is neutral when noise is 0'); + filter.noise = 1; + assert.notOk(filter.isNeutralState(), 'Is no neutral when noise change'); + }); QUnit.module('fabric.Image.filters.Pixelate'); @@ -917,6 +1003,13 @@ assert.deepEqual(fabric.Image.filters.Pixelate.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Pixelate(); + filter.blocksize = 1; + assert.ok(filter.isNeutralState(), 'Is neutral when blockSize is 1'); + filter.blocksize = 4; + assert.notOk(filter.isNeutralState(), 'Is no neutral when blockSize change'); + }); QUnit.module('fabric.Image.filters.RemoveColor'); @@ -980,6 +1073,11 @@ assert.deepEqual(fabric.Image.filters.RemoveColor.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.RemoveColor(); + assert.notOk(filter.isNeutralState(), 'Is never neutral'); + }); + QUnit.module('fabric.Image.filters.Sepia'); QUnit.test('constructor', function(assert) { @@ -1024,6 +1122,11 @@ assert.deepEqual(fabric.Image.filters.Sepia.fromObject(object), filter); }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Sepia(); + assert.notOk(filter.isNeutralState(), 'Is never neutral'); + }); + QUnit.module('fabric.Image.filters.Resize'); QUnit.test('constructor', function(assert) { @@ -1040,8 +1143,8 @@ assert.equal(filter.resizeType, 'hermite'); assert.equal(filter.lanczosLobes, 3); - assert.equal(filter.scaleX, 0); - assert.equal(filter.scaleY, 0); + assert.equal(filter.scaleX, 1); + assert.equal(filter.scaleY, 1); var filter2 = new fabric.Image.filters.Resize({resizeType: 'bilinear', scaleX: 0.3, scaleY: 0.3}); assert.equal(filter2.resizeType, 'bilinear'); @@ -1060,11 +1163,11 @@ assert.ok(typeof filter.toObject === 'function'); var object = filter.toObject(); - assert.equal(JSON.stringify(object), '{"type":"Resize","scaleX":0,"scaleY":0,"resizeType":"hermite","lanczosLobes":3}'); + assert.equal(JSON.stringify(object), '{"type":"Resize","scaleX":1,"scaleY":1,"resizeType":"hermite","lanczosLobes":3}'); filter.resizeType = 'bilinear'; object = filter.toObject(); - assert.equal(JSON.stringify(object), '{"type":"Resize","scaleX":0,"scaleY":0,"resizeType":"bilinear","lanczosLobes":3}'); + assert.equal(JSON.stringify(object), '{"type":"Resize","scaleX":1,"scaleY":1,"resizeType":"bilinear","lanczosLobes":3}'); }); QUnit.test('fromObject', function(assert) { @@ -1079,18 +1182,20 @@ filter.scaleY = 0.8; assert.deepEqual(fabric.Image.filters.Resize.fromObject(filter.toObject()), filter); }); - // QUnit.test('fromObject', function(assert) { - // var done = assert.async(); - // createImageObject(function(image) { - // var filter = new fabric.Image.filters.Mask({mask: image}); - // var object = filter.toObject(); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Resize(); + assert.ok(filter.isNeutralState(), 'If scale is 1'); + filter.scaleX = 1.4; + assert.notOk(filter.isNeutralState(), 'Is not neutral when gamma changes'); + }); - // fabric.Image.filters.Mask.fromObject(object, function(filterObj) { - // assert.deepEqual(filterObj, filter); + QUnit.module('fabric.Image.filters.Blur'); - // done(); - // }); - // }); - // }); + QUnit.test('isNeutralState', function(assert) { + var filter = new fabric.Image.filters.Blur(); + assert.ok(filter.isNeutralState(), 'Is neutral when blur is 0'); + filter.blur = 0.3; + assert.notOk(filter.isNeutralState(), 'Is not neutral when blur changes'); + }); })();