feat(Group): render selected objects

src/brushes/spray_brush.class.js

@@ -112,7 +112,7 @@ fabric.SprayBrush = fabric.util.createClass( fabric.BaseBrush, /** @lends fabric
       rects = this._getOptimizedRects(rects);
     }
 
-    var group = new fabric.Group(rects);
+    var group = new fabric.Group(rects, { objectCaching: true, layout: 'fixed', subTargetCheck: false });
     this.shadow && group.set('shadow', new fabric.Shadow(this.shadow));
     this.canvas.fire('before:path:created', { path: group });
     this.canvas.add(group);

src/canvas.class.js

@@ -873,7 +873,7 @@
           this._normalizePointer(objToCheck.group, pointer) : pointer;
         if (this._checkTarget(pointerToUse, objToCheck, pointer)) {
           target = objects[i];
-          if (target.subTargetCheck && target instanceof fabric.Group) {
+          if (target.subTargetCheck && Array.isArray(target._objects)) {
             subTarget = this._searchPossibleTargets(target._objects, pointer);
             subTarget && this.targets.push(subTarget);
           }
@@ -892,7 +892,7 @@
      */
     searchPossibleTargets: function (objects, pointer) {
       var target = this._searchPossibleTargets(objects, pointer);
-      return target;
+      return target && target.interactive && this.targets[0] ? this.targets[0] : target;
     },
 
     /**

src/mixins/animation.mixin.js

@@ -25,11 +25,11 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
 
     return fabric.util.animate({
       target: this,
-      startValue: object.left,
+      startValue: object.getX(),
       endValue: this.getCenterPoint().x,
       duration: this.FX_DURATION,
       onChange: function(value) {
-        object.set('left', value);
+        object.setX(value);
         _this.requestRenderAll();
         onChange();
       },
@@ -58,11 +58,11 @@ fabric.util.object.extend(fabric.StaticCanvas.prototype, /** @lends fabric.Stati
 
     return fabric.util.animate({
       target: this,
-      startValue: object.top,
+      startValue: object.getY(),
       endValue: this.getCenterPoint().y,
       duration: this.FX_DURATION,
       onChange: function(value) {
-        object.set('top', value);
+        object.setY(value);
         _this.requestRenderAll();
         onChange();
       },

src/mixins/canvas_events.mixin.js

@@ -712,6 +712,7 @@
           }
         }
       }
+      this._objectsToRender = this._chooseObjectsToRender();
       this._handleEvent(e, 'down');
       // we must renderAll so that we update the visuals
       (shouldRender || shouldGroup) && this.requestRenderAll();
@@ -900,13 +901,19 @@
      */
     _transformObject: function(e) {
       var pointer = this.getPointer(e),
-          transform = this._currentTransform;
+          transform = this._currentTransform,
+          target = transform.target,
+          //  transform pointer to target's containing coordinate plane
+          //  both pointer and object should agree on every point
+          localPointer = target.group ?
+            fabric.util.sendPointToPlane(pointer, null, target.group.calcTransformMatrix()) :
+            pointer;
 
       transform.reset = false;
       transform.shiftKey = e.shiftKey;
       transform.altKey = e[this.centeredKey];
 
-      this._performTransformAction(e, transform, pointer);
+      this._performTransformAction(e, transform, localPointer);
       transform.actionPerformed && this.requestRenderAll();
     },
 

src/mixins/canvas_grouping.mixin.js

@@ -13,8 +13,12 @@
      */
     _shouldGroup: function(e, target) {
       var activeObject = this._activeObject;
-      return activeObject && this._isSelectionKeyPressed(e) && target && target.selectable && this.selection &&
-            (activeObject !== target || activeObject.type === 'activeSelection') && !target.onSelect({ e: e });
+      return !!activeObject && this._isSelectionKeyPressed(e)
+        && !!target && target.selectable && this.selection
+        && (activeObject !== target || activeObject.type === 'activeSelection')
+        //  make sure `activeObject` and `target` aren't ancestors of each other
+        && !target.isDescendantOf(activeObject) && !activeObject.isDescendantOf(target)
+        && !target.onSelect({ e: e });
     },
 
     /**
@@ -50,8 +54,8 @@
     _updateActiveSelection: function(target, e) {
       var activeSelection = this._activeObject,
           currentActiveObjects = activeSelection._objects.slice(0);
-      if (activeSelection.contains(target)) {
-        activeSelection.removeWithUpdate(target);
+      if (target.group === activeSelection) {
+        activeSelection.remove(target);
         this._hoveredTarget = target;
         this._hoveredTargets = this.targets.concat();
         if (activeSelection.size() === 1) {
@@ -60,7 +64,7 @@
         }
       }
       else {
-        activeSelection.addWithUpdate(target);
+        activeSelection.add(target);
         this._hoveredTarget = activeSelection;
         this._hoveredTargets = this.targets.concat();
       }
@@ -80,17 +84,19 @@
       this._fireSelectionEvents(currentActives, e);
     },
 
+
     /**
      * @private
      * @param {Object} target
+     * @returns {fabric.ActiveSelection}
      */
     _createGroup: function(target) {
-      var objects = this._objects,
-          isActiveLower = objects.indexOf(this._activeObject) < objects.indexOf(target),
-          groupObjects = isActiveLower
-            ? [this._activeObject, target]
-            : [target, this._activeObject];
-      this._activeObject.isEditing && this._activeObject.exitEditing();
+      var activeObject = this._activeObject;
+      var groupObjects = target.isInFrontOf(activeObject) ?
+        [activeObject, target] :
+        [target, activeObject];
+      activeObject.isEditing && activeObject.exitEditing();
+      //  handle case: target is nested
       return new fabric.ActiveSelection(groupObjects, {
         canvas: this
       });

src/mixins/collection.mixin.js

@@ -3,79 +3,69 @@
  */
 fabric.Collection = {
 
+  /**
+   * @type {fabric.Object[]}
+   */
   _objects: [],
 
   /**
    * Adds objects to collection, Canvas or Group, then renders canvas
    * (if `renderOnAddRemove` is not `false`).
-   * in case of Group no changes to bounding box are made.
    * Objects should be instances of (or inherit from) fabric.Object
-   * Use of this function is highly discouraged for groups.
-   * you can add a bunch of objects with the add method but then you NEED
-   * to run a addWithUpdate call for the Group class or position/bbox will be wrong.
-   * @param {...fabric.Object} object Zero or more fabric instances
-   * @return {Self} thisArg
-   * @chainable
+   * @private
+   * @param {fabric.Object[]} objects to add
+   * @param {(object:fabric.Object) => any} [callback]
+   * @returns {number} new array length
    */
-  add: function () {
-    this._objects.push.apply(this._objects, arguments);
-    if (this._onObjectAdded) {
-      for (var i = 0, length = arguments.length; i < length; i++) {
-        this._onObjectAdded(arguments[i]);
+  add: function (objects, callback) {
+    var size = this._objects.push.apply(this._objects, objects);
+    if (callback) {
+      for (var i = 0, length = objects.length; i < length; i++) {
+        callback.call(this, objects[i]);
       }
     }
-    this.renderOnAddRemove && this.requestRenderAll();
-    return this;
+    return size;
   },
 
   /**
    * Inserts an object into collection at specified index, then renders canvas (if `renderOnAddRemove` is not `false`)
    * An object should be an instance of (or inherit from) fabric.Object
-   * Use of this function is highly discouraged for groups.
-   * you can add a bunch of objects with the insertAt method but then you NEED
-   * to run a addWithUpdate call for the Group class or position/bbox will be wrong.
-   * @param {Object} object Object to insert
+   * @private
+   * @param {fabric.Object|fabric.Object[]} objects Object(s) to insert
    * @param {Number} index Index to insert object at
-   * @param {Boolean} nonSplicing When `true`, no splicing (shifting) of objects occurs
-   * @return {Self} thisArg
-   * @chainable
+   * @param {(object:fabric.Object) => any} [callback]
    */
-  insertAt: function (object, index, nonSplicing) {
-    var objects = this._objects;
-    if (nonSplicing) {
-      objects[index] = object;
-    }
-    else {
-      objects.splice(index, 0, object);
+  insertAt: function (objects, index, callback) {
+    var args = [index, 0].concat(objects);
+    this._objects.splice.apply(this._objects, args);
+    if (callback) {
+      for (var i = 2, length = args.length; i < length; i++) {
+        callback.call(this, args[i]);
+      }
     }
-    this._onObjectAdded && this._onObjectAdded(object);
-    this.renderOnAddRemove && this.requestRenderAll();
-    return this;
   },
 
   /**
    * Removes objects from a collection, then renders canvas (if `renderOnAddRemove` is not `false`)
-   * @param {...fabric.Object} object Zero or more fabric instances
-   * @return {Self} thisArg
-   * @chainable
+   * @private
+   * @param {fabric.Object[]} objectsToRemove objects to remove
+   * @param {(object:fabric.Object) => any} [callback] function to call for each object removed
+   * @returns {boolean} true if objects were removed
    */
-  remove: function() {
+  remove: function(objectsToRemove, callback) {
     var objects = this._objects,
         index, somethingRemoved = false;
 
-    for (var i = 0, length = arguments.length; i < length; i++) {
-      index = objects.indexOf(arguments[i]);
-
+    for (var i = 0, length = objectsToRemove.length; i < length; i++) {
+      index = objects.indexOf(objectsToRemove[i]);
       // only call onObjectRemoved if an object was actually removed
       if (index !== -1) {
         somethingRemoved = true;
         objects.splice(index, 1);
-        this._onObjectRemoved && this._onObjectRemoved(arguments[i]);
+        callback && callback.call(this, objectsToRemove[i]);
       }
     }
-
-    this.renderOnAddRemove && somethingRemoved && this.requestRenderAll();
-    return this;
+    return somethingRemoved;
   },
 
   /**
@@ -100,17 +90,16 @@ fabric.Collection = {
 
   /**
    * Returns an array of children objects of this instance
-   * Type parameter introduced in 1.3.10
-   * since 2.3.5 this method return always a COPY of the array;
-   * @param {String} [type] When specified, only objects of this type are returned
+   * @param {...String} [types] When specified, only objects of these types are returned
    * @return {Array}
    */
-  getObjects: function(type) {
-    if (typeof type === 'undefined') {
+  getObjects: function() {
+    if (arguments.length === 0) {
       return this._objects.concat();
     }
-    return this._objects.filter(function(o) {
-      return o.type === type;
+    var types = Array.from(arguments);
+    return this._objects.filter(function (o) {
+      return types.indexOf(o.type) > -1;
     });
   },
 
@@ -140,7 +129,8 @@ fabric.Collection = {
   },
 
   /**
-   * Returns true if collection contains an object
+   * Returns true if collection contains an object.\
+   * **Prefer using {@link `fabric.Object#isDescendantOf`} for performance reasons**
    * @param {Object} object Object to check against
    * @param {Boolean} [deep=false] `true` to check all descendants, `false` to check only `_objects`
    * @return {Boolean} `true` if collection contains an object

src/mixins/eraser_brush.mixin.js

@@ -125,7 +125,6 @@
     /* _TO_SVG_END_ */
   });
 
-  var __restoreObjectsState = fabric.Group.prototype._restoreObjectsState;
   fabric.util.object.extend(fabric.Group.prototype, {
     /**
      * @private
@@ -181,15 +180,6 @@
               });
           }
         });
-    },
-
-    /**
-     * Propagate the group's eraser to its objects, crucial for proper functionality of the eraser within the group and nested objects.
-     * @private
-     */
-    _restoreObjectsState: function () {
-      this.erasable === true && this.applyEraserToObjects();
-      return __restoreObjectsState.call(this);
     }
   });
 
@@ -217,23 +207,21 @@
      */
     originY: 'center',
 
-    drawObject: function (ctx) {
-      ctx.save();
-      ctx.fillStyle = 'black';
-      ctx.fillRect(-this.width / 2, -this.height / 2, this.width, this.height);
-      ctx.restore();
-      this.callSuper('drawObject', ctx);
-    },
-
     /**
      * eraser should retain size
      * dimensions should not change when paths are added or removed
      * handled by {@link fabric.Object#_drawClipPath}
      * @override
      * @private
      */
-    _getBounds: function () {
-      //  noop
+    layout: 'fixed',
+
+    drawObject: function (ctx) {
+      ctx.save();
+      ctx.fillStyle = 'black';
+      ctx.fillRect(-this.width / 2, -this.height / 2, this.width, this.height);
+      ctx.restore();
+      this.callSuper('drawObject', ctx);
     },
 
     /* _TO_SVG_START_ */

src/mixins/itext_click_behavior.mixin.js

@@ -152,7 +152,7 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
    */
   mouseUpHandler: function(options) {
     this.__isMousedown = false;
-    if (!this.editable || this.group ||
+    if (!this.editable ||
       (options.transform && options.transform.actionPerformed) ||
       (options.e.button && options.e.button !== 1)) {
       return;