Add backface-visibility support on Android #15970

charpeni · 2017-09-16T19:00:09Z

backfaceVisibility was only available on iOS and 3D transformations were lacking on Android.

Backface Visibility is computed from ~~the decomposed matrix of transform prop~~ the view with their rotation degree values.

~~MatrixDecompositionContext properties have been made public so we can access to decomposed matrix values from outside (ReactViewGroup).~~

I'm not sure if this is the best implementation, so if it's not let's discuss it.

cc @janicduplessis @foghina

Test Plan

Tested in https://github.com/charpeni/react-native-backface-visibility.

Before	Now

RNTester

iOS	Android

Release Notes

[ANDROID] [FEATURE] [ReactViewGroup] - Add backface-visibility support on Android

pull-bot · 2017-09-16T19:07:19Z

@facebook-github-bot label Core Team

Generated by 🚫 dangerJS

elicwhite · 2017-09-22T21:01:31Z

I could imagine this would be a good thing to have screenshot tests for on both ios and android to ensure it doesn't break in the future.

chirag04 · 2017-09-22T21:15:04Z

i think there is an example for ios in RNTester. @charpeni would be nice to get that in for android too

janicduplessis · 2017-10-14T21:40:17Z

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java

+    setBackfaceVisibilityDependantOpacity();
+  }
+
+  public void decomposeMatrix(ReadableArray matrix) {


Could we just use View#getRotationX and View#getRotationY instead of keeping our own transform info here?

Good point!

janicduplessis · 2017-10-14T21:40:59Z

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java

+    float rotationX = (float) sMatrixDecompositionContext.rotationDegrees[0];
+    float rotationY = (float) sMatrixDecompositionContext.rotationDegrees[1];
+
+    boolean isFrontfaceVisible = (rotationX >= -90.f && rotationX < 90.f) &&


Could a negative scale affect the backface visibility?

Negative degrees are handled.

flipAnimation.interpolate({ inputRange: [0, 180], outputRange: ['0deg', '180deg'], });

flipAnimation.interpolate({ inputRange: [0, 180], outputRange: ['0deg', '-180deg'], });

janicduplessis · 2017-10-14T21:41:59Z

Yea screenshot test for this would amazing

facebook-github-bot · 2017-10-22T17:03:50Z

@charpeni I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

stale · 2017-12-21T17:08:13Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

charpeni · 2017-12-21T17:15:16Z

I'll do the requested changes.

charpeni · 2018-03-05T20:13:00Z

@chirag04 The example was already there for both platforms, I've updated the PR with RNTester screenshots.

@TheSavior @janicduplessis I can find snapshot tests, but I can't find anything related to screenshot tests, is this setup internally?

elicwhite · 2018-03-05T20:53:30Z

I think this link might be the right thing: https://facebook.github.io/react-native/docs/testing.html#screenshot-snapshot-tests

charpeni · 2018-03-05T21:04:43Z

Oh I see, they're the same/bundle that's why screenshots were referring to snapshots. Thanks Eli!

bduyng · 2018-03-14T17:16:48Z

any update on this issue?

pkozhin · 2018-07-30T17:12:32Z

Please, would be awesome to get backfaceVisibility working on Android.
Any updates on review of the last changes?

elicwhite · 2018-07-30T20:19:40Z

It looks like the added screenshots include a screenshot for front, but not for back. Support for the back surface is the main thing being added in this diff, right? We should probably have a screenshot for that.

charpeni · 2018-08-01T17:57:53Z

Actually, the initial issue with backface visibility is that the second view (back) was always drawn on the first view (front). So, the front view was showing the backface with a rotation as shown here:

So, if we can see the front view, it means the backface visibility should work or you don't have a backface.

Anyway, just to be sure have add both cases in RNTester, updated screenshots, and rebase on master.

facebook-github-bot

hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

hramos · 2018-08-23T17:55:49Z

error: undefined reference to 'folly::makeConversionError(folly::ConversionCode, folly::Range<char const*>)'

 ** Summary of failures encountered during the build **
Rule __REDACTED__ FAILED because Command failed with exit code 1.
stderr: __REDACTED__/folly/Conv.h:1515: error: undefined reference to 'folly::makeConversionError(folly::ConversionCode, folly::Range<char const*>)'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

charpeni · 2018-08-24T19:56:38Z

Where did you get this error @hramos?

I'm not sure this is directly related to these changes, I've rebased on a working RNTester commit (e2f2e41) and I can't reproduce any Folly errors.

jsoendermann · 2018-09-06T11:46:24Z

@hramos Any chance you can take another look at this? Much appreciated!

facebook-github-bot

hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

react-native-bot · 2018-09-07T17:30:07Z

@charpeni merged commit 0cce0a6 into facebook:master.

Once this commit is added to a release, you will see the corresponding version tag below the description at 0cce0a6. If the commit has a single master tag, it is not yet part of a release.

Summary: `backfaceVisibility` was only available on iOS and 3D transformations were lacking on Android. Backface Visibility is computed from ~the decomposed matrix of transform prop~ the view with their rotation degree values. ~`MatrixDecompositionContext` properties have been made public so we can access to decomposed matrix values from outside (`ReactViewGroup`).~ I'm not sure if this is the best implementation, so if it's not let's discuss it. cc janicduplessis foghina Tested in https://github.com/charpeni/react-native-backface-visibility. | Before | Now | | - | - | | <img src="https://user-images.githubusercontent.com/7189823/30123717-e5361598-9300-11e7-8e2e-a87a7a8d896a.gif" width="260" /> | <img src="https://user-images.githubusercontent.com/7189823/30514997-4d203572-9aee-11e7-8542-bfde41678eb6.gif" width="244" /> | | iOS | Android | | - | - | | <img src="https://user-images.githubusercontent.com/7189823/36995899-609513b4-2083-11e8-9834-ee44c1a292e1.gif" width="300" /> | <img src="https://user-images.githubusercontent.com/7189823/36995978-9ed3b158-2083-11e8-841e-b9e3357d2509.gif" width="240" /> | [ANDROID] [FEATURE] [ReactViewGroup] - Add backface-visibility support on Android Pull Request resolved: facebook#15970 Differential Revision: D9411130 Pulled By: hramos fbshipit-source-id: 62f646a4de37d83922286cb98893a95b55fa889e

Summary: `backfaceVisibility` was only available on iOS and 3D transformations were lacking on Android. Backface Visibility is computed from ~the decomposed matrix of transform prop~ the view with their rotation degree values. ~`MatrixDecompositionContext` properties have been made public so we can access to decomposed matrix values from outside (`ReactViewGroup`).~ I'm not sure if this is the best implementation, so if it's not let's discuss it. cc janicduplessis foghina Tested in https://github.com/charpeni/react-native-backface-visibility. | Before | Now | | - | - | | <img src="https://user-images.githubusercontent.com/7189823/30123717-e5361598-9300-11e7-8e2e-a87a7a8d896a.gif" width="260" /> | <img src="https://user-images.githubusercontent.com/7189823/30514997-4d203572-9aee-11e7-8542-bfde41678eb6.gif" width="244" /> | | iOS | Android | | - | - | | <img src="https://user-images.githubusercontent.com/7189823/36995899-609513b4-2083-11e8-9834-ee44c1a292e1.gif" width="300" /> | <img src="https://user-images.githubusercontent.com/7189823/36995978-9ed3b158-2083-11e8-841e-b9e3357d2509.gif" width="240" /> | [ANDROID] [FEATURE] [ReactViewGroup] - Add backface-visibility support on Android Pull Request resolved: facebook/react-native#15970 Differential Revision: D9411130 Pulled By: hramos fbshipit-source-id: 62f646a4de37d83922286cb98893a95b55fa889e

Summary: `backfaceVisibility` was only available on iOS and 3D transformations were lacking on Android. Backface Visibility is computed from ~the decomposed matrix of transform prop~ the view with their rotation degree values. ~`MatrixDecompositionContext` properties have been made public so we can access to decomposed matrix values from outside (`ReactViewGroup`).~ I'm not sure if this is the best implementation, so if it's not let's discuss it. cc janicduplessis foghina Tested in https://github.com/charpeni/react-native-backface-visibility. | Before | Now | | - | - | | <img src="https://user-images.githubusercontent.com/7189823/30123717-e5361598-9300-11e7-8e2e-a87a7a8d896a.gif" width="260" /> | <img src="https://user-images.githubusercontent.com/7189823/30514997-4d203572-9aee-11e7-8542-bfde41678eb6.gif" width="244" /> | | iOS | Android | | - | - | | <img src="https://user-images.githubusercontent.com/7189823/36995899-609513b4-2083-11e8-9834-ee44c1a292e1.gif" width="300" /> | <img src="https://user-images.githubusercontent.com/7189823/36995978-9ed3b158-2083-11e8-841e-b9e3357d2509.gif" width="240" /> | [ANDROID] [FEATURE] [ReactViewGroup] - Add backface-visibility support on Android Pull Request resolved: facebook#15970 Differential Revision: D9411130 Pulled By: hramos fbshipit-source-id: 62f646a4de37d83922286cb98893a95b55fa889e

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Sep 16, 2017

This was referenced Sep 21, 2017

[Android] Added backface-visibility support for android #6695

Closed

[Style] Support backface-visibility style property on views. #1973

Closed

janicduplessis suggested changes Oct 14, 2017

View reviewed changes

stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 21, 2017

stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 21, 2017

charpeni added Android and removed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 10, 2018

sdwilsh removed the cla signed label Mar 1, 2018

charpeni force-pushed the android-backface-visibility branch from 25a89c9 to 54709a4 Compare March 2, 2018 22:06

hramos removed the GH Review: review-needed label Mar 8, 2018

react-native-bot added Android Ran Commands One of our bots successfully processed a command. labels Mar 16, 2018

plmok61 mentioned this pull request Jul 31, 2018

Shows back on RN 0.46 Android plmok61/react-native-flip-component#2

Closed

charpeni force-pushed the android-backface-visibility branch from 367bba1 to 5bbb738 Compare August 1, 2018 17:51

elicwhite approved these changes Aug 2, 2018

View reviewed changes

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Aug 20, 2018

facebook-github-bot approved these changes Aug 20, 2018

View reviewed changes

charpeni added 5 commits August 24, 2018 15:41

Add backface-visibility support on Android

ab03bf1

Use getRotation instead of keeping our own transform info

e090b00

Add BackfaceVisibility to RNTester

3cd2af2

Update screenshots of RNTester

9678d01

Add example for back view

526f509

charpeni force-pushed the android-backface-visibility branch from 5bbb738 to 526f509 Compare August 24, 2018 19:53

facebook-github-bot approved these changes Sep 6, 2018

View reviewed changes

facebook-github-bot closed this in 0cce0a6 Sep 7, 2018

facebook locked as resolved and limited conversation to collaborators Sep 7, 2018

react-native-bot added the Merged This PR has been merged. label Sep 7, 2018

charpeni deleted the android-backface-visibility branch October 5, 2018 16:52

hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add backface-visibility support on Android #15970

Add backface-visibility support on Android #15970

charpeni commented Sep 16, 2017 •

edited

Loading

pull-bot commented Sep 16, 2017

elicwhite commented Sep 22, 2017

chirag04 commented Sep 22, 2017 •

edited

Loading

janicduplessis Oct 14, 2017

charpeni Mar 2, 2018

janicduplessis Oct 14, 2017

charpeni Mar 2, 2018

janicduplessis commented Oct 14, 2017

facebook-github-bot commented Oct 22, 2017

stale bot commented Dec 21, 2017

charpeni commented Dec 21, 2017

charpeni commented Mar 5, 2018

elicwhite commented Mar 5, 2018

charpeni commented Mar 5, 2018

bduyng commented Mar 14, 2018

pkozhin commented Jul 30, 2018

elicwhite commented Jul 30, 2018

charpeni commented Aug 1, 2018

facebook-github-bot left a comment

hramos commented Aug 23, 2018

charpeni commented Aug 24, 2018

jsoendermann commented Sep 6, 2018

facebook-github-bot left a comment

react-native-bot commented Sep 7, 2018

Add backface-visibility support on Android #15970

Add backface-visibility support on Android #15970

Conversation

charpeni commented Sep 16, 2017 • edited Loading

Test Plan

RNTester

Release Notes

pull-bot commented Sep 16, 2017

elicwhite commented Sep 22, 2017

chirag04 commented Sep 22, 2017 • edited Loading

janicduplessis Oct 14, 2017

Choose a reason for hiding this comment

charpeni Mar 2, 2018

Choose a reason for hiding this comment

janicduplessis Oct 14, 2017

Choose a reason for hiding this comment

charpeni Mar 2, 2018

Choose a reason for hiding this comment

janicduplessis commented Oct 14, 2017

facebook-github-bot commented Oct 22, 2017

stale bot commented Dec 21, 2017

charpeni commented Dec 21, 2017

charpeni commented Mar 5, 2018

elicwhite commented Mar 5, 2018

charpeni commented Mar 5, 2018

bduyng commented Mar 14, 2018

pkozhin commented Jul 30, 2018

elicwhite commented Jul 30, 2018

charpeni commented Aug 1, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

hramos commented Aug 23, 2018

charpeni commented Aug 24, 2018

jsoendermann commented Sep 6, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

react-native-bot commented Sep 7, 2018

charpeni commented Sep 16, 2017 •

edited

Loading

chirag04 commented Sep 22, 2017 •

edited

Loading