Skip to content

Commit

Permalink
Do not cache group if a child has shadow. (#3864)
Browse files Browse the repository at this point in the history
* fix caching with shadows
* add travis install for canvas
  • Loading branch information
asturur authored Apr 22, 2017
1 parent d95612e commit ea17776
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 3 deletions.
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ node_js:
- "6"
- "4"
script: 'npm run build && npm run test && npm run lint && npm run lint_tests'
before_install:
- sudo apt-get update -qq
- sudo apt-get install libc6 -qq
dist: trusty
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"all": "npm run build && npm run test && npm run lint && npm run lint_tests && npm run export_dist_to_site && npm run export_tests_to_site"
},
"optionalDependencies": {
"canvas-prebuilt": "1.6.x",
"canvas-prebuilt": "1.6.5-prerelease.1",
"jsdom": "9.x.x",
"xmldom": "0.1.x"
},
Expand Down
1 change: 0 additions & 1 deletion src/mixins/itext_key_behavior.mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
}
}
if (insertedText.length) {
console.log(insertedText, fromPaste, fabric.copiedText, fabric.copiedTextStyle)
if (fromPaste && insertedText.join('') === fabric.copiedText) {
this.insertNewStyleBlock(insertedText, this.selectionStart, fabric.copiedTextStyle);
}
Expand Down
46 changes: 46 additions & 0 deletions src/shapes/group.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,52 @@
this._transformDone = false;
},

/**
* Decide if the object should cache or not.
* objectCaching is a global flag, wins over everything
* needsItsOwnCache should be used when the object drawing method requires
* a cache step. None of the fabric classes requires it.
* Generally you do not cache objects in groups because the group outside is cached.
* @return {Boolean}
*/
shouldCache: function() {
var parentCache = this.objectCaching && (!this.group || this.needsItsOwnCache || !this.group.isCaching());
this.caching = parentCache;
if (parentCache) {
for (var i = 0, len = this._objects.length; i < len; i++) {
if (this._objects[i].willDrawShadow()) {
this.caching = false;
return false;
}
}
}
return parentCache;
},

/**
* Check if this object or a child object will cast a shadow
* @return {Boolean}
*/
willDrawShadow: function() {
if (this.shadow) {
return true;
}
for (var i = 0, len = this._objects.length; i < len; i++) {
if (this._objects[i].willDrawShadow()) {
return true;
}
}
return false;
},

/**
* Check if this group or its parent group are caching, recursively up
* @return {Boolean}
*/
isCaching: function() {
return this.caching || this.group && this.group.isCaching();
},

/**
* Execute the drawing operation for an object on a specified context
* @param {CanvasRenderingContext2D} ctx Context to render on
Expand Down
24 changes: 23 additions & 1 deletion src/shapes/object.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@
ctx.transform.apply(ctx, this.transformMatrix);
}
this.clipTo && fabric.util.clipContext(this, ctx);
if (this.objectCaching && (!this.group || this.needsItsOwnCache)) {
if (this.shouldCache()) {
if (!this._cacheCanvas) {
this._createCacheCanvas();
}
Expand All @@ -1175,6 +1175,28 @@
ctx.restore();
},

/**
* Decide if the object should cache or not.
* objectCaching is a global flag, wins over everything
* needsItsOwnCache should be used when the object drawing method requires
* a cache step. None of the fabric classes requires it.
* Generally you do not cache objects in groups because the group outside is cached.
* @return {Boolean}
*/
shouldCache: function() {
return this.objectCaching &&
(!this.group || this.needsItsOwnCache || !this.group.isCaching());
},

/**
* Check if this object or a child object will cast a shadow
* used by Group.shouldCache to know if child has a shadow recursively
* @return {Boolean}
*/
willDrawShadow: function() {
return !!this.shadow;
},

/**
* Execute the drawing operation for an object on a specified context
* @param {CanvasRenderingContext2D} ctx Context to render on
Expand Down
1 change: 1 addition & 0 deletions src/util/lang_class.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
parentMethod = superClassMethod;
break;
}
// eslint-disable-next-line
_this = _this.constructor.superclass.prototype;
}

Expand Down
47 changes: 47 additions & 0 deletions test/unit/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,53 @@

equal(dataless.objects[0].paths, 'sourcePath', 'the paths have been changed with the sourcePath');
});

test('group willDrawShadow', 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 = {};
equal(group3.willDrawShadow(), true, 'group will cast shadow because group itself has shadow');
delete group3.shadow;
group2.shadow = {};
equal(group3.willDrawShadow(), true, 'group will cast shadow because inner group2 has shadow');
delete group2.shadow;
rect1.shadow = {};
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 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}),
rect3 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: true}),
rect4 = new fabric.Rect({ top: 5, left: 5, width: 2, height: 2, strokeWidth: 0, fill: 'red', opacity: 1, objectCaching: true}),
group = new fabric.Group([rect1, rect2], { objectCaching: true}),
group2 = new fabric.Group([rect3, rect4], { objectCaching: true}),
group3 = new fabric.Group([group, group2], { objectCaching: true});

equal(group3.shouldCache(), true, 'group3 will cache because no child has shadow');
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 = {};

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');
equal(group.shouldCache(), false, 'group will not cache because even if is not drawing on parent group3 cache children have shadow');

equal(rect1.shouldCache(), true, 'rect1 will cache because none of its parent is caching');
equal(rect3.shouldCache(), false, 'rect3 will not cache because group2 is caching');

});
// asyncTest('cloning group with image', function() {
// var rect = new fabric.Rect({ top: 100, left: 100, width: 30, height: 10 }),
// img = new fabric.Image(_createImageElement()),
Expand Down

0 comments on commit ea17776

Please sign in to comment.