fixed interaction between filters (#5165)

* fixed interaction between filters * added tests
fabricjs · Aug 13, 2018 · e5e4387 · e5e4387
1 parent 87c994d
commit e5e4387
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 12 deletions.
diff --git a/src/shapes/image.class.js b/src/shapes/image.class.js
@@ -157,11 +157,8 @@
      * @chainable
      */
     setElement: function(element, options) {
-      var backend = fabric.filterBackend;
-      if (backend && backend.evictCachesForKey) {
-        backend.evictCachesForKey(this.cacheKey);
-        backend.evictCachesForKey(this.cacheKey + '_filtered');
-      }
+      this.removeTexture(this.cacheKey);
+      this.removeTexture(this.cacheKey + '_filtered');
       this._element = element;
       this._originalElement = element;
       this._initConfig(options);
@@ -175,15 +172,21 @@
     },
 
     /**
-     * Delete cacheKey if we have a webGlBackend
-     * delete reference to image elements
+     * Delete a single texture if in webgl mode
      */
-    dispose: function() {
+    removeTexture: function(key) {
       var backend = fabric.filterBackend;
       if (backend && backend.evictCachesForKey) {
-        backend.evictCachesForKey(this.cacheKey);
-        backend.evictCachesForKey(this.cacheKey + '_filtered');
+        backend.evictCachesForKey(key);
       }
+    },
+
+    /**
+     * Delete textures, reference to elements and eventually JSDOM cleanup
+     */
+    dispose: function() {
+      this.removeTexture(this.cacheKey);
+      this.removeTexture(this.cacheKey + '_filtered');
       this._cacheContext = undefined;
       ['_originalElement', '_element', '_filteredEl', '_cacheCanvas'].forEach((function(element) {
         fabric.util.cleanUpJsdomNode(this[element]);
@@ -407,7 +410,7 @@
         fabric.filterBackend = fabric.initFilterBackend();
       }
       var canvasEl = fabric.util.createCanvasElement(),
-          cacheKey = this._filteredEl ? this.cacheKey : (this.cacheKey + '_filtered'),
+          cacheKey = this._filteredEl ? (this.cacheKey + '_filtered') : this.cacheKey,
           sourceWidth = elementToFilter.width, sourceHeight = elementToFilter.height;
       canvasEl.width = sourceWidth;
       canvasEl.height = sourceHeight;
@@ -435,6 +438,10 @@
       if (this.group) {
         this.set('dirty', true);
       }
+
+      // needs to clear out or WEBGL will not resize correctly
+      this.removeTexture(this.cacheKey + '_filtered');
+
       if (filters.length === 0) {
         this._element = this._originalElement;
         this._filteredEl = null;
@@ -457,7 +464,12 @@
       }
       else {
         // clear the existing element to get new filter data
-        this._element.getContext('2d').clearRect(0, 0, sourceWidth, sourceHeight);
+        // also dereference the eventual resized _element
+        this._element = this._filteredEl;
+        this._filteredEl.getContext('2d').clearRect(0, 0, sourceWidth, sourceHeight);
+        // we also need to resize again at next renderAll, so remove saved _lastScaleX/Y
+        this._lastScaleX = 1;
+        this._lastScaleY = 1;
       }
       if (!fabric.filterBackend) {
         fabric.filterBackend = fabric.initFilterBackend();

diff --git a/test/unit/image.js b/test/unit/image.js
@@ -690,6 +690,50 @@
     });
   });
 
+  QUnit.test('apply filters reset _element and _filteredEl', function(assert) {
+    var done = assert.async();
+    createImageObject(function(image) {
+      var contrast = new fabric.Image.filters.Contrast({ contrast: 0.5 });
+      image.applyFilters();
+      var element = image._element;
+      var filtered = image._filteredEl;
+      image.filters = [contrast];
+      image.applyFilters();
+      assert.notEqual(image._element, element, 'image element has changed');
+      assert.notEqual(image._filteredEl, filtered, 'image _filteredEl element has changed');
+      assert.equal(image._element, image._filteredEl, 'after filtering elements are the same');
+      done();
+    });
+  });
+
+  QUnit.test('apply filters and resize filter', function(assert) {
+    var done = assert.async();
+    createImageObject(function(image) {
+      var contrast = new fabric.Image.filters.Contrast({ contrast: 0.5 });
+      var resizeFilter = new fabric.Image.filters.Resize();
+      image.filters = [contrast];
+      image.resizeFilter = resizeFilter;
+      var element = image._element;
+      var filtered = image._filteredEl;
+      image.scaleX = 0.4;
+      image.scaleY = 0.4;
+      image.applyFilters();
+      assert.notEqual(image._element, element, 'image element has changed');
+      assert.notEqual(image._filteredEl, filtered, 'image _filteredEl element has changed');
+      assert.equal(image._element, image._filteredEl, 'after filtering elements are the same');
+      image.applyResizeFilters();
+      assert.notEqual(image._element, image._filteredEl, 'after resizing the 2 elements differ');
+      assert.equal(image._lastScaleX, image.scaleX, 'after resizing we know how much we scaled');
+      assert.equal(image._lastScaleY, image.scaleY, 'after resizing we know how much we scaled');
+      image.applyFilters();
+      assert.equal(image._element, image._filteredEl, 'after filters again the elements changed');
+      assert.equal(image._lastScaleX, 1, 'lastScale X is reset');
+      assert.equal(image._lastScaleY, 1, 'lastScale Y is reset');
+      assert.equal(image._needsResize(), true, 'resizing is needed again');
+      done();
+    });
+  });
+
   QUnit.test('apply filters set the image dirty and also the group', function(assert) {
     var done = assert.async();
     createImageObject(function(image) {