Skip to content

fixed skew transformations - #11886

Closed
MaxGraey wants to merge 8 commits into
react:masterfrom
MaxGraey:master
Closed

fixed skew transformations#11886
MaxGraey wants to merge 8 commits into
react:masterfrom
MaxGraey:master

Conversation

@MaxGraey

@MaxGraeyMaxGraey commented Jan 13, 2017

Copy link
Copy Markdown
Contributor

Motivation:
fix#11884 issue

@sahrens@vjeux

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 14, 2017
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@vjeux

Copy link
Copy Markdown
Contributor

@jordwalke you wrote that code, could you review it please? Thanks!

@MaxGraey

MaxGraey commented Jan 19, 2017

Copy link
Copy Markdown
ContributorAuthor

Hi, @jordwalke. I will try in short to explain what was wrong.
Let's look to transformation's matrix for skewY for example.

Was

cos(a)sin(a)00
0100
0010
txtytz1

Yes, this visually produce skewing transform but it slightly incorrect. This way affects horizontal scale as well. See this

Now with PR

1tan(a)00
0100
0010
txtytz1

According to www.w3.org/css-transforms

Only one differance React Native use row major matrix style, so we change m[0][1] instead m[1][0].

@facebook-github-botfacebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jan 19, 2017
@vjeux

Copy link
Copy Markdown
Contributor

Thanks for the thorough explanation, that sounds good to me. I just clicked the land button, so unless something goes wrong, it's going to ship in the next tens of minutes! Thanks!

@facebook-github-botfacebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jan 19, 2017
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Something went wrong when importing this pull request. Please cc someone from the team at fb to help with importing this.

This reverts commit cca743c, reversing
changes made to 7aefe36.
# Conflicts:
#	Libraries/ReactNative/YellowBox.js
#	docs/Debugging.md
warn.apply(console, arguments);
updateWarningMap.apply(null, arguments);
};
<<<<<<< HEAD

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think something went wrong with your rebase

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@vjeux My last fork was 5 days ago, so I try to remote update to actual base, but anyway I still have unresolved 2 conflicts. What would you recommend?

@vjeux

Copy link
Copy Markdown
Contributor

Since your diff is so small ( 7aefe36 ), I would recommend removing this whole branch, applying it back manually and sending a force update. The git commands should look something like:

gitcheckoutorigin/master;gitbranch-Dmaster;gitcheckout-bmaster;// apply the changesgitcommit// copy paste the description from githubgitpush-fMaxGraey

In the future, I recommend that you create a branch with

gitcheckout-bfix_skew

this way you don't need to work on master and you can have multiple of them going on.

@vjeux

Copy link
Copy Markdown
Contributor

Looks like there are still a lot of spurious changes :(

@MaxGraey

Copy link
Copy Markdown
ContributorAuthor

@vjeux I choose second approach

New PR from fix-skew

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transform 'skewY' affect to dimensions

3 participants

@MaxGraey@facebook-github-bot@vjeux