Skip to content
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

incorrect paths rendering #2814

Closed
doublelam opened this issue Dec 17, 2024 · 6 comments · Fixed by #2823
Closed

incorrect paths rendering #2814

doublelam opened this issue Dec 17, 2024 · 6 comments · Fixed by #2823
Labels

Comments

@doublelam
Copy link

Hi, I am facing a very strange issue, when I change the state of paths, the canvas renders them incorrectly.

here is the demo:
https://github.com/user-attachments/assets/31602b1c-95f9-4208-9bc2-00b9e2e9f2f7

and here is the code:

import { Text, SafeAreaView, StyleSheet, Button, View } from 'react-native';
import {useState, useCallback} from 'react';
import { Canvas, Path } from '@shopify/react-native-skia';
const initPath = new Array(10).fill('').map((p, i) => `M50 ${(i + 1) * 20} L300 ${(i + 1) * 20}`);
export default function App() {
  const [paths, setPaths] = useState(initPath);
  const onPress = useCallback(() => {
    setPaths(ps => ps.slice(0, -1))
  }, []);

  const onAdd = useCallback(() => {
    setPaths((ps) => {
      return ps.concat(`M50 ${(ps.length + 1) * 20} L300 ${(ps.length + 1) * 20}`);
    });
  }, []);
  return (
    <SafeAreaView style={styles.container}>
      <Text style={styles.paragraph}>
        Paths not rendered correctly by set state
      </Text>
      <Text style={styles.paragraph}>
        Paths count: {paths.length}
      </Text>
      <Canvas style={{flex: 1}}>
        {
          paths.map(path => <Path 
            strokeWidth={10}
            strokeJoin="round"
            strokeCap="round"
            color="#222" 
            key={path} 
            style="stroke" 
            path={path} />)
        }
      </Canvas>
      <View style={{padding: 10, gap: 10}}>
      <Button onPress={onAdd} title="increase 1 path" />
      <Button onPress={onPress} title="decrease 1 path" />
      <Button onPress={() => setPaths(initPath)} title="reset" />
      </View>
    </SafeAreaView>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    backgroundColor: '#ecf0f1',
    padding: 8,
  },
  paragraph: {
    margin: 24,
    fontSize: 18,
    fontWeight: 'bold',
    textAlign: 'center',
  },
  
});

The weird part is that when I test it on Expo https://snack.expo.dev/@loom2023/react-native-skia-path , it works properly. I am not sure if it's my code issue or package issue, can you help?

@wcandillon
Copy link
Contributor

My apologies but I couldn't spot the problem from the video recording?

@doublelam
Copy link
Author

My apologies but I couldn't spot the problem from the video recording?

Hi, for example:
Image
you can see that when I set the state to 4 paths, the canvas only renders 3 paths, and some times when I decrease it to 1 path, it still renders 2, it only happens in bare react native project, actually I am not sure if it's my project issue or skia issue because I can not reproduce it on Expo,
Image

@doublelam
Copy link
Author

@wcandillon Hi, I find that when comment/delete this.tick() in SkiaDomView, it works properly
Image

@wcandillon
Copy link
Contributor

yes I think this is a nice catch, thank you for investigating this, I should have a fix for it soon hopefully.

wcandillon added a commit that referenced this issue Dec 18, 2024
fixes #2814

The race condition would occur when prop update in the view would trigger a redraw request, then the DOM  would also emit a  redraw request itself would get canceled due to the pending redraw. That first redraw request however, is not using the updated DOM version.

We are rearchitecturing this part of the system at the moment, where such issues won't be possible anymore. However this should be good fix for now.
@wcandillon
Copy link
Contributor

@doublelam thank you again for catching this, I hadn't realized such race condition was possible.

wcandillon added a commit that referenced this issue Dec 18, 2024
fixes #2814

The race condition would occur when prop update in the view would trigger a redraw request, then the DOM  would also emit a  redraw request itself would get canceled due to the pending redraw. That first redraw request however, is not using the updated DOM version.

We are rearchitecturing this part of the system at the moment, where such issues won't be possible anymore. However this should be good fix for now.
Copy link
Contributor

🎉 This issue has been resolved in version 1.7.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants