Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING changing objectCaching #5567

Merged
merged 10 commits into from
Mar 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion HEADER.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ fabric.DPI = 96;
fabric.reNum = '(?:[-+]?(?:\\d+|\\d*\\.\\d+)(?:e[-+]?\\d+)?)';
fabric.fontPaths = { };
fabric.iMatrix = [1, 0, 0, 1, 0, 0];
fabric.canvasModule = 'canvas';

/**
* Pixel limit for cache canvases. 1Mpx , 4Mpx should be fine.
Expand Down
1 change: 0 additions & 1 deletion build.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ var filesToInclude = [
ifSpecifiedInclude('itext', 'src/mixins/itext.svg_export.js'),

ifSpecifiedInclude('textbox', 'src/shapes/textbox.class.js'),
ifSpecifiedInclude('textbox', 'src/mixins/textbox_behavior.mixin.js'),

];

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"build:watch": "onchange 'src/**/**' 'HEADER.js' 'lib/**/**' -- npm run build_export",
"build_with_gestures": "node build.js modules=ALL exclude=accessors",
"build_export": "npm run build:fast && npm run export_dist_to_site",
"test:single": "quint test/node_test_setup.js test/lib",
"test:single": "qunit test/node_test_setup.js test/lib",
"test": "nyc qunit test/node_test_setup.js test/lib test/unit",
"test:visual": "qunit test/node_test_setup.js test/lib test/visual",
"test:visual:single": "qunit test/node_test_setup.js test/lib",
Expand Down
14 changes: 12 additions & 2 deletions src/canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,18 @@
changeX = target.scaleX !== scaleX,
changeY = target.scaleY !== scaleY;

transform.newScaleX = scaleX;
transform.newScaleY = scaleY;
if (by === 'x' && target instanceof fabric.Textbox) {
var w = target.width * (localMouse.x / _dim.x);
if (w >= target.getMinWidth()) {
scaled = w !== target.width;
target.set('width', w);
return scaled;
}
return false;
}

if (lockScalingFlip && scaleX <= 0 && scaleX < target.scaleX) {
forbidScalingX = true;
localMouse.x = 0;
Expand All @@ -928,8 +940,6 @@
else if (by === 'y' && !target.get('lockUniScaling')) {
forbidScalingY || lockScalingY || (target.set('scaleY', scaleY) && (scaled = changeY));
}
transform.newScaleX = scaleX;
transform.newScaleY = scaleY;
forbidScalingX || forbidScalingY || this._flipObject(transform, by);
return scaled;
},
Expand Down
45 changes: 0 additions & 45 deletions src/mixins/textbox_behavior.mixin.js

This file was deleted.

6 changes: 2 additions & 4 deletions src/shapes/group.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,13 @@

