From 95ecab8a6f54a5e21adfd5cd040add43ad1eeac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Wed, 12 Jun 2013 20:12:08 -0700 Subject: [PATCH] Improve handling of merged CompoundPath style through #getStyle(). Only access _style directly in core code if you really know what you're doing! --- src/item/Item.js | 12 +++++++----- src/path/CompoundPath.js | 2 +- src/path/Curve.js | 4 ++-- src/path/Path.js | 28 ++++++++-------------------- src/svg/SVGExport.js | 6 +++--- 5 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/item/Item.js b/src/item/Item.js index d6c1a73d..030e8603 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -381,15 +381,17 @@ var Item = Base.extend(Callback, /** @lends Item# */{ }, setStyle: function(style) { - this._style.set(style); + // Don't access _style directly so Path#getStyle() can be overriden for + // CompoundPaths. + this.getStyle().set(style); }, hasFill: function() { - return !!this._style.getFillColor(); + return !!this.getStyle().getFillColor(); }, hasStroke: function() { - return !!this._style.getStrokeColor(); + return !!this.getStyle().getStrokeColor(); } }, Base.each(['locked', 'visible', 'blendMode', 'opacity', 'guide'], // Produce getter/setters for properties. We need setters because we want to @@ -2890,8 +2892,8 @@ var Item = Base.extend(Callback, /** @lends Item# */{ var blending = this._blendMode !== 'normal', parentCtx, itemOffset, prevOffset; if (blending || this._opacity < 1 && this._type !== 'raster' - && (this._type !== 'path' - || this.getFillColor() && this.getStrokeColor())) { + && (this._type !== 'path' + || this.hasFill() && this.hasStroke())) { // Apply the paren't global matrix to the calculation of correct // bounds. var bounds = this.getStrokeBounds(parentMatrix); diff --git a/src/path/CompoundPath.js b/src/path/CompoundPath.js index fc54b362..47b77eaa 100644 --- a/src/path/CompoundPath.js +++ b/src/path/CompoundPath.js @@ -227,7 +227,7 @@ var CompoundPath = PathItem.extend(/** @lends CompoundPath# */{ _hitTest: function _hitTest(point, options) { var res = _hitTest.base.call(this, point, Base.merge(options, { fill: false })); - if (!res && options.fill && this._style.getFillColor()) { + if (!res && options.fill && this.hasFill()) { res = this._contains(point); res = res ? new HitResult('fill', res[0]) : null; } diff --git a/src/path/Curve.js b/src/path/Curve.js index 09744575..de88490d 100644 --- a/src/path/Curve.js +++ b/src/path/Curve.js @@ -729,8 +729,8 @@ statics: { if (!bounds) { // Calculate the curve bounds by passing a segment list for the // curve to the static Path.get*Boudns methods. - bounds = this._bounds[name] = Path[name]( - [this._segment1, this._segment2], false, this._path._style); + bounds = this._bounds[name] = Path[name]([this._segment1, + this._segment2], false, this._path.getStyle()); } return bounds.clone(); }; diff --git a/src/path/Path.js b/src/path/Path.js index 31d9de2a..4bda2877 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -204,7 +204,6 @@ var Path = PathItem.extend(/** @lends Path# */{ */ getPathData: function(/* precision */) { var segments = this._segments, - style = this._style, precision = arguments[0], f = Formatter.instance, parts = []; @@ -1585,18 +1584,11 @@ var Path = PathItem.extend(/** @lends Path# */{ return this.getNearestLocation(point).getPoint(); }, - hasFill: function() { - // If this path is part of a CompoundPath, we need to check that - // for fillColor... + getStyle: function() { + // If this path is part of a CompoundPath, use the paren't style instead var parent = this._parent; - return !!(parent && parent._type === 'compound-path' - ? parent : this)._style.getFillColor(); - }, - - hasStroke: function() { - var parent = this._parent; - return !!(parent && parent._type === 'compound-path' - ? parent : this)._style.getStrokeColor(); + return (parent && parent._type === 'compound-path' + ? parent : this)._style; }, _contains: function(point) { @@ -1641,9 +1633,7 @@ var Path = PathItem.extend(/** @lends Path# */{ }, _hitTest: function(point, options) { - // See #draw() for an explanation of why we can access _style properties - // directly here: - var style = this._style, + var style = this.getStyle(), tolerance = options.tolerance || 0, radius = (options.stroke && style.getStrokeColor() ? style.getStrokeWidth() / 2 : 0) + tolerance, @@ -1829,10 +1819,7 @@ var Path = PathItem.extend(/** @lends Path# */{ if (!compound) ctx.beginPath(); - // We can access styles directly on the internal _styles object, - // since Path items do not have children, thus do not need style - // accessors for merged styles. - var style = this._style, + var style = this.getStyle(), fillColor = style.getFillColor(), strokeColor = style.getStrokeColor(), dashArray = style.getDashArray(), @@ -2215,7 +2202,8 @@ var Path = PathItem.extend(/** @lends Path# */{ _getBounds: function(getter, matrix) { // See #draw() for an explanation of why we can access _style // properties directly here: - return Path[getter](this._segments, this._closed, this._style, matrix); + return Path[getter](this._segments, this._closed, this.getStyle(), + matrix); }, // Mess with indentation in order to get more line-space below... diff --git a/src/svg/SVGExport.js b/src/svg/SVGExport.js index aca2dfff..21336a89 100644 --- a/src/svg/SVGExport.js +++ b/src/svg/SVGExport.js @@ -199,7 +199,7 @@ new function() { function exportText(item) { var attrs = getTransform(item, true), - style = item._style, + style = item.getStyle(), font = style.getFont(), fontSize = style.getFontSize(); if (font) @@ -407,9 +407,9 @@ new function() { function applyStyle(item, node) { var attrs = {}, - style = item._style, + style = item.getStyle(), parent = item.getParent(), - parentStyle = parent && parent._style; + parentStyle = parent && parent.getStyle(); if (item._name != null) attrs.id = item._name;