Skip to content

Commit

Permalink
Android: Reduce overdraw layers by hiding cards when they are not vis…
Browse files Browse the repository at this point in the history
…ible

Summary:
Cards which are not visible because another card is occluding them are still being rendered by Android resulting in overdraw. This results in wasted GPU time because some pixels are drawn multiple times. This change reduces overdraw by changing the opacity of occluded cards to 0.

This bug was found using the tools described in Android's overdraw docs: https://developer.android.com/topic/performance/rendering/overdraw.html

**Test plan (required)**

This change is being used in my team's app.

Adam Comella
Microsoft Corp.
Closes #10908

Differential Revision: D4175758

Pulled By: ericvicenti

fbshipit-source-id: 4bfac7df16d2a7ea67db977659237a9aa6598f87
  • Loading branch information
Adam Comella authored and Facebook Github Bot committed Nov 14, 2016
1 parent 16b2d5a commit 54beee2
Showing 1 changed file with 9 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,21 @@ function forHorizontal(props: NavigationSceneRendererProps): Object {
}

const index = scene.index;
const inputRange = [index - 1, index, index + 1];
const inputRange = [index - 1, index, index + 0.99, index + 1];

This comment has been minimized.

Copy link
@ptomasroos

ptomasroos Dec 20, 2016

Contributor

May I ask what index + 0.99 actually target? I've been trying to wrap my head around this :) Since i haven't really seen a float within the scene's array / range @rigdern

This comment has been minimized.

Copy link
@ptomasroos

ptomasroos Dec 20, 2016

Contributor

Is it just a hack in order to be able to target the scene before everything thats index +1 or larger? I'm trying to bring in the change to our interpolators and would love to understand that. @ericvicenti

This comment has been minimized.

Copy link
@rigdern

rigdern Dec 20, 2016

Contributor

@ptomasroos Here's an explanation. The card at position N in the nav stack is the top card that the user sees. Prior to this change, when a card moves to the N-1 position, it does a fade animation from opacity 1 to opacity 0.3. Towards the end of the animation, the user can no longer see the card because it's being occluded by the new top card.

A bug with this approach is that it results in overdraw on Android which means that some pixels get drawn multiple times which is a waste of time. The card has an opacity of 0.3 so Android wastes time drawing this card's pixels even though the user doesn't end up seeing them. The goal of this change was to remove the overdraw. The technique is to set the card at position N-1 to an opacity of 0 so Android doesn't spend any time drawing its pixels. We also want to preserve the animation we're currently playing on that card.

The way the fix is implemented is we essentially still do the same animation. The card at N-1 fades from opacity 1 to 0.3. However, right at the end of the animation, the card's opacity jumps to 0. Adding a new value of index + 0.99 between index and index + 1 in the inputRange gives us the opportunity to do a near instantaneous change at the end of the animation. The near instaneaous change is the jump from opacity 0.3 to 0.

This comment has been minimized.

Copy link
@ptomasroos

ptomasroos Dec 20, 2016

Contributor

I see. Now it makes sense. Thanks a lot for taking the time to explain @rigdern !

const width = layout.initWidth;
const outputRange = I18nManager.isRTL ?
([-width, 0, 10]: Array<number>) :
([width, 0, -10]: Array<number>);
([-width, 0, 10, 10]: Array<number>) :
([width, 0, -10, -10]: Array<number>);


const opacity = position.interpolate({
inputRange,
outputRange: ([1, 1, 0.3]: Array<number>),
outputRange: ([1, 1, 0.3, 0]: Array<number>),
});

const scale = position.interpolate({
inputRange,
outputRange: ([1, 1, 0.95]: Array<number>),
outputRange: ([1, 1, 0.95, 0.95]: Array<number>),
});

const translateY = 0;
Expand Down Expand Up @@ -132,23 +132,23 @@ function forVertical(props: NavigationSceneRendererProps): Object {
}

const index = scene.index;
const inputRange = [index - 1, index, index + 1];
const inputRange = [index - 1, index, index + 0.99, index + 1];
const height = layout.initHeight;

const opacity = position.interpolate({
inputRange,
outputRange: ([1, 1, 0.3]: Array<number>),
outputRange: ([1, 1, 0.3, 0]: Array<number>),
});

const scale = position.interpolate({
inputRange,
outputRange: ([1, 1, 0.95]: Array<number>),
outputRange: ([1, 1, 0.95, 0.95]: Array<number>),
});

const translateX = 0;
const translateY = position.interpolate({
inputRange,
outputRange: ([height, 0, -10]: Array<number>),
outputRange: ([height, 0, -10, -10]: Array<number>),
});

return {
Expand Down

0 comments on commit 54beee2

Please sign in to comment.