Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Handle group transformation with skew and rotate + scale #2549

Merged
merged 3 commits into from
Oct 24, 2015
Merged

[WIP] Handle group transformation with skew and rotate + scale #2549

merged 3 commits into from
Oct 24, 2015

Conversation

asturur
Copy link
Member

@asturur asturur commented Oct 14, 2015

This is a WIP.
it is fully working excluding when we involve flipping in transformations.

I upload a demo as soon as i have time.
The strange thing is that everything works fine, but when i REGROUP items after ungrouping some flipping appears.

closes #726
closes #2515
closes #2297
closes #833

@asturur
Copy link
Member Author

asturur commented Oct 14, 2015

_CONTROL BOXES WILL BE FIXED LATER_
Usual demo here:
http://www.deltalink.it/andreab/fabric/kitchensink.html

// clear canvas
canvas.clear();

// remove currently selected object
canvas.remove(canvas.getActiveObject());

// add red rectangle
canvas.add(new fabric.Rect({
  width: 50,
  height: 50,
  left: 50,
  top: 50,
  fill: 'rgb(255,255,0)'
}));

// add red rectangle
canvas.add(new fabric.Rect({
  width: 50,
  height: 50,
  left: 50,
  top: 50,
  fill: 'rgb(255,0,255)'
}));

// add red rectangle
canvas.add(new fabric.Rect({
  width: 50,
  height: 50,
  left: 50,
  top: 50,
  fill: 'rgb(0,0,255)'
}));

// add red rectangle
canvas.add(new fabric.Rect({
  width: 50,
  height: 50,
  left: 50,
  top: 50,
  fill: 'rgb(0,255,0)'
}));

-Create 4 rectangle ( free code given :) )
-move them, scale, rotate, skew them at will
-group them with mouse selection
-rotate group
-destroy group clicking outside
-regroup
-scale, skew, rotate [optionally flip group] at will
-destroy group clicking outside
everything is ok.

if you flip group and destroy group, everythings looks fine.
click every single object, still everything fine.
Regroup the objects and they will get some bad change.

it involves flipping and negative scaling, but i cannot figure out why.
Any help is appreciated.

@asturur
Copy link
Member Author

asturur commented Oct 14, 2015

issues described here no loger exist.

@asturur
Copy link
Member Author

asturur commented Oct 18, 2015

controlboxes fixed, active group now behaves very good. Also adding an object to an already transformed active group looks fine.

@kangax
Copy link
Member

kangax commented Oct 18, 2015

@asturur what else is left?

@asturur
Copy link
Member Author

asturur commented Oct 18, 2015

Code needs optimizations and caching. But this we can do later after we are sure everything is working properly. Transformations now more general and work in most situation. Any i could test.

Also in drawBorders, drawControls you can check i added a "sign" var. Consider this var as a temp patch because i could not figure out what was happening with negative scaling. comment the code if necessary, i will then correct and squeeze commits

@asturur
Copy link
Member Author

asturur commented Oct 18, 2015

If it is green you can merge @kangax.
I did not notice any regression with final code, but i had many while coding. So i would be suprised if there is none.
I'm very satisfied with those changes!

@asturur
Copy link
Member Author

asturur commented Oct 18, 2015

ready to merge

var matrix = object.calcTransformMatrix(),
options = fabric.util.qrDecompose(matrix),
center = new fabric.Point(options.translateX, options.translateY);
object.scaleX = options.scaleX;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move all these calls into 1 method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you mean object.set(options) , no it does not work. Scale is often negative and is triggering an automatic flip change that creates a random flip when regrouping the objects.
I think is part of optmization possible, but as of now it would just stop it from working.

@asturur
Copy link
Member Author

asturur commented Oct 19, 2015

Wait, i found some glitches!

@asturur
Copy link
Member Author

asturur commented Oct 19, 2015

This is working in group:
test1

This is working alone:
bb

the second gif "looks" good, but corners coordinates are handled differently and then are not clickable anymore.
But if @kangax like more the new behaviour, fixing corner coords is easy. It is just a matter of choice.

@uimos
Copy link

uimos commented Oct 19, 2015

This is cool. I have one question, will this work with image object?

@asturur
Copy link
Member Author

asturur commented Oct 19, 2015

of course. it works with any object. try it on:

http://www.deltalink.it/andreab/fabric/kitchensink.html

images must be added by execute tab or using the load json tab.

@VictorHagsand
Copy link
Contributor

Any timeframe for this great fix?

@asturur
Copy link
Member Author

asturur commented Oct 23, 2015

i hope weekend!

@VictorHagsand
Copy link
Contributor

Sounds great, this is a very needed fix, great job on it : )
Any one know anything about when this might show up in a release?

@asturur
Copy link
Member Author

asturur commented Oct 24, 2015

@kangax ready to merge.

kangax added a commit that referenced this pull request Oct 24, 2015
[WIP] Handle group transformation with skew and rotate + scale
@kangax kangax merged commit faf18bf into fabricjs:master Oct 24, 2015
@asturur asturur deleted the fix-group-transform branch October 24, 2015 23:50
@uimos
Copy link

uimos commented Oct 25, 2015

Thanks for the update. Do I need to initial something in code so that the skewing will work? Currently I'm on version 1.6.0-rc.1 but not able to get the skewing work.

@asturur
Copy link
Member Author

asturur commented Oct 25, 2015

skew by pressing shift button and using middle controls on side. or by code using skewX and skewY properties

@uimos
Copy link

uimos commented Oct 25, 2015

Hi @asturur , pressing shift didn't work in my code though. I tried to use jsfiddle (I manually upload 1.6.0-rc.1 js file), the shift key work. but the coordinate seem off for me. I still not sure why it work in jsfiddle but not my code though. hmm.

Here is the sample:
http://jsfiddle.net/uimos/wxte50gg/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants