Shopify / react-native-skia

High-performance React Native Graphics using Skia
https://shopify.github.io/react-native-skia
MIT License
6.97k stars 450 forks source link

Renders pile on top of each other when navigating between screens on iOS #183

Closed sultson closed 2 years ago

sultson commented 2 years ago

Issue

When navigating back and forth between screens (using React Navigation), new layers of the Canvas contents are rendered on top of the previous layer.

Reproduction

(the blurred element becomes increasingly dominant) https://user-images.githubusercontent.com/11300079/152666148-dba0ce8b-1059-46f2-b4d1-45a209c7d5c3.mov

The component

import React  from 'react';
import { Pressable } from 'react-native';
import {
  Canvas,
  Paint,
  LinearGradient,
  vec,
  BlurMask,
  RoundedRect,
  Group,
} from "@shopify/react-native-skia";

const SkiaCategoryMEMO = ({onPress}) => {
  console.log('[skiaCategoryMEMO]: render')

  return (
    <Pressable
      onPress={onPress}
      style= {{
        marginVertical:2,
        width: 115,
        height: 145,
        alignItems: 'center',
        justifyContent:'center', 
    }}>
      <Canvas 
        style={{ 
          flex: 1, 
          width: 125,
          height: 120}} >

        <Paint>
        <BlurMask sigma={20} style="outer" />
          <LinearGradient
            start={vec(60, 0)}
            end={vec(256, 120)}
            colors={['white', "black"]}
          />
        </Paint>
        <RoundedRect
            x={10}
            y={12}
            width={105}
            height={120}
            rx={25}
          />
        <Group>
          <Paint>
            <LinearGradient
              start={vec(0, 0)}
              end={vec(60, 60)}
              colors={['white', '#EBECF0']}
            />
          </Paint>
        <RoundedRect
            x={10}
            y={12}
            width={105}
            height={120}
            rx={25}
          />
      </Group>

      </Canvas>
    </Pressable>
  );
};

export default React.memo(SkiaCategoryMEMO)

Platform

chrfalch commented 2 years ago

Hi, @m2llar - thanks for reporting this. I'm working on a fix for something related, so this might very soon be fixed!

chrfalch commented 2 years ago

Hi again @m2llar :) Sorry for this one taking some time - but remember that the Skia Views renders transparently - could that be an issue? Use the component to set the color of the background (or give your parent views a backgroundcolor) - and see if that helps?

sultson commented 2 years ago

Thank you for getting back @chrfalch ! The problem is that this would only fix the visual side, but the renders piling on top of each other also decrease the performance. As a result, when using a Skia View to map several items on a screen, the user experience becomes laggy after navigating back and forth a couple of times. On Android, the renders do not stack like that when navigating and the issue does not occur. Would it be possible to mirror this behaviour on iOS?

chrfalch commented 2 years ago

Yeah, that's a common problem using React Navigation - have you tried enabling react-native-screens? It removes previous views from being rendered when they're not visible in the navigation stack. We've successfully testet previous versions of react-native-skia with this and verified that views will stop animating and rendering when removed.

sultson commented 2 years ago

@chrfalch To my knowledge, React Navigation v6 should enable react-native-screens by default. However, actually disabling react-native-screens on the Bottom Tabs Navigator by adding detatchInactiveScreens={false} as a parameter, solved the issue. I am not sure how viable the solution is as it disables memory optimizations, but it seems to be a workaround for now.

chrfalch commented 2 years ago

Great that you found a solution - will close this one for now.

sultson commented 2 years ago

If anyone is having an issue with the re-renders in Stack screens, the switch from Native Stack to Stack did the trick for me.

wcandillon commented 2 years ago

@m2llar we would be interested to investigate it further. Our example app is using react-navigation. Could you add a small example that would reproduce the issue? It would be extremely helpful to understand how to reproduce the bug.

sultson commented 2 years ago

@wcandillon Hi, I will aim to provide an example for the reproduction of the bug within the next few days. Sorry for a late-ish reply! :)

chrfalch commented 2 years ago

Hi @m2llar - we're still interested in a small project that shows this behaviour for testing purposes, so I you have the opportunity we'd be super happy!

sultson commented 2 years ago

Hi @wcandillon & @chrfalch, apologies for not getting back earlier. Really appreciate the work you guys are doing!!

Here's the example which is condensed into a single App.js file.

It showcases the rendering behaviour with (1) @react-navigation/bottom-tabs (switching back and forth between tabs); (2) @react-navigation/native-stack (pressing on the SkiaContainer).

For bottom-tabs, detatchInactiveScreens={false} solves the issue, however, the solution is not ideal as it affects the screen transition animations.

Reproduction

App.js

