diff --git a/src/item/Item.js b/src/item/Item.js index 3aea710b..656d36d5 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -309,7 +309,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ return this._name; }, - setName: function(name, unique) { + setName: function(name) { // Note: Don't check if the name has changed and bail out if it has not, // because setName is used internally also to update internal structures // when an item is moved from one parent to another. @@ -326,12 +326,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ var parent = this._parent; if (name && parent) { var children = parent._children, - namedChildren = parent._namedChildren, - orig = name, - i = 1; - // If unique is true, make sure we're not overriding other names - while (unique && children[name]) - name = orig + ' ' + (i++); + namedChildren = parent._namedChildren; (namedChildren[name] = namedChildren[name] || []).push(this); children[name] = this; } @@ -1437,61 +1432,82 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ * } */ clone: function(insert) { - return this._clone(new this.constructor(Item.NO_INSERT), insert); - }, - - /** - * Clones the item within the same project and places the copy above the - * item. - * - * @param {Boolean} [insert=true] specifies whether the copy should be - * inserted into the DOM. When set to {@code true}, it is inserted above the - * original - * @return {Item} the newly cloned item - */ - _clone: function(copy, insert, includeMatrix) { - var keys = ['_locked', '_visible', '_blendMode', '_opacity', - '_clipMask', '_guide'], - children = this._children; - // Copy over style - copy.setStyle(this._style); - // Clone all children and add them to the copy. tell #addChild we're - // cloning, as needed by CompoundPath#insertChild(). - for (var i = 0, l = children && children.length; i < l; i++) { - copy.addChild(children[i].clone(false), true); - } - // Only copy over these fields if they are actually defined in 'this', - // meaning the default value has been overwritten (default is on - // prototype). - for (var i = 0, l = keys.length; i < l; i++) { - var key = keys[i]; - if (this.hasOwnProperty(key)) - copy[key] = this[key]; - } - // Use Matrix#initialize to easily copy over values. - if (includeMatrix !== false) - copy._matrix.initialize(this._matrix); - // In case of Path#toShape(), we can't just set _applyMatrix as - // Shape won't allow it. Using the setter instead takes care of it. - // NOTE: This will also bake in the matrix that we just initialized, - // in case #applyMatrix is true. - copy.setApplyMatrix(this._applyMatrix); - copy.setPivot(this._pivot); - // Copy over the selection state, use setSelected so the item - // is also added to Project#selectedItems if it is selected. - copy.setSelected(this._selected); - // Copy over _data as well. - copy._data = this._data ? Base.clone(this._data) : null; + var copy = new this.constructor(Item.NO_INSERT); + copy.copyAttributes(this); + copy.copyContent(this); // Insert is true by default. if (insert || insert === undefined) copy.insertAbove(this); - // Clone the name too, but make sure we're not overriding the original - // in the same parent, by passing true for the unique parameter. - if (this._name) - copy.setName(this._name, true); + // Make sure we're not overriding the original name in the same parent + var name = this._name, + parent = this._parent; + if (name && parent) { + var children = parent._children, + orig = name, + i = 1; + while (children[name]) + name = orig + ' ' + (i++); + if (name !== orig) + copy.setName(name); + } return copy; }, + /** + * Copies the content of the specified item over to this item. + * + * @param {Item} source the item to copy the content from + */ + copyContent: function(source) { + var children = source._children; + // Clone all children and add them to the copy. tell #addChild we're + // cloning, as needed by CompoundPath#insertChild(). + for (var i = 0, l = children && children.length; i < l; i++) { + this.addChild(children[i].clone(false), true); + } + }, + + /** + * Copies all attributes of the specified item over to this item. This + * includes its style, visibility, matrix, pivot, blend-mode, opacity, + * selection state, data, name, etc. + * + * @param {Item} source the item to copy the attributes from + */ + copyAttributes: function(source, _preConcatenate) { + // Copy over style + this.setStyle(source._style); + // Only copy over these fields if they are actually defined in 'source', + // meaning the default value has been overwritten (default is on + // prototype). + var keys = ['_locked', '_visible', '_blendMode', '_opacity', + '_clipMask', '_guide']; + for (var i = 0, l = keys.length; i < l; i++) { + var key = keys[i]; + if (source.hasOwnProperty(key)) + this[key] = source[key]; + } + // Use Matrix#initialize to easily copy over values. + this._matrix[_preConcatenate ? 'preConcatenate' : 'initialize']( + source._matrix); + // We can't just set _applyMatrix as many item types won't allow it, + // e.g. creating a Shape in Path#toShape(). + // Using the setter instead takes care of it. + // NOTE: This will also bake in the matrix that we just initialized, + // in case #applyMatrix is true. + this.setApplyMatrix(source._applyMatrix); + this.setPivot(source._pivot); + // Copy over the selection state, use setSelected so the item + // is also added to Project#selectedItems if it is selected. + this.setSelected(source._selected); + // Copy over data and name as well. + var data = source._data, + name = source._name; + this._data = data ? Base.clone(data) : null; + if (name) + this.setName(name); + }, + /** * When passed a project, copies the item to the project, * or duplicates it within the same project. When passed an item, @@ -2299,7 +2315,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{ if (this._children && this._children.length === 1) { var child = this._children[0].reduce(); child.insertAbove(this); - child.setStyle(this._style); + child.copyAttributes(this); this.remove(); return child; } diff --git a/src/item/PlacedSymbol.js b/src/item/PlacedSymbol.js index 2f345b76..f30febcf 100644 --- a/src/item/PlacedSymbol.js +++ b/src/item/PlacedSymbol.js @@ -84,6 +84,10 @@ var PlacedSymbol = Item.extend(/** @lends PlacedSymbol# */{ return this._symbol === item._symbol; }, + copyContent: function(source) { + this.setSymbol(source._symbol); + }, + /** * The symbol that the placed symbol refers to. * @@ -99,16 +103,11 @@ var PlacedSymbol = Item.extend(/** @lends PlacedSymbol# */{ this._changed(/*#=*/Change.GEOMETRY); }, - clone: function(insert) { - var copy = new PlacedSymbol(Item.NO_INSERT); - copy.setSymbol(this._symbol); - return this._clone(copy, insert); - }, - isEmpty: function() { return this._symbol._definition.isEmpty(); }, + _getBounds: function(getter, matrix, cacheItem) { var definition = this.symbol._definition; // Redirect the call to the symbol definition to calculate the bounds diff --git a/src/item/Raster.js b/src/item/Raster.js index a0ef9a41..6329950c 100644 --- a/src/item/Raster.js +++ b/src/item/Raster.js @@ -101,21 +101,20 @@ var Raster = Item.extend(/** @lends Raster# */{ return this.getSource() === item.getSource(); }, - clone: function(insert) { - var copy = new Raster(Item.NO_INSERT), - image = this._image, - canvas = this._canvas; + copyContent: function(source) { + var image = source._image, + canvas = source._canvas; if (image) { - copy.setImage(image); + this.setImage(image); } else if (canvas) { // If the Raster contains a Canvas object, we need to create a new // one and draw this raster's canvas on it. - var copyCanvas = CanvasProvider.getCanvas(this._size); + var copyCanvas = CanvasProvider.getCanvas(source._size); copyCanvas.getContext('2d').drawImage(canvas, 0, 0); - copy.setImage(copyCanvas); + this.setImage(copyCanvas); } - copy._crossOrigin = this._crossOrigin; - return this._clone(copy, insert); + // TODO: Shouldn't this be copied with attributes instead of content? + this._crossOrigin = source._crossOrigin; }, /** diff --git a/src/item/Shape.js b/src/item/Shape.js index af19a001..f81ce25e 100644 --- a/src/item/Shape.js +++ b/src/item/Shape.js @@ -39,12 +39,10 @@ var Shape = Item.extend(/** @lends Shape# */{ && Base.equals(this._radius, item._radius); }, - clone: function(insert) { - var copy = new Shape(Item.NO_INSERT); - copy.setType(this._type); - copy.setSize(this._size); - copy.setRadius(this._radius); - return this._clone(copy, insert); + copyContent: function(source) { + this.setType(source._type); + this.setSize(source._size); + this.setRadius(source._radius); }, /** @@ -169,17 +167,20 @@ var Shape = Item.extend(/** @lends Shape# */{ * @see Path#toShape(insert) */ toPath: function(insert) { - var path = this._clone(new Path[Base.capitalize(this._type)]({ + // TODO: Move to Path.createTYPE creators instead of fake constructors. + var path = new Path[Base.capitalize(this._type)]({ center: new Point(), size: this._size, radius: this._radius, insert: false - }), insert); + }); + path.copyAttributes(this); // The created path will inherit #applyMatrix from this Shape, hence it // will always be false. // Respect the setting of paper.settings.applyMatrix for new paths: if (paper.settings.applyMatrix) path.setApplyMatrix(true); + path.insertAbove(this); return path; }, diff --git a/src/path/CompoundPath.js b/src/path/CompoundPath.js index 1278fc78..5c192e54 100644 --- a/src/path/CompoundPath.js +++ b/src/path/CompoundPath.js @@ -152,10 +152,8 @@ var CompoundPath = PathItem.extend(/** @lends CompoundPath# */{ } if (children.length === 0) { // Replace with a simple empty Path var path = new Path(Item.NO_INSERT); + path.copyAttributes(this); path.insertAbove(this); - // TODO: Consider using Item#_clone() for this, but find a way to - // not clone children / name (content). - path.setStyle(this._style); this.remove(); return path; } diff --git a/src/path/Path.js b/src/path/Path.js index c059b3d5..3506aceb 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -131,13 +131,12 @@ var Path = PathItem.extend(/** @lends Path# */{ && Base.equals(this._segments, item._segments); }, - clone: function(insert) { - var copy = new Path(Item.NO_INSERT); - copy.setSegments(this._segments); - copy._closed = this._closed; - if (this._clockwise !== undefined) - copy._clockwise = this._clockwise; - return this._clone(copy, insert); + copyContent: function(source) { + this.setSegments(source._segments); + this._closed = source._closed; + var clockwise = source._clockwise; + if (clockwise !== undefined) + this._clockwise = clockwise; }, _changed: function _changed(flags) { @@ -1247,8 +1246,7 @@ var Path = PathItem.extend(/** @lends Path# */{ // Pass true for _preserve, in case of CompoundPath, to avoid // reversing of path direction, which would mess with segments! path.insertAbove(this, true); - // Use _clone to copy over all other attributes, including style - this._clone(path); + path.copyAttributes(this); } path._add(segs, 0); // Add dividing segment again. In case of a closed path, that's the @@ -1496,14 +1494,17 @@ var Path = PathItem.extend(/** @lends Path# */{ if (type) { var center = this.getPosition(true), - shape = this._clone(new type({ + shape = new type({ center: center, size: size, radius: radius, insert: false - }), insert, false); + }); + // Pass `true` to preconcatenate the matrix to the center-transform. + shape.copyAttributes(this, true); // Determine and apply the shape's angle of rotation. shape.rotate(topCenter.subtract(center).getAngle() + 90); + shape.insertAbove(this); return shape; } return null; diff --git a/src/path/PathItem.Boolean.js b/src/path/PathItem.Boolean.js index afc3961f..1be2e198 100644 --- a/src/path/PathItem.Boolean.js +++ b/src/path/PathItem.Boolean.js @@ -67,9 +67,7 @@ PathItem.inject(new function() { && path1.getIndex() < path2.getIndex() ? path2 : path1); // Copy over the left-hand item's style and we're done. - // TODO: Consider using Item#_clone() for this, but find a way to not - // clone children / name (content). - result.setStyle(path1._style); + result.copyAttributes(path1); return result; } @@ -861,9 +859,7 @@ PathItem.inject(new function() { item = new CompoundPath(Item.NO_INSERT); item.setChildren(paths); item = item.reduce(); - // TODO: Consider using Item#_clone() for this, but find a way to - // not clone children / name (content). - item.setStyle(this._style); + item.copyAttributes(this); this.replaceWith(item); } return item; diff --git a/src/text/PointText.js b/src/text/PointText.js index 3c4e1e4a..18e446bd 100644 --- a/src/text/PointText.js +++ b/src/text/PointText.js @@ -58,10 +58,6 @@ var PointText = TextItem.extend(/** @lends PointText# */{ TextItem.apply(this, arguments); }, - clone: function(insert) { - return this._clone(new PointText(Item.NO_INSERT), insert); - }, - /** * The PointText's anchor point * diff --git a/src/text/TextItem.js b/src/text/TextItem.js index 8aeb6168..44c373e7 100644 --- a/src/text/TextItem.js +++ b/src/text/TextItem.js @@ -50,9 +50,8 @@ var TextItem = Item.extend(/** @lends TextItem# */{ return this._content === item._content; }, - _clone: function _clone(copy, insert, includeMatrix) { - copy.setContent(this._content); - return _clone.base.call(this, copy, insert, includeMatrix); + copyContent: function(source) { + this.setContent(source._content); }, /** diff --git a/test/js/helpers.js b/test/js/helpers.js index cb2d5b03..3b7cdf07 100644 --- a/test/js/helpers.js +++ b/test/js/helpers.js @@ -39,9 +39,10 @@ function compareItem(actual, expected, message, options, properties) { QUnit.notStrictEqual(actual.id, 'not ' + expected.id, message + '.id'); QUnit.strictEqual(actual.constructor, expected.constructor, message + '.constructor'); - // When item was cloned and had a name, the name will be versioned - equals(options && options.cloned && actual.name ? actual.name + ' 1' - : actual.name, expected.name, + equals(actual.name, + // When item was cloned and had a name, the name will be versioned + options && options.cloned && expected.name + ? expected.name + ' 1' : expected.name, message + '.name'); compareProperties(actual, expected, ['children', 'bounds', 'position', 'matrix', 'data', 'opacity', 'locked', 'visible', 'blendMode', diff --git a/test/tests/Item_Cloning.js b/test/tests/Item_Cloning.js index d2076126..fbb20c74 100644 --- a/test/tests/Item_Cloning.js +++ b/test/tests/Item_Cloning.js @@ -24,7 +24,7 @@ function cloneAndCompare(item) { return copy.parent.children[copy.name] == copy; }, true); } - equals(item, copy, 'item.clone()', { cloned: true }); + equals(copy, item, 'item.clone()', { cloned: true }); // Remove the cloned item to restore the document: copy.remove(); } diff --git a/test/tests/SVGImport.js b/test/tests/SVGImport.js index 212443dc..d1c5aa68 100644 --- a/test/tests/SVGImport.js +++ b/test/tests/SVGImport.js @@ -117,5 +117,5 @@ test('Import SVG polyline', function() { test('Import complex CompoundPath and clone', function() { var svg = createSVG(';'); var item = paper.project.importSVG(svg.getElementById('path')); - equals(item, item.clone(), null, { cloned: true }); + equals(item.clone(), item, null, { cloned: true }); }); diff --git a/test/tests/Shape.js b/test/tests/Shape.js index d246874d..db9c056b 100644 --- a/test/tests/Shape.js +++ b/test/tests/Shape.js @@ -50,6 +50,6 @@ test('shape.toPath().toShape()', function() { }; Base.each(shapes, function(shape, name) { - equals(shape, shape.toPath().toShape(), name + '.toPath().toShape()'); + equals(shape.toPath().toShape(), shape, name + '.toPath().toShape()'); }); });