/**
* Decide if the object should cache or not. Create its own cache level
* 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.
* Generally you do not cache objects in groups because the group is already cached.
* @return {Boolean}
*/
shouldCache: function() {
var ownCache = this.objectCaching && (!this.group || this.needsItsOwnCache() || !this.group.isOnACache());
this.ownCaching = ownCache;
var ownCache = fabric.Object.prototype.shouldCache.call(this);
if (ownCache) {
for (var i = 0, len = this._objects.length; i < len; i++) {
if (this._objects[i].willDrawShadow()) {
Expand Down
4 changes: 1 addition & 3 deletions src/shapes/image.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,6 @@

/**
* Decide if the object should cache or not. Create its own cache level
* 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.
Expand All @@ -515,8 +514,7 @@
* @return {Boolean}
*/
shouldCache: function() {
this.ownCaching = this.objectCaching && this.needsItsOwnCache();
return this.ownCaching;
return this.needsItsOwnCache();
},

_renderFill: function(ctx) {
Expand Down
45 changes: 39 additions & 6 deletions src/shapes/object.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -522,16 +522,17 @@

/**
* When `true`, object is not exported in OBJECT/JSON
* since 1.6.3
* @since 1.6.3
* @type Boolean
* @default
*/
excludeFromExport: false,

/**
* When `true`, object is cached on an additional canvas.
* When `false`, object is not cached unless necessary ( clipPath )
* default to true
* since 1.7.0
* @since 1.7.0
* @type Boolean
* @default true
*/
Expand Down Expand Up @@ -1115,16 +1116,45 @@
this.cacheHeight = 0;
},

/**
* return true if the object will draw a stroke
* Does not consider text styles. This is just a shortcut used at rendering time
* We want it to be an aproximation and be fast.
* wrote to avoid extra caching, it has to return true when stroke happens,
* can guess when it will not happen at 100% chance, does not matter if it misses
* some use case where the stroke is invisible.
* @since 3.0.0
* @returns Boolean
*/
hasStroke: function() {
return this.stroke && this.stroke !== 'transparent' && this.strokeWidth !== 0;
},

/**
* return true if the object will draw a fill
* Does not consider text styles. This is just a shortcut used at rendering time
* We want it to be an aproximation and be fast.
* wrote to avoid extra caching, it has to return true when fill happens,
* can guess when it will not happen at 100% chance, does not matter if it misses
* some use case where the fill is invisible.
* @since 3.0.0
* @returns Boolean
*/
hasFill: function() {
return this.fill && this.fill !== 'transparent';
},

/**
* When set to `true`, force the object to have its own cache, even if it is inside a group
* it may be needed when your object behave in a particular way on the cache and always needs
* its own isolated canvas to render correctly.
* Created to be overridden
* since 1.7.12
* @returns false
* @returns Boolean
*/
needsItsOwnCache: function() {
if (this.paintFirst === 'stroke' && typeof this.shadow === 'object') {
if (this.paintFirst === 'stroke' &&
this.hasFill() && this.hasStroke() && typeof this.shadow === 'object') {
return true;
}
if (this.clipPath) {
Expand All @@ -1139,11 +1169,14 @@
* 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.
* Read as: cache if is needed, or if the feature is enabled but we are not already caching.
* @return {Boolean}
*/
shouldCache: function() {
this.ownCaching = this.objectCaching &&
(!this.group || this.needsItsOwnCache() || !this.group.isOnACache());
this.ownCaching = this.needsItsOwnCache() || (
this.objectCaching &&
(!this.group || !this.group.isOnACache())
);
return this.ownCaching;
},

Expand Down
14 changes: 14 additions & 0 deletions src/shapes/textbox.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,20 @@
return Math.max(this.minWidth, this.dynamicMinWidth);
},

_removeExtraneousStyles: function() {
var linesToKeep = {};
for (var prop in this._styleMap) {
if (this._textLines[prop]) {
linesToKeep[this._styleMap[prop].line] = 1;
}
}
for (var prop in this.styles) {
if (!linesToKeep[prop]) {
delete this.styles[prop];
}
}
},

/**
* Returns object representation of an instance
* @method toObject
Expand Down
Binary file added test/fixtures/Ubuntu-Bold.ttf
Binary file not shown.
Binary file added test/fixtures/Ubuntu-BoldItalic.ttf
Binary file not shown.
Binary file added test/fixtures/Ubuntu-Italic.ttf
Binary file not shown.
Binary file added test/fixtures/Ubuntu-Regular.ttf
Binary file not shown.
15 changes: 15 additions & 0 deletions test/unit/itext.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,21 @@
iText.abortCursorAnimation();
});

QUnit.test('_removeExtraneousStyles', function(assert) {
var iText = new fabric.IText('a\nq\qo', { styles: {
0: { 0: { fontSize: 4 } },
1: { 0: { fontSize: 4 } },
2: { 0: { fontSize: 4 } },
3: { 0: { fontSize: 4 } },
4: { 0: { fontSize: 4 } },
} });
assert.deepEqual(iText.styles[3], { 0: { fontSize: 4 } }, 'style line 3 exists');
assert.deepEqual(iText.styles[4], { 0: { fontSize: 4 } }, 'style line 4 exists');
iText._removeExtraneousStyles();
assert.equal(iText.styles[3], undefined, 'style line 3 has been removed');
assert.equal(iText.styles[4], undefined, 'style line 4 has been removed');
});

QUnit.test('hiddenTextarea does not move DOM', function(assert) {
var iText = new fabric.IText('a', { fill: '#ffffff', fontSize: 50 });
var canvas2 = new fabric.Canvas(null, { width: 800, height: 800, renderOnAddRemove: false });
Expand Down
86 changes: 86 additions & 0 deletions test/unit/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -1254,4 +1254,90 @@
object = new fabric.Object({ fill: 'blue', width: 0, height: 0, strokeWidth: 0 });
assert.equal(object.isNotVisible(), true, 'object is not visilbe with also strokeWidth equal 0');
});
QUnit.test('shouldCache', function(assert) {
var object = new fabric.Object();
object.objectCaching = false;
assert.equal(object.shouldCache(), false, 'if objectCaching is false, object should not cache');
object.objectCaching = true;
assert.equal(object.shouldCache(), true, 'if objectCaching is true, object should cache');
object.objectCaching = false;
object.needsItsOwnCache = function () { return true; };
assert.equal(object.shouldCache(), true, 'if objectCaching is false, but we have a clipPath, shouldCache returns true');

object.needsItsOwnCache = function () { return false; };

object.objectCaching = true;
object.group = { isOnACache: function() { return true; }};
assert.equal(object.shouldCache(), false, 'if objectCaching is true, but we are in a group, shouldCache returns false');

object.objectCaching = true;
object.group = { isOnACache: function() { return false; }};
assert.equal(object.shouldCache(), true, 'if objectCaching is true, but we are in a not cached group, shouldCache returns true');

object.objectCaching = false;
object.group = { isOnACache: function() { return false; }};
assert.equal(object.shouldCache(), false, 'if objectCaching is false, but we are in a not cached group, shouldCache returns false');

object.objectCaching = false;
object.group = { isOnACache: function() { return true; }};
assert.equal(object.shouldCache(), false, 'if objectCaching is false, but we are in a cached group, shouldCache returns false');

object.needsItsOwnCache = function () { return true; };

object.objectCaching = false;
object.group = { isOnACache: function() { return true; }};
assert.equal(object.shouldCache(), true, 'if objectCaching is false, but we have a clipPath, group cached, we cache anyway');

object.objectCaching = false;
object.group = { isOnACache: function() { return false; }};
assert.equal(object.shouldCache(), true, 'if objectCaching is false, but we have a clipPath, group not cached, we cache anyway');

});
QUnit.test('needsItsOwnCache', function(assert) {
var object = new fabric.Object();
assert.equal(object.needsItsOwnCache(), false, 'default needsItsOwnCache is false');
object.clipPath = {};
assert.equal(object.needsItsOwnCache(), true, 'with a clipPath is true');
delete object.clipPath;

object.paintFirst = 'stroke';
object.stroke = 'black';
object.shadow = {};
assert.equal(object.needsItsOwnCache(), true, 'if stroke first will return true');

object.paintFirst = 'stroke';
object.stroke = 'black';
object.shadow = null;
assert.equal(object.needsItsOwnCache(), true, 'if stroke first will return false if no shadow');

object.paintFirst = 'stroke';
object.stroke = '';
object.shadow = {};
assert.equal(object.needsItsOwnCache(), false, 'if stroke first will return false if no stroke');

object.paintFirst = 'stroke';
object.stroke = 'black';
object.fill = '';
object.shadow = {};
assert.equal(object.needsItsOwnCache(), false, 'if stroke first will return false if no fill');
});
QUnit.test('hasStroke', function(assert) {
var object = new fabric.Object({ fill: 'blue', width: 100, height: 100, strokeWidth: 3, stroke: 'black' });
assert.equal(object.hasStroke(), true, 'if strokeWidth is present and stroke is black hasStroke is true');
object.stroke = '';
assert.equal(object.hasStroke(), false, 'if strokeWidth is present and stroke is empty string hasStroke is false');
object.stroke = 'transparent';
assert.equal(object.hasStroke(), false, 'if strokeWidth is present and stroke is transparent hasStroke is false');
object.stroke = 'black';
object.strokeWidth = 0;
assert.equal(object.hasStroke(), false, 'if strokeWidth is 0 and stroke is a color hasStroke is false');
});
QUnit.test('hasFill', function(assert) {
var object = new fabric.Object({ fill: 'blue', width: 100, height: 100 });
assert.equal(object.hasFill(), true, 'with a color that is not transparent, hasFill is true');
object.fill = '';
assert.equal(object.hasFill(), false, 'without a color, hasFill is false');
object.fill = 'transparent';
assert.equal(object.hasFill(), false, 'with a color that is transparent, hasFill is true');
});
})();
Loading