import React from 'react';
import { Text, View, Pressable, StyleSheet} from 'react-native';
import { NavigationContainer } from '@react-navigation/native';
import { createBottomTabNavigator } from '@react-navigation/bottom-tabs';
import { createNativeStackNavigator } from '@react-navigation/native-stack';
import {
  Canvas,
  Paint,
  LinearGradient,
  vec,
  BlurMask,
  RoundedRect,
  Group,
} from '@shopify/react-native-skia';
import { useNavigation } from '@react-navigation/core';

const SkiaContainer = () => {
  const navigation = useNavigation();
  const onPress = () => navigation.navigate('Container');

  return (
    <Pressable
      onPress={onPress}
      style={styles.pressable}>
      <Canvas
        style={styles.canvas} >

        <Paint>
          <BlurMask blur={50} style="outer" />
          <LinearGradient
            start={vec(60, 0)}
            end={vec(256, 120)}
            colors={['white', 'black']}
          />
        </Paint>
        <RoundedRect
          x={10}
          y={12}
          width={105}
          height={120}
          r={25}
        />
        <Group>
          <Paint>
            <LinearGradient
              start={vec(0, 0)}
              end={vec(60, 60)}
              colors={['white', '#EBECF0']}
            />
          </Paint>
          <RoundedRect
            x={10}
            y={12}
            width={105}
            height={120}
            r={25}
          />
        </Group>

      </Canvas>
    </Pressable>
  );
};

const Stack = createNativeStackNavigator();

function HomeScreen() {
  return (
    <SkiaContainer />
  );
}

function ContainerScreen() {
  return (
    <View style={styles.screen}>
      <Text>Further details</Text>
    </View>
  );
}

function HomeStack() {
  return (

      <Stack.Navigator screenOptions={{headerShown: false}}>
        <Stack.Screen name="HomeStack" component={HomeScreen} />
        <Stack.Screen name="Container" component={ContainerScreen} />
      </Stack.Navigator>

  );
}

function SettingsScreen() {
  return (
    <View style={styles.screen}>
      <Text>Settings tab</Text>
    </View>
  );
}

const Tab = createBottomTabNavigator();

export default function App() {
  return (
    <NavigationContainer>
      <Tab.Navigator>
        <Tab.Screen name="Home" component={HomeStack} />
        <Tab.Screen name="Settings" component={SettingsScreen} />
      </Tab.Navigator>
    </NavigationContainer>
  );
}

const styles = StyleSheet.create({
  screen: {
    flex:1,
    justifyContent:'center',
    alignItems:'center',
  },
  pressable: {
    width:400,
    height:400,
    alignItems:'center',
    justifyContent:'center',
  },
  canvas: {
    width:130,
    height:150,
    alignItems:'center',
    justifyContent:'center',
  },
});

Platform

chrfalch commented 2 years ago

Thanks so much for providing us with this example - will test it a bit later.

The only thing I'm thinking of (before testing it) is the fact that you are not clearing/filling the screen with any colour - and the Skia Canvas renders as transparent as default (to be able to co-exist with other React Native Views with transparency). Could you try first to add the <Fill color="white" /> component in the Canvas and see if this solves anything?

chrfalch commented 2 years ago

After testing I could verify the issue you were seeing, but after adding the component <Fill color="pink" /> on line 25 (after Canvas) the issue was no longer visible. Would this be a good enough solution?

sultson commented 2 years ago

Thank you for testing it out!

The fill colour really only provides a cosmetic fix, which is probably fine for most use cases. However, when rendering a bunch of those elements, the performance also seems to be impacted because of the piling. Say the application has a Flatlist/map of those items, if the user navigates back and forth, the experience gradually worsens.

chrfalch commented 2 years ago

After measuring what's going on it is clear that there are no extra views rendered on top of each other - the problem is purely cosmetic and caused by React Native Screens handling stopping/starting rendering of views that have been removed - and the view does have a transparent background.

When adding some logging we can see what's happening, this is the output when going to the settings tab from the home tab and back:

2022-03-27 19:59:16.426898+0200 RNSkia[76909:8142356] RNSkDrawView::~RNSkDrawView <- clicking Settings
2022-03-27 19:59:17.408502+0200 RNSkia[76909:8142356] RNSkDrawView::RNSkDrawView <- Clicking Home
2022-03-27 19:59:17.408820+0200 RNSkia[76909:8142356] RNSkDrawView::setNativeId - 1000

So first the Skia view is removed (by React Native Screens), then the view is recreated and the same ID is set (meaning that it represents the same view as the previous time it was visible - and hence keeps the state it had.

So from my point of view this seems to be working as designed :)

wcandillon commented 2 years ago

This has been fixed part of 0.1.120