From e1807214f42516614ca17ecfa06ca0451d9ae4d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Fri, 19 Apr 2013 19:31:29 -0700 Subject: [PATCH] Large refactoring of Style handling for notable speed improvements. --- src/basic/Point.js | 2 +- src/basic/Rectangle.js | 2 +- src/basic/Size.js | 2 +- src/core/Base.js | 5 +-- src/item/Item.js | 22 +++++----- src/item/Shape.js | 4 +- src/path/CompoundPath.js | 6 +-- src/path/Path.js | 35 +++++++-------- src/style/Color.js | 4 +- src/style/Style.js | 93 ++++++++++++++++++++++++---------------- src/svg/SvgExport.js | 12 +++--- src/text/PointText.js | 4 +- 12 files changed, 104 insertions(+), 87 deletions(-) diff --git a/src/basic/Point.js b/src/basic/Point.js index a23b534a..6a039b8c 100644 --- a/src/basic/Point.js +++ b/src/basic/Point.js @@ -25,7 +25,7 @@ */ var Point = this.Point = Base.extend(/** @lends Point# */{ _class: 'Point', - // Tell Base.read that the Point constructor supporst reading with index + // Tell Base.read that the Point constructor supports reading with index _readIndex: true, /** diff --git a/src/basic/Rectangle.js b/src/basic/Rectangle.js index c4448de3..6bdfc6ef 100644 --- a/src/basic/Rectangle.js +++ b/src/basic/Rectangle.js @@ -19,7 +19,7 @@ */ var Rectangle = this.Rectangle = Base.extend(/** @lends Rectangle# */{ _class: 'Rectangle', - // Tell Base.read that the Rectangle constructor supporst reading with index + // Tell Base.read that the Rectangle constructor supports reading with index _readIndex: true, /** diff --git a/src/basic/Size.js b/src/basic/Size.js index 9cb8fec2..10dc970f 100644 --- a/src/basic/Size.js +++ b/src/basic/Size.js @@ -24,7 +24,7 @@ */ var Size = this.Size = Base.extend(/** @lends Size# */{ _class: 'Size', - // Tell Base.read that the Point constructor supporst reading with index + // Tell Base.read that the Point constructor supports reading with index _readIndex: true, // DOCS: improve Size class description diff --git a/src/core/Base.js b/src/core/Base.js index 32a0e4f8..0797ee29 100644 --- a/src/core/Base.js +++ b/src/core/Base.js @@ -170,10 +170,7 @@ this.Base = Base.inject(/** @lends Base# */{ if (!length) length = list.length - index; var obj = list[index]; - if (obj instanceof this - // If the class defines _readNull, return null when nothing - // was provided - || (proto._readNull || readNull) && obj == null && length <= 1) { + if (obj instanceof this || readNull && obj == null && length <= 1) { if (readIndex) list._index = index + 1; return obj && clone ? obj.clone() : obj; diff --git a/src/item/Item.js b/src/item/Item.js index fcb8056c..0f0cfcc1 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -156,7 +156,7 @@ var Item = this.Item = Base.extend(Callback, { // Do not serialize styles on Groups and Layers, since they just unify // their children's own styles. if (!(this instanceof Group)) - serialize(this._style.getDefaults()); + serialize(this._style._defaults); // There is no compact form for Item serialization, we always keep the // type. return [ this._class, props ]; @@ -2155,8 +2155,8 @@ var Item = this.Item = Base.extend(Callback, { // When the matrix could be applied, we also need to transform // color styles with matrices (only gradients so far): var style = this._style, - fillColor = style._fillColor, - strokeColor = style._strokeColor; + fillColor = style.getFillColor(), + strokeColor = style.getStrokeColor(); if (fillColor) fillColor.transform(this._matrix); if (strokeColor) @@ -2743,14 +2743,14 @@ var Item = this.Item = Base.extend(Callback, { // We can access internal properties since we're only using this on // items without children, where styles would be merged. var style = this._style, - width = style._strokeWidth, - join = style._strokeJoin, - cap = style._strokeCap, - limit = style._miterLimit, - fillColor = style._fillColor, - strokeColor = style._strokeColor, - dashArray = style._dashArray, - dashOffset = style._dashOffset; + width = style.getStrokeWidth(), + join = style.getStrokeJoin(), + cap = style.getStrokeCap(), + limit = style.getMiterLimit(), + fillColor = style.getFillColor(), + strokeColor = style.getStrokeColor(), + dashArray = style.getDashArray(), + dashOffset = style.getDashOffset(); if (width != null) ctx.lineWidth = width; if (join) diff --git a/src/item/Shape.js b/src/item/Shape.js index 580c4124..d6cc5c01 100644 --- a/src/item/Shape.js +++ b/src/item/Shape.js @@ -31,8 +31,8 @@ var Shape = this.Shape = Item.extend(/** @lends Shape# */{ size = this._size, width = size.width, height = size.height, - fillColor = style._fillColor, - strokeColor = style._strokeColor; + fillColor = style.getFillColor(), + strokeColor = style.getStrokeColor(); if (fillColor || strokeColor || param.clip) { ctx.beginPath(); switch (this._type) { diff --git a/src/path/CompoundPath.js b/src/path/CompoundPath.js index b876352d..9a40690f 100644 --- a/src/path/CompoundPath.js +++ b/src/path/CompoundPath.js @@ -186,7 +186,7 @@ var CompoundPath = this.CompoundPath = PathItem.extend(/** @lends CompoundPath# _hitTest: function(point, options) { var res = this.base(point, Base.merge(options, { fill: false })); - if (!res && options.fill && this._style._fillColor) { + if (!res && options.fill && this._style.getFillColor()) { res = this._contains(point); res = res ? new HitResult('fill', res[0]) : null; } @@ -206,9 +206,9 @@ var CompoundPath = this.CompoundPath = PathItem.extend(/** @lends CompoundPath# param.compound = false; if (!param.clip) { this._setStyles(ctx); - if (style._fillColor) + if (style.getFillColor()) ctx.fill(); - if (style._strokeColor) + if (style.getStrokeColor()) ctx.stroke(); } } diff --git a/src/path/Path.js b/src/path/Path.js index e0a4506c..f58ce7f7 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -240,7 +240,7 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{ // We only need to draw the connecting curve if it is not a line, and if // the path is closed and has a stroke color, or if it is filled. // TODO: Verify this, sound dodgy - if (this._closed && style._strokeColor || style._fillColor) + if (this._closed && style.getStrokeColor() || style.getFillColor()) addCurve(segments[segments.length - 1], segments[0], true); if (this._closed) parts.push('z'); @@ -1599,8 +1599,9 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{ hasFill: function() { // If this path is part of a CompoundPath, we need to check that // for fillColor too... - return this._style._fillColor || this._parent instanceof CompoundPath - && this._parent._style._fillColor; + return this._style.getFillColor() + || this._parent._type === 'compound-path' + && this._parent._style.getFillColor(); }, contains: function(point) { @@ -1637,8 +1638,8 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{ // directly here: var style = this._style, tolerance = options.tolerance || 0, - radius = (options.stroke && style._strokeColor - ? style._strokeWidth / 2 : 0) + tolerance, + radius = (options.stroke && style.getStrokeColor() + ? style.getStrokeWidth() / 2 : 0) + tolerance, loc, res; // If we're asked to query for segments, ends or handles, do all that @@ -1823,9 +1824,9 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{ // since Path items do not have children, thus do not need style // accessors for merged styles. var style = this._style, - fillColor = style._fillColor, - strokeColor = style._strokeColor, - dashArray = style._dashArray, + fillColor = style.getFillColor(), + strokeColor = style.getStrokeColor(), + dashArray = style.getDashArray(), drawDash = !paper.support.nativeDash && strokeColor && dashArray && dashArray.length; @@ -1850,7 +1851,7 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{ // Use CurveFlatteners to draw dashed paths: ctx.beginPath(); var flattener = new PathFlattener(this), - from = style._dashOffset, to, + from = style.getDashOffset(), to, i = 0; while (from < flattener.length) { to = from + dashArray[(i++) % dashArray.length]; @@ -2298,16 +2299,16 @@ statics: { } // TODO: Find a way to reuse 'bounds' cache instead? - if (!style._strokeColor || !style._strokeWidth) + if (!style.getStrokeColor() || !style.getStrokeWidth()) return Path.getBounds(segments, closed, style, matrix); - var radius = style._strokeWidth / 2, + var radius = style.getStrokeWidth() / 2, padding = getPenPadding(radius, matrix), bounds = Path.getBounds(segments, closed, style, matrix, padding), - join = style._strokeJoin, - cap = style._strokeCap, + join = style.getStrokeJoin(), + cap = style.getStrokeCap(), // miter is relative to stroke width. Divide it by 2 since we're // measuring half the distance below - miter = style._miterLimit * style._strokeWidth / 2; + miter = style.getMiterLimit() * style.getStrokeWidth() / 2; // Create a rectangle of padding size, used for union with bounds // further down var joinBounds = new Rectangle(new Size(padding).multiply(2)); @@ -2429,11 +2430,11 @@ statics: { // Delegate to handleBounds, but pass on radius values for stroke and // joins. Hanlde miter joins specially, by passing the largets radius // possible. - var strokeWidth = style._strokeColor ? style._strokeWidth : 0; + var strokeWidth = style.getStrokeColor() ? style.getStrokeWidth() : 0; return Path.getHandleBounds(segments, closed, style, matrix, strokeWidth, - style._strokeJoin == 'miter' - ? strokeWidth * style._miterLimit + style.getStrokeJoin() == 'miter' + ? strokeWidth * style.getMiterLimit() : strokeWidth); } }}); diff --git a/src/style/Color.js b/src/style/Color.js index 9376414c..14dc4bd8 100644 --- a/src/style/Color.js +++ b/src/style/Color.js @@ -277,9 +277,7 @@ var Color = this.Color = Base.extend(new function() { }, this); }, /** @lends Color# */{ _class: 'Color', - // Tell Base.read that we do not want null to be converted to a color. - _readNull: true, - // Tell Base.read that the Point constructor supporst reading with index + // Tell Base.read that the Point constructor supports reading with index _readIndex: true, /** diff --git a/src/style/Style.js b/src/style/Style.js index 716bfe79..fdcf7ed4 100644 --- a/src/style/Style.js +++ b/src/style/Style.js @@ -68,11 +68,6 @@ var Style = this.Style = Base.extend(new function() { justification: 'left' }; - // Override default fillColor for text items - var textDefaults = Base.merge(defaults, { - fillColor: 'black' - }); - var flags = { strokeWidth: /*#=*/ Change.STROKE, strokeCap: /*#=*/ Change.STROKE, @@ -86,14 +81,17 @@ var Style = this.Style = Base.extend(new function() { var item = {}, fields = { - getDefaults: function() { - return this._item instanceof TextItem ? textDefaults : defaults; - } + _defaults: defaults, + // Override default fillColor for text items + _textDefaults: Base.merge(defaults, { + fillColor: 'black' + }) }; Base.each(defaults, function(value, key) { var isColor = /Color$/.test(key), part = Base.capitalize(key), + flag = flags[key], set = 'set' + part, get = 'get' + part; @@ -101,50 +99,61 @@ var Style = this.Style = Base.extend(new function() { fields[set] = function(value) { var children = this._item && this._item._children; // Clone color objects since they reference their owner - value = isColor ? Color.read(arguments, 0, 0, true) : value; + // value = isColor ? Color.read(arguments, 0, 0, true) : value; // Only unify styles on children of Groups, excluding CompoundPaths. if (children && children.length > 0 && this._item._type !== 'compound-path') { for (var i = 0, l = children.length; i < l; i++) children[i]._style[set](value); } else { - var old = this['_' + key]; - if (!Base.equals(old, value)) { + var old = this._values[key]; + if (old === undefined || !Base.equals(old, value)) { if (isColor) { if (old) delete old._owner; - if (value) { + if (value && value.constructor === Color) value._owner = this._item; - } } - this['_' + key] = value; + this._values[key] = value; // Notify the item of the style change STYLE is always set, - // additional flags come from _flags, as used for STROKE: + // additional flags come from flags, as used for STROKE: if (this._item) - this._item._changed(flags[key] || /*#=*/ Change.STYLE); + this._item._changed(flag || /*#=*/ Change.STYLE); } } }; fields[get] = function() { - var style, + var value, children = this._item && this._item._children; // If this item has children, walk through all of them and see if // they all have the same style. if (!children || children.length === 0 - || this._item._type === 'compound-path') - return this['_' + key]; + || this._item._type === 'compound-path') { + var value = this._values[key]; + if (value === undefined) { + value = this._defaults[key]; + if (value && value.clone) + value = value.clone(); + this._values[key] = value; + } else if (isColor && !(value instanceof Color)) { + this._values[key] = value = Color.read([value], 0, 0, true, true); + if (value) + value._owner = this._item; + } + return value; + } for (var i = 0, l = children.length; i < l; i++) { - var childStyle = children[i]._style[get](); - if (!style) { - style = childStyle; - } else if (!Base.equals(style, childStyle)) { + var childValue = children[i]._style[get](); + if (!value) { + value = childValue; + } else if (!Base.equals(value, childValue)) { // If there is another item with a different // style, the style is not defined: return undefined; } } - return style; + return value; }; // Inject style getters and setters into the Item class, which redirect @@ -162,27 +171,37 @@ var Style = this.Style = Base.extend(new function() { return fields; }, /** @lends Style# */{ initialize: function(style) { - // If the passed style object is also a Style, clone its clonable fields - // rather than simply copying them. - var clone = style instanceof Style; - // Note: This relies on bean getters and setters that get implicetly - // called when getting from style[key] and setting on this[key]. - return Base.each(this.getDefaults(), function(value, key) { - value = style && style[key] || value; - this[key] = value && clone && value.clone - ? value.clone() : value; - }, this); + // Keep values in a separate object that we can iterate over. + this._values = {}; + if (this._item instanceof TextItem) + this._defaults = this._textDefaults; + if (style) { + // If the passed style object is also a Style, clone its clonable + // fields rather than simply copying them. + var isStyle = style instanceof Style, + // Use the other stlyle's _values object for iteration + values = isStyle ? style._values : style; + for (var key in values) { + if (key in this._defaults) { + var value = values[key]; + // Delegate to setter, so Group styles work too. + this[key] = value && isStyle && value.clone + ? value.clone() : value; + } + } + } }, getLeading: function() { // Override leading to return fontSize * 1.2 by default. var leading = this.base(); - return leading != null ? leading : this._fontSize * 1.2; + return leading != null ? leading : this.getFontSize() * 1.2; }, getFontStyle: function() { - var size = this._fontSize; - return (/[a-z]/i.test(size) ? size + ' ' : size + 'px ') + this._font; + var size = this.getFontSize(); + return (/[a-z]/i.test(size) ? size + ' ' : size + 'px ') + + this.getFont(); }, statics: { diff --git a/src/svg/SvgExport.js b/src/svg/SvgExport.js index 8d9082ad..ee569963 100644 --- a/src/svg/SvgExport.js +++ b/src/svg/SvgExport.js @@ -193,11 +193,13 @@ new function() { function exportText(item) { var attrs = getTransform(item, true), - style = item._style; - if (style._font != null) - attrs['font-family'] = style._font; - if (style._fontSize != null) - attrs['font-size'] = style._fontSize; + style = item._style, + font = style.getFont(), + fontSize = style.getFontSize(); + if (font) + attrs['font-family'] = font; + if (fontSize) + attrs['font-size'] = fontSize; var node = createElement('text', attrs); node.textContent = item._content; return node; diff --git a/src/text/PointText.js b/src/text/PointText.js index a46d63fe..b92303ce 100644 --- a/src/text/PointText.js +++ b/src/text/PointText.js @@ -68,9 +68,9 @@ var PointText = this.PointText = TextItem.extend(/** @lends PointText# */{ ctx.textAlign = style.getJustification(); for (var i = 0, l = lines.length; i < l; i++) { var line = lines[i]; - if (style._fillColor) + if (style.getFillColor()) ctx.fillText(line, 0, 0); - if (style._strokeColor) + if (style.getStrokeColor()) ctx.strokeText(line, 0, 0); ctx.translate(0, leading); }