fix group initialization

fabricjs · Jun 2, 2015 · e07350c · e07350c
1 parent f0fcd77
commit e07350c
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 16 deletions.
diff --git a/src/shapes/group.class.js b/src/shapes/group.class.js
@@ -52,11 +52,19 @@
      * Constructor
      * @param {Object} objects Group objects
      * @param {Object} [options] Options object
+     * @param {Boolean} [isAlreadyGrouped] if true, objects have been grouped already.
      * @return {Object} thisArg
      */
-    initialize: function(objects, options) {
+    initialize: function(objects, options, isAlreadyGrouped) {
       options = options || { };
 
+      this._objects = [];
+      // if objects enclosed in a group have been grouped already,
+      // we cannot change properties of objects.
+      // Thus we need to set options to group without objects,
+      // because delegatedProperties propagate to objects.
+      isAlreadyGrouped && this.callSuper('initialize', options);
+
       this._objects = objects || [];
       for (var i = this._objects.length; i--; ) {
         this._objects[i].group = this;
@@ -67,31 +75,49 @@
       if (options.originX) {
         this.originX = options.originX;
       }
-
       if (options.originY) {
         this.originY = options.originY;
       }
 
-      this._calcBounds();
-      this._updateObjectsCoords();
-
-      this.callSuper('initialize', options);
+      if (isAlreadyGrouped) {
+        // do not change coordinate of objects enclosed in a group,
+        // because objects coordinate system have been group coodinate system already.
+        this._updateObjectsCoords(true);
+      }
+      else {
+        this._calcBounds();
+        this._updateObjectsCoords();
+        this.callSuper('initialize', options);
+      }
 
       this.setCoords();
       this.saveCoords();
     },
 
     /**
      * @private
+     * @param {Boolean} [skipCoordsChange] if true, coordinates of objects enclosed in a group do not change
      */
-    _updateObjectsCoords: function() {
-      this.forEachObject(this._updateObjectCoords, this);
+    _updateObjectsCoords: function(skipCoordsChange) {
+      for (var i = this._objects.length; i--; ){
+        this._updateObjectCoords(this._objects[i], skipCoordsChange);
+      }
     },
 
     /**
      * @private
+     * @param {Object} object
+     * @param {Boolean} [skipCoordsChange] if true, coordinates of object dose not change
      */
-    _updateObjectCoords: function(object) {
+    _updateObjectCoords: function(object, skipCoordsChange) {
+      // do not display corners of objects enclosed in a group
+      object.__origHasControls = object.hasControls;
+      object.hasControls = false;
+
+      if (skipCoordsChange) {
+        return;
+      }
+
       var objectLeft = object.getLeft(),
           objectTop = object.getTop(),
           center = this.getCenterPoint();
@@ -102,12 +128,7 @@
         left: objectLeft - center.x,
         top: objectTop - center.y
       });
-
       object.setCoords();
-
-      // do not display corners of objects enclosed in a group
-      object.__origHasControls = object.hasControls;
-      object.hasControls = false;
     },
 
     /**
@@ -558,7 +579,7 @@
   fabric.Group.fromObject = function(object, callback) {
     fabric.util.enlivenObjects(object.objects, function(enlivenedObjects) {
       delete object.objects;
-      callback && callback(new fabric.Group(enlivenedObjects, object));
+      callback && callback(new fabric.Group(enlivenedObjects, object, true));
     });
   };
 

diff --git a/test/unit/group.js b/test/unit/group.js
@@ -16,6 +16,13 @@
     return new fabric.Group([ rect1, rect2 ], {strokeWidth: 0});
   }
 
+  function makeGroupWith2ObjectsWithOpacity() {
+    var rect1 = new fabric.Rect({ top: 100, left: 100, width: 30, height: 10, strokeWidth: 0, opacity: 0.5 }),
+        rect2 = new fabric.Rect({ top: 120, left: 50, width: 10, height: 40, strokeWidth: 0, opacity: 0.8 });
+
+    return new fabric.Group([ rect1, rect2 ], {strokeWidth: 0});
+  }
+
   function makeGroupWith4Objects() {
     var rect1 = new fabric.Rect({ top: 100, left: 100, width: 30, height: 10 }),
         rect2 = new fabric.Rect({ top: 120, left: 50, width: 10, height: 40 }),
@@ -363,7 +370,7 @@ test('toObject without default values', function() {
   });
 
   asyncTest('fromObject', function() {
-    var group = makeGroupWith2Objects();
+    var group = makeGroupWith2ObjectsWithOpacity();
 
     ok(typeof fabric.Group.fromObject == 'function');
     var groupObject = group.toObject();
@@ -375,6 +382,9 @@ test('toObject without default values', function() {
 
       ok(newGroupFromObject instanceof fabric.Group);
 
+      deepEqual(objectFromOldGroup.objects[0], objectFromNewGroup.objects[0]);
+      deepEqual(objectFromOldGroup.objects[1], objectFromNewGroup.objects[1]);
+
       // delete `objects` arrays, since `assertHashEqual` fails to compare them for equality
       delete objectFromOldGroup.objects;
       delete objectFromNewGroup.objects;