Allow toggling of flipX/Y to flip the whole group

[briefbanane]

This will add a function toggle to the group class, which will redirect everything to its children. Moreover it will change the positions of the objects (mirror them respective to the group center) when flipX or flipY is the given property.

Fixes #575

[kangax]

Hey @briefbanane thanks for this. But how exactly does it fix #575 if it only introduces toggle property on fabric.Group?

[briefbanane]

@kangax Let me explain how it works: When you called toggle on a selected group the function object.toggle was called (the group had no own implementation of toggle, so it passed the call to object). If you call toggle now on a selected group, the respective function in group will be called, because of polymorphism.

You can check that this is true with adding a console.log("in object") in object.toggle and another console.log("in group") in group.toggle. Before the patch, the whole selected group was treated as a single object, resulting in one object.toggle call for the whole group. Now the group delegetes the toggle call to all its children and handles flipX and flipY in a special manner (by changing the actual position of the objects in the group). This results in one call of group.toggle and n calls of object.toggle (one for every object in the group).

I've used the following example to test it: http://jsfiddle.net/Kienz/3EAHH/3/

[kangax]

@briefbanane I see what you did there :)

Yes, we can totally take advantage of polymorphism here but there's a little twist. I got confused why you changed toggle. Now I see that you were basing off @Kienz example on jsfiddle. However the problem lies a bit deeper. Group (as any other object) can also be flipped via set('flipX', ...) or set('flipY', ...) or even group.flipX = ..., group.flipY = ....

Thankfully, toggle uses set so we would just need to move this logic to group's set. We can't do much with direct property assignments (since we support browsers that don't support getters/setters), but fixing set (and so toggle, indirectly) will already be much better.

Would you be able to update pull request accordingly?

[briefbanane]

@kangax I tried to move the logic to _set, but I found another problem.

If I change the group's flipX/Y property, the group will be drawn flipped. But if the objects are flipped and moved at the same time, the group will be drawn straight (because it's like two flips). Only after the group isn't selected anymore, the flip will be visible.

If I don't set the flipX/Y in group however, there is no way to tell wheter or not the property really changed (if somebody changes a true property to true, nothing should happen).

I'll try to move the logic to _restoreObjectState

[briefbanane]

This should work even for direct assignments.

[kangax]

Thanks @briefbanane!

I tested it out a little and notice just one problem. When one of the objects in the group is rotated, the object ends up rotated in the opposite (?) direction after restoring group objects:

Group created:

Group flipped horizontally:

Group restored:

[briefbanane]

@kangax thanks for reviewing.
I forgot to test that case, but now it should work. Obviously flipping the group also mirrors the rotations of the objects.

I needed to set the object origin to center temporarly, because otherwise setAngle would change the object position for non-centric origins. This approach makes the code a bit bloated, but the other alternative is to make trigonometric calculations respectively for every possible origin orientation, which would be even worse.

[Kienz]

@briefbanane After flipping objects horizontally (flipX) and group is restored the rotation handler is flipped too. If you flip single object rotation handler isn't flipped.

If you flip objects (flipX) and add another object to the group i think the position is not correct:

Group created:

Group flipped horizontally:

After 3rd object is added to group:

[kangax]

FWIW, we can make code a bit cleaner with object notation for setting properties.

Replacing this:

object.set('originX', 'center');
object.set('originY', 'center');
object.set('left', center.x);
object.set('top', center.y);

with:

object.set({
  originX: 'center',
  originY: 'center',
  left: center.x,
  top: center.y
});

[briefbanane]

@Kienz

After flipping objects horizontally (flipX) and group is restored the rotation handler is flipped too. If you flip single object rotation handler isn't flipped.

Fixed.

If you flip objects (flipX) and add another object to the group i think the position is not correct:

I fixed the problem that the objects already in the group were re-flipped.

The positions are not correct in flipped groups, because the group is only painted flipped. Not before the group is destroyed the positions of the objects really change. There is no trivial way of fixing this, because at least for direct assignments to flipX/Y you can't really get notified.

@kangax
I changed that.

[kangax]

Great, thanks @briefbanane! This is already miles better than what we had so I think we should merge it.

@Kienz?

[Kienz]

@kangax Let's merge it. We can create new issue for the problem with incorrect position after adding new object to existing flipped group.

[coveralls]

Changes Unknown when pulling 44756d4 on briefbanane:master into * on kangax:master*.