Implement an efficient mechanism to prioritize key in Item#set()

Closes #1096

src/core/Base.js

@@ -78,17 +78,20 @@ Base.inject(/** @lends Base# */{
     },
 
     /**
-     * #_set() is part of the mechanism for constructors which take one object
+     * #set() is part of the mechanism for constructors which take one object
      * literal describing all the properties to be set on the created instance.
      * Through {@link Base.filter()} it supports `_filtered`
      * handling as required by the {@link Base.readNamed()} mechanism.
      *
      * @param {Object} props an object describing the properties to set
-     * @return {Boolean} {@true if the object is a plain object}
+     * @param {Object} [exclude] an object that can define any properties as
+     *     `true` that should be excluded
+     * @return {Object} a reference to `this`, for chainability.
      */
-    _set: function(props) {
+    set: function(props, exclude) {
-        if (props && Base.isPlainObject(props))
+        if (props)
-            return Base.filter(this, props);
+            Base.filter(this, props, exclude, this._prioritize);
+        return this;
     },
 
     statics: /** @lends Base */{
@@ -276,14 +279,14 @@ Base.inject(/** @lends Base# */{
             var value = this.getNamed(list, name),
                 hasObject = value !== undefined;
             if (hasObject) {
-                // Create a _filtered object that inherits from argument 0, and
+                // Create a _filtered object that inherits from list[0], and
                 // override all fields that were already read with undefined.
                 var filtered = list._filtered;
                 if (!filtered) {
                     filtered = list._filtered = Base.create(list[0]);
-                    // Point _filtering to the original so Base#_set() can
+                    // Point _unfiltered to the original so Base#_set() can
                     // execute hasOwnProperty on it.
-                    filtered._filtering = list[0];
+                    filtered._unfiltered = list[0];
                 }
                 // delete wouldn't work since the masked parent's value would
                 // shine through.
@@ -319,17 +322,24 @@ Base.inject(/** @lends Base# */{
         /**
          * Copies all properties from `source` over to `dest`, supporting
          * `_filtered` handling as required by {@link Base.readNamed()}
-         * mechanism, as well as an optional exclude` object that lists
+         * mechanism, as well as a way to exclude and prioritize properties.
-         * properties to exclude.
+         *
+         * @param {Object} dest the destination that is to receive the
+         *     properties
+         * @param {Object} source the source from where to retrieve the
+         *     properties to be copied
+         * @param {Object} [exclude] an object that can define any properties
+         *     as `true` that should be excluded when copying
+         * @param {String[]} [prioritize] a list of keys that should be
+         *     prioritized when copying, if they are defined in `source`,
+         *     processed in the order of appearance
          */
-        filter: function(dest, source, exclude) {
+        filter: function(dest, source, exclude, prioritize) {
-            // If source is a filtering object, we need to get the keys from the
+            var processed;
-            // the original object (it's parent / prototype). See _filtered
+
-            // inheritance trick in the argument reading code.
+            function handleKey(key) {
-            var keys = Object.keys(source._filtering || source);
+                if (!(exclude && key in exclude) &&
-            for (var i = 0, l = keys.length; i < l; i++) {
+                    !(processed && key in processed)) {
-                var key = keys[i];
-                if (!(exclude && exclude[key])) {
                     // Due to the _filtered inheritance trick, undefined is used
                     // to mask already consumed named arguments.
                     var value = source[key];
@@ -337,6 +347,25 @@ Base.inject(/** @lends Base# */{
                         dest[key] = value;
                 }
             }
+
+            // If there are prioritized keys, process them first.
+            if (prioritize) {
+                var keys = {};
+                prioritize.forEach(function(key) {
+                    if (key in source) {
+                        handleKey(key);
+                        keys[key] = true;
+                    }
+                });
+                // Now reference the processed keys as processed, so that
+                // handleKey() will not set them again below.
+                processed = keys;
+            }
+
+            // If source is a filtered object, we get the keys from the
+            // the original object (it's parent / prototype). See _filtered
+            // inheritance trick in the argument reading code.
+            Object.keys(source._unfiltered || source).forEach(handleKey);
             return dest;
         },
 

src/item/Item.js

@@ -79,7 +79,9 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
         clipMask: false,
         selected: false,
         data: {}
-    }
+    },
+    // Prioritize `applyMatrix` over `matrix`:
+    _prioritize: ['applyMatrix']
 },
 new function() { // Injection scope for various item event handlers
     var handlers = ['onMouseDown', 'onMouseUp', 'onMouseDrag', 'onClick',
@@ -161,9 +163,8 @@ new function() { // Injection scope for various item event handlers
         }
         // Filter out Item.NO_INSERT before _set(), for performance reasons.
         if (hasProps && props !== Item.NO_INSERT) {
-            // Filter out internal, insert, parent and project properties as
+            this.set(props, {
-            // these were handled above.
+                // Filter out these properties as they were handled above:
-            Base.filter(this, props, {
                 internal: true, insert: true, project: true, parent: true
             });
         }
@@ -243,6 +244,8 @@ new function() { // Injection scope for various item event handlers
      * values defined in the object literal, if the item has property of the
      * given name (or a setter defined for it).
      *
+     * @name Item#set
+     * @function
      * @param {Object} props
      * @return {Item} the item itself
      *
@@ -260,11 +263,6 @@ new function() { // Injection scope for various item event handlers
      *     selected: true
      * });
      */
-    set: function(props) {
-        if (props)
-            this._set(props);
-        return this;
-    },
 
     /**
      * The unique id of the item.

src/item/Raster.js

@@ -28,6 +28,8 @@ var Raster = Item.extend(/** @lends Raster# */{
         crossOrigin: null, // NOTE: Needs to be set before source to work!
         source: null
     },
+    // Prioritize `crossOrigin` over `source`:
+    _prioritize: ['crossOrigin'],
 
     // TODO: Implement type, width, height.
     // TODO: Have SymbolItem & Raster inherit from a shared class?

src/style/Gradient.js

@@ -68,7 +68,8 @@ var Gradient = Base.extend(/** @lends Gradient# */{
     initialize: function Gradient(stops, radial) {
         // Use UID here since Gradients are exported through dictionary.add().
         this._id = UID.get();
-        if (stops && this._set(stops)) {
+        if (stops && Base.isPlainObject(stops)) {
+            this.set(stops);
             // Erase arguments since we used the passed object instead.
             stops = radial = null;
         }

src/tool/Tool.js

@@ -56,7 +56,7 @@ var Tool = PaperScopeItem.extend(/** @lends Tool# */{
         // -1 so first event is 0:
         this._moveCount = -1;
         this._downCount = -1;
-        this._set(props);
+        this.set(props);
     },
 
     /**

test/tests/Group.js

@@ -48,6 +48,9 @@ test('new Group({children:[item]})', function() {
     equals(function() {
         return paper.project.activeLayer.children.length;
     }, 1);
+    equals(function() {
+        return path.parent == group;
+    }, true);
     equals(function() {
         return group.children[0] == path;
     }, true);

test/tests/Item.js

@@ -787,10 +787,11 @@ test('Item#applyMatrix', function() {
 
     var path = new Path.Rectangle({
         size: [100, 100],
-        position: [0, 0],
+        position: [0, 0]
-        applyMatrix: false
     });
 
+    path.applyMatrix = false;
+
     equals(path.matrix, new Matrix(),
             'path.matrix before scaling');
     equals(path.bounds, new Rectangle(-50, -50, 100, 100),