Skip to content

Commit

Permalink
Fix a bug in recent caching changes (#4051)
Browse files Browse the repository at this point in the history
* small fix
* fixed and added tests
* comeon andrea wake up
  • Loading branch information
asturur authored Jul 1, 2017
1 parent 4a8df81 commit 98ed4cc
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/shapes/group.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@
*/
willDrawShadow: function() {
if (this.shadow) {
return true;
return this.callSuper('willDrawShadow');
}
for (var i = 0, len = this._objects.length; i < len; i++) {
if (this._objects[i].willDrawShadow()) {
Expand Down
6 changes: 3 additions & 3 deletions src/shapes/object.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -945,15 +945,15 @@
if (shouldResizeCanvas) {
this._cacheCanvas.width = Math.max(Math.ceil(width) + additionalWidth, minCacheSize);
this._cacheCanvas.height = Math.max(Math.ceil(height) + additionalHeight, minCacheSize);
this.cacheWidth = width;
this.cacheHeight = height;
this.cacheTranslationX = (width + additionalWidth) / 2;
this.cacheTranslationY = (height + additionalHeight) / 2;
}
else {
this._cacheContext.setTransform(1, 0, 0, 1, 0, 0);
this._cacheContext.clearRect(0, 0, this._cacheCanvas.width, this._cacheCanvas.height);
}
this.cacheWidth = width;
this.cacheHeight = height;
this._cacheContext.translate(this.cacheTranslationX, this.cacheTranslationY);
this._cacheContext.scale(zoomX, zoomY);
this.zoomX = zoomX;
Expand Down Expand Up @@ -1268,7 +1268,7 @@
* @return {Boolean}
*/
willDrawShadow: function() {
return !!this.shadow;
return !!this.shadow && (this.shadow.offsetX !== 0 || this.shadow.offsetY !== 0);
},

/**
Expand Down
32 changes: 27 additions & 5 deletions test/unit/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -612,18 +612,40 @@
group3 = new fabric.Group([group, group2]);

equal(group3.willDrawShadow(), false, 'group will not cast shadow because objects do not have it');
group3.shadow = {};
group3.shadow = { offsetX: 1, offsetY: 2, };
equal(group3.willDrawShadow(), true, 'group will cast shadow because group itself has shadow');
delete group3.shadow;
group2.shadow = {};
group2.shadow = { offsetX: 1, offsetY: 2, };
equal(group3.willDrawShadow(), true, 'group will cast shadow because inner group2 has shadow');
delete group2.shadow;
rect1.shadow = {};
rect1.shadow = { offsetX: 1, offsetY: 2, };
equal(group3.willDrawShadow(), true, 'group will cast shadow because inner rect1 has shadow');
equal(group.willDrawShadow(), true, 'group will cast shadow because inner rect1 has shadow');
equal(group2.willDrawShadow(), false, 'group will not cast shadow because no child has shadow');
});

test('group willDrawShadow with no offsets', function() {
var rect1 = new fabric.Rect({ top: 1, left: 1, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false}),
rect2 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false}),
rect3 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false}),
rect4 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: false}),
group = new fabric.Group([rect1, rect2]),
group2 = new fabric.Group([rect3, rect4]),
group3 = new fabric.Group([group, group2]);

equal(group3.willDrawShadow(), false, 'group will not cast shadow because objects do not have it');
group3.shadow = { offsetX: 0, offsetY: 0 };
equal(group3.willDrawShadow(), false, 'group will NOT cast shadow because group itself has shadow but not offsets');
group3.shadow = { offsetX: 2, offsetY: 0 };
equal(group3.willDrawShadow(), true, 'group will cast shadow because group itself has shadow and one offsetX different than 0');
group3.shadow = { offsetX: 0, offsetY: 2 };
equal(group3.willDrawShadow(), true, 'group will cast shadow because group itself has shadow and one offsetY different than 0');
group3.shadow = { offsetX: -2, offsetY: 0 };
equal(group3.willDrawShadow(), true, 'group will cast shadow because group itself has shadow and one offsetX different than 0');
group3.shadow = { offsetX: 0, offsetY: -2 };
equal(group3.willDrawShadow(), true, 'group will cast shadow because group itself has shadow and one offsetY different than 0');
});

test('group shouldCache', function() {
var rect1 = new fabric.Rect({ top: 1, left: 1, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: true}),
rect2 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: true}),
Expand All @@ -637,8 +659,8 @@
equal(group2.shouldCache(), false, 'group2 will not cache because is drawing on parent group3 cache');
equal(rect3.shouldCache(), false, 'rect3 will not cache because is drawing on parent2 group cache');

group2.shadow = {};
rect1.shadow = {};
group2.shadow = { offsetX: 2, offsetY: 0 };
rect1.shadow = { offsetX: 0, offsetY: 2 };

equal(group3.shouldCache(), false, 'group3 will cache because children have shadow');
equal(group2.shouldCache(), true, 'group2 will cache because is not drawing on parent group3 cache and no children have shadow');
Expand Down
7 changes: 7 additions & 0 deletions test/unit/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -1351,4 +1351,11 @@
equal(context.shadowOffsetY, object.shadow.offsetY * object.scaleY * group.scaleY);
equal(context.shadowBlur, object.shadow.blur * (object.scaleX * group.scaleX + object.scaleY * group.scaleY) / 2);
});

test('willDrawShadow', function() {
var object = new fabric.Object({ shadow: { offsetX: 0, offsetY: 0 }});
equal(object.willDrawShadow(), false, 'object will not drawShadow');
object.shadow.offsetX = 1;
equal(object.willDrawShadow(), true, 'object will drawShadow');
});
})();

0 comments on commit 98ed4cc

Please sign in to comment.