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

Implementing perspective, rotateX, rotateY, skewX and skewY for Android #3522

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
10 participants
@felipemartim

felipemartim commented Oct 19, 2015

Setting 3D transform properties to the Android View. The "perspective" property is a workaround and maybe needs to be thought a little better.

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Oct 19, 2015

By analyzing the blame information on this pull request, we identified @kmagiera, @browniefed and @mkonicek to be potential reviewers.

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Oct 19, 2015

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!

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Oct 19, 2015

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

@chirag04

View changes

Libraries/Utilities/MatrixMath.js Outdated
@@ -581,7 +581,7 @@ var MatrixMath = {
skew,
translation,
rotate: rotationDegrees[2],
perspectiveZ: perspective[2] != 0 ? -1/perspective[2] : 1280,

This comment has been minimized.

@chirag04

chirag04 Oct 19, 2015

Collaborator

we should keep it just perspective, just like ios. I see some perspective code already in decomposed matrix. you can pass perspective from js like:

transform: { perspective: 1280 }

This comment has been minimized.

@felipemartim

felipemartim Oct 19, 2015

I called it "perspectiveZ" because there's already a perspective array being returned on this statement. I don't know if it will be useful for something.

One thing that I couldn't manage to do is to retrieve the pixel density, as in Android View docs:

float scale = context.getResources().getDisplayMetrics().density;
view.setCameraDistance(distance * scale);

The default distance depends on the screen density. For instance, on a medium density display, the default
distance is 1280. On a high density display, the default distance is 1920."

This comment has been minimized.

@chirag04

chirag04 Oct 20, 2015

Collaborator

return value / DisplayMetricsHolder.getDisplayMetrics().density;

@chirag04

View changes

ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java Outdated
extends ViewManager<T, C> {
private static final String PROP_BACKGROUND_COLOR = ViewProps.BACKGROUND_COLOR;
private static final String PROP_DECOMPOSED_MATRIX = "decomposedMatrix";
private static final String PROP_DECOMPOSED_MATRIX_ROTATE = "rotate";
private static final String PROP_DECOMPOSED_MATRIX_PERSPECTIVE_Z = "perspectiveZ";

This comment has been minimized.

@chirag04

chirag04 Oct 19, 2015

Collaborator

keep it only perspective

@@ -30,7 +33,10 @@
private static final String PROP_IMPORTANT_FOR_ACCESSIBILITY = "importantForAccessibility";
// DEPRECATED

This comment has been minimized.

@chirag04

chirag04 Oct 19, 2015

Collaborator

@kmagiera can we take out deprecated ones just like ios?

@PhilippKrone

This comment has been minimized.

Contributor

PhilippKrone commented Oct 21, 2015

@felipemartim @chirag04 Really looking forward to have this PR merged! Can I somehow support here to bring it to a state where it can be reviewed / merged by @kmagiera?

@chirag04

This comment has been minimized.

Collaborator

chirag04 commented Oct 21, 2015

@PhilippKrone sure. feel free to review and drop comments. I would use uiexplorer perspective example and see if it works after applying this PR.

@felipemartim

This comment has been minimized.

felipemartim commented Oct 21, 2015

@PhilippKrone sure! I already identified a bug with perspective. Depending on the order of the transform properties, -1/perspective[2] can be a completely different number. This has to do with how the perspectiveMatrix is calculated. I believe it would be better to set the perspective value specified on "styles" directly to "setCameraDistance()" Android View without all that magic that's happening in "decomposeMatrix()". Another issue is to set this value correctly according to the device pixel density.

Unfortunately I don't have the time to work on this right now and this is my first PR ever, so I don't know how can I help here. Are you able to evolve this PR?

@chirag04

This comment has been minimized.

Collaborator

chirag04 commented Oct 21, 2015

@felipemartim @PhilippKrone I really want to work on this but can't get the uiexplorer to compile on master branch. I will spend some more time on it.

@kmagiera

View changes

ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java Outdated
/**
* Base class that should be suitable for the majority of subclasses of {@link ViewManager}.
* It provides support for base view properties such as backgroundColor, opacity, etc.
*/
public abstract class BaseViewManager<T extends View, C extends LayoutShadowNode>
public abstract class BaseViewManager<T extends View, C extends ReactShadowNode>

This comment has been minimized.

@kmagiera

kmagiera Oct 21, 2015

Contributor

Please revert this line

@chirag04

This comment has been minimized.

Collaborator

chirag04 commented Oct 21, 2015

I got the perspective to work with all the examples on the uiexplore transform. I will share the update tomorrow.

@kmagiera would you have ideas about how to implement backfaceVisibility of ios on android? It is implemented on ios already. I can do it if you can give some info on java side of things. Goal is to get the flip animation example working on android as well.

https://developer.mozilla.org/en-US/docs/Web/CSS/backface-visibility

@PhilippKrone

This comment has been minimized.

Contributor

PhilippKrone commented Oct 22, 2015

@chirag04 cool! thanks! :) The backfaceVisibility topic is exactly the last thing missing for me to have my animation / screen working!

@chirag04

This comment has been minimized.

Collaborator

chirag04 commented Oct 22, 2015

diff --git a/Libraries/Utilities/MatrixMath.js b/Libraries/Utilities/MatrixMath.js
index 23db738..702578f 100755
--- a/Libraries/Utilities/MatrixMath.js
+++ b/Libraries/Utilities/MatrixMath.js
@@ -423,62 +423,26 @@ var MatrixMath = {
     );

     // output values
-    var perspective = [];
     var quaternion = [];
     var scale = [];
     var skew = [];
     var translation = [];

     // create normalized, 2d array matrix
-    // and normalized 1d array perspectiveMatrix with redefined 4th column
     if (!transformMatrix[15]) {
       return;
     }
     var matrix = [];
-    var perspectiveMatrix = [];
     for (var i = 0; i < 4; i++) {
       matrix.push([]);
       for (var j = 0; j < 4; j++) {
         var value = transformMatrix[(i * 4) + j] / transformMatrix[15];
         matrix[i].push(value);
-        perspectiveMatrix.push(j === 3 ? 0 : value);
       }
     }
-    perspectiveMatrix[15] = 1;

-    // test for singularity of upper 3x3 part of the perspective matrix
-    if (!MatrixMath.determinant(perspectiveMatrix)) {
-      return;
-    }
-
-    // isolate perspective
-    if (matrix[0][3] !== 0 || matrix[1][3] !== 0 || matrix[2][3] !== 0) {
-      // rightHandSide is the right hand side of the equation.
-      // rightHandSide is a vector, or point in 3d space relative to the origin.
-      var rightHandSide = [
-        matrix[0][3],
-        matrix[1][3],
-        matrix[2][3],
-        matrix[3][3]
-      ];
-
-      // Solve the equation by inverting perspectiveMatrix and multiplying
-      // rightHandSide by the inverse.
-      var inversePerspectiveMatrix = MatrixMath.inverse3x3(
-        perspectiveMatrix
-      );
-      var transposedInversePerspectiveMatrix = MatrixMath.transpose4x4(
-        inversePerspectiveMatrix
-      );
-      var perspective = MatrixMath.multiplyVectorByMatrix(
-        rightHandSide,
-        transposedInversePerspectiveMatrix
-      );
-    } else {
-      // no perspective
-      perspective[0] = perspective[1] = perspective[2] = 0;
-      perspective[3] = 1;
-    }
+    //perspective
+    var perspective = -1 / matrix[2][3];

     // translation is simple
     for (var i = 0; i < 3; i++) {
diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java
index fbbae4c..b2d28a2 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java
@@ -6,6 +6,7 @@ import android.graphics.Color;
 import android.os.Build;
 import android.view.View;

+import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.bridge.ReadableMap;

 /**
@@ -17,7 +18,8 @@ public abstract class BaseViewManager<T extends View, C extends LayoutShadowNode

   private static final String PROP_BACKGROUND_COLOR = ViewProps.BACKGROUND_COLOR;
   private static final String PROP_DECOMPOSED_MATRIX = "decomposedMatrix";
-  private static final String PROP_DECOMPOSED_MATRIX_ROTATE = "rotate";
+  private static final String PROP_DECOMPOSED_MATRIX_PERSPECTIVE = "perspective";
+  private static final String PROP_DECOMPOSED_MATRIX_ROTATION = "rotationDegrees";
   private static final String PROP_DECOMPOSED_MATRIX_SCALE_X = "scaleX";
   private static final String PROP_DECOMPOSED_MATRIX_SCALE_Y = "scaleY";
   private static final String PROP_DECOMPOSED_MATRIX_TRANSLATE_X = "translateX";
@@ -142,12 +144,18 @@ public abstract class BaseViewManager<T extends View, C extends LayoutShadowNode
             (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_TRANSLATE_X)));
     view.setTranslationY(PixelUtil.toPixelFromDIP(
             (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_TRANSLATE_Y)));
-    view.setRotation(
-        (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_ROTATE));
     view.setScaleX(
         (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_SCALE_X));
     view.setScaleY(
         (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_SCALE_Y));
+    
+    float scale = DisplayMetricsHolder.getDisplayMetrics().density;
+    view.setCameraDistance(
+      (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_PERSPECTIVE) * scale);
+    ReadableArray rotationArray = matrix.getArray(PROP_DECOMPOSED_MATRIX_ROTATION);
+    view.setRotationX((float) rotationArray.getDouble(0));
+    view.setRotationY((float) rotationArray.getDouble(1));
+    view.setRotation((float) rotationArray.getDouble(2));
   }

   private static void resetTransformMatrix(View view) {
diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewPropertyApplicator.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewPropertyApplicator.java
index ec9117e..0613efd 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewPropertyApplicator.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewPropertyApplicator.java
@@ -16,6 +16,7 @@ import java.util.HashMap;
 import android.graphics.Color;
 import android.os.Build;
 import android.view.View;
+import com.facebook.react.bridge.ReadableArray;
 import com.facebook.react.bridge.ReadableMap;

 /**
@@ -26,7 +27,8 @@ import com.facebook.react.bridge.ReadableMap;
 public class BaseViewPropertyApplicator {

   private static final String PROP_DECOMPOSED_MATRIX = "decomposedMatrix";
-  private static final String PROP_DECOMPOSED_MATRIX_ROTATE = "rotate";
+  private static final String PROP_DECOMPOSED_MATRIX_PERSPECTIVE = "perspective";
+  private static final String PROP_DECOMPOSED_MATRIX_ROTATION = "rotationDegrees";
   private static final String PROP_DECOMPOSED_MATRIX_SCALE_X = "scaleX";
   private static final String PROP_DECOMPOSED_MATRIX_SCALE_Y = "scaleY";
   private static final String PROP_DECOMPOSED_MATRIX_TRANSLATE_X = "translateX";
@@ -130,23 +132,6 @@ public class BaseViewPropertyApplicator {
         view.setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_NO_HIDE_DESCENDANTS);
       }
     }
-
-    // DEPRECATED
-    if (props.hasKey(PROP_ROTATION)) {
-      view.setRotation(props.getFloat(PROP_ROTATION, 0));
-    }
-    if (props.hasKey(PROP_SCALE_X)) {
-      view.setScaleX(props.getFloat(PROP_SCALE_X, 1.f));
-    }
-    if (props.hasKey(PROP_SCALE_Y)) {
-      view.setScaleY(props.getFloat(PROP_SCALE_Y, 1.f));
-    }
-    if (props.hasKey(PROP_TRANSLATE_X)) {
-      view.setTranslationX(PixelUtil.toPixelFromDIP(props.getFloat(PROP_TRANSLATE_X, 0)));
-    }
-    if (props.hasKey(PROP_TRANSLATE_Y)) {
-      view.setTranslationY(PixelUtil.toPixelFromDIP(props.getFloat(PROP_TRANSLATE_Y, 0)));
-    }
   }

   private static void setTransformMatrix(View view, ReadableMap matrix) {
@@ -154,12 +139,19 @@ public class BaseViewPropertyApplicator {
       (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_TRANSLATE_X)));
     view.setTranslationY(PixelUtil.toPixelFromDIP(
       (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_TRANSLATE_Y)));
-    view.setRotation(
-      (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_ROTATE));
     view.setScaleX(
       (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_SCALE_X));
     view.setScaleY(
       (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_SCALE_Y));
+
+    float scale = DisplayMetricsHolder.getDisplayMetrics().density;
+    view.setCameraDistance(
+      (float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_PERSPECTIVE) * scale);
+    
+    ReadableArray rotationArray = matrix.getArray(PROP_DECOMPOSED_MATRIX_ROTATION);
+    view.setRotationX((float) rotationArray.getDouble(0));
+    view.setRotationY((float) rotationArray.getDouble(1));
+    view.setRotation((float) rotationArray.getDouble(2));
   }

   private static void resetTransformMatrix(View view) {

Above is the diff for getting flip. We really need to understand backfaceVisibility for android. Hoping for ideas from @kmagiera . I couldn't find anything related to backfaceVisibility on android.

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Oct 22, 2015

@felipemartim updated the pull request.

@PhilippKrone

This comment has been minimized.

Contributor

PhilippKrone commented Oct 23, 2015

@chirag04

This comment has been minimized.

Collaborator

chirag04 commented Oct 23, 2015

@PhilippKrone I saw that link and it seem it uses native animation lib. I'm not sure how we can using the js animated library to work with the native one.

If it's a blocker, I think specific cardflip animation would be possible by bridging a new package and using the native animated lib. That's what i think we are going to do.

@PhilippKrone

This comment has been minimized.

Contributor

PhilippKrone commented Oct 23, 2015

@chirag04 From how I understand it, currently no cardflip animation is possible at all in android as you will always see the backside, wont you?

quaternion,
scale,
skew,
translation,
rotate: rotationDegrees[2],
perspective: perspective[2] != 0 ? -1/perspective[2] : 1280,
scaleX: scale[0],
scaleY: scale[1],
translateX: translation[0],

This comment has been minimized.

@mkonicek

mkonicek Oct 23, 2015

Contributor

Why are the changes in this file needed? What's your test plan to make sure it doesn't break existing iOS behavior?

view.setRotationX(rotation);
}
@Deprecated

This comment has been minimized.

@mkonicek

mkonicek Oct 23, 2015

Contributor

Why Deprecated? You've just added this code.

@mkonicek

This comment has been minimized.

Contributor

mkonicek commented Oct 23, 2015

Not sure we want to merge this in the current form. In any case, features like this need an example in the UIExplorer. Is there an iOS example? Have you tried running it on Android? Provide lots of screenshots.

@maraujop

This comment has been minimized.

maraujop commented Oct 28, 2015

I was trying to do some animations today, realized rotateX and rotateY weren't working in Android and found issue #3491 and from there I got here.

I've applied current diff to my local react-native and I'm getting this exception:

captura de pantalla 2015-10-28 a las 16 51 48

I'm doing a very basic rotateX with an interpolated Animated.Value, just in case it helps. It would be wonderful to have support for this animation properties in Android.

Cheers,
Miguel

@PhilippKrone

This comment has been minimized.

Contributor

PhilippKrone commented Nov 3, 2015

@chirag04 can you make a new PR with your proposed changes? Perhaps we can get this stuff merged soon then

@PhilippKrone

This comment has been minimized.

Contributor

PhilippKrone commented Nov 3, 2015

@kmagiera Sorry for pushing this, it's kind of a show stopper for me at the moment. Do you have an idea concerning the implementation of the css attribute "backfaceVisiblity" in android? (https://developer.mozilla.org/en-US/docs/Web/CSS/backface-visibility)

@PhilippKrone

This comment has been minimized.

Contributor

PhilippKrone commented Nov 3, 2015

@chirag04 I get an exception using your diff. It seems that if matrix[2][3] is 0, then perspective is infinite, which will be handled as "null" in java. Following this, the line "view.setCameraDistance(
(float) matrix.getDouble(PROP_DECOMPOSED_MATRIX_PERSPECTIVE) * scale);" throws an exception.

@kmagiera

This comment has been minimized.

Contributor

kmagiera commented Nov 3, 2015

@PhilippKrone I'm out of the office this week so won't be able to dig into this and answer your question anytime soon. Maybe @astreet can help?

@astreet

This comment has been minimized.

Contributor

astreet commented Nov 3, 2015

I'm not very familiar with the Android 3D APIs, but I doubt anything like backface-visibility is available on the View class. You'll probably just need to mimic it (or polyfill it in JS using the standard 3D transform APIs?)

@felipemartim

This comment has been minimized.

felipemartim commented Nov 13, 2015

@PhilippKrone I managed to overcome the backfaceVisibility issue by setting the opacity of the front side of the card to 0 when the rotation reached 90º.

quaternion,
scale,
skew,
translation,
rotate: rotationDegrees[2],
perspective: perspective[2] != 0 ? -1/perspective[2] : 1280,

This comment has been minimized.

@mkonicek

mkonicek Nov 14, 2015

Contributor

Why 1280? Can you add a comment explaining this?

view.setScaleX(1);
view.setScaleY(1);
float scale = DisplayMetricsHolder.getDisplayMetrics().density;
view.setCameraDistance(1280 * scale);

This comment has been minimized.

@mkonicek

mkonicek Nov 14, 2015

Contributor

Why 1280? Can you add a comment?

@mkonicek

This comment has been minimized.

Contributor

mkonicek commented Nov 14, 2015

Can you check all of the places where MatrixMath is used? Can you verify your changes to that file don't break anything?

@@ -17,9 +18,11 @@
private static final String PROP_BACKGROUND_COLOR = ViewProps.BACKGROUND_COLOR;
private static final String PROP_DECOMPOSED_MATRIX = "decomposedMatrix";
private static final String PROP_DECOMPOSED_MATRIX_ROTATE = "rotate";
private static final String PROP_DECOMPOSED_MATRIX_PERSPECTIVE = "perspective";
private static final String PROP_DECOMPOSED_MATRIX_ROTATION = "rotationDegrees";

This comment has been minimized.

@mkonicek

mkonicek Nov 14, 2015

Contributor

Oh OK, is it rotationDegrees, rather than rotate on iOS as well?

@mbinge

This comment has been minimized.

mbinge commented Nov 27, 2015

I have the same problem. Ugly, I use state to control rotation

@satya164

This comment has been minimized.

Collaborator

satya164 commented Nov 27, 2015

Any updates on this @felipemartim ?

@felipemartim

This comment has been minimized.

felipemartim commented Nov 27, 2015

@satya164 I'll probably have to create a pull request to set rotateY, rotateX, skewX, skewY separated from perspective. I'll try to do it this weekend.

@satya164

This comment has been minimized.

Collaborator

satya164 commented Nov 28, 2015

Awesome. Thanks for working on this @felipemartim

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