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

Feature request: Deep integration with react-native-skia (e.g. not via ImageSVG tag) #190

Closed
liamjones opened this issue Aug 1, 2024 · 8 comments

Comments

@liamjones
Copy link

Is your feature request related to a problem? Please describe.
This is related to the fact that labels, etc no longer render when the react-native-skia renderer is used as reported in issue #174.

We'd still like the option of rendering via the rn-skia library as it is much more performant than react-native-svg rendering.

Describe the solution you'd like
rn-echarts uses the rn-skia Path, Polygon, etc APIs to render the chart and isn't reliant on the ImageSVG tag.

Describe alternatives you've considered
For now, as a workaround, we've taken to moving to an old version of rn-skia that still renders text in Android but crashes on iOS and then rendering the chart with different renderers based on platform, see #174 (comment) for details. I fully expect this to break entirely at some point in the future as RN moves forward. This also isn't a solution for anyone who still needs to use rn-skia for non echart related stuff on iOS.

Another alternative - fix the text rendering in rn-skia via the ImageSVG tag. The rn-skia maintainers have said they're not going to do it in Shopify/react-native-skia#2404 / Shopify/react-native-skia#2450. Doing this would require C++ knowledge.

Additional context
Add any other context or screenshots about the feature request here.

@liamjones liamjones changed the title Deep integration with react-native-skia (e.g. not via ImageSVG tag) Feature request: Deep integration with react-native-skia (e.g. not via ImageSVG tag) Aug 1, 2024
@zhiqingchen
Copy link
Member

Yes, we have also considered such a solution, which is more in line with the principles of react-native-skia. It's just that there is not enough time to do it at the moment, but in the long run, we will consider this approach.

@troberts-28
Copy link

Massive +1 for this.

Great to hear that you're considering it @zhiqingchen. In the meantime, this needs to be more clearly acknowledged as a major limitation of the skia element of this library. The fact that you can use Skia is a huge performance benefit, but it's basically unusable without text rendering.

@zhiqingchen
Copy link
Member

I completely agree. The performance benefits of using Skia are indeed significant, but the lack of text rendering capability is a major limitation

@zhiqingchen
Copy link
Member

I am working on the 2.x branch, and there has been some progress, but there is still a lot of work to be done.

If you are interested, please review the code and provide me with some feedback.

@zhiqingchen
Copy link
Member

try 2.0.0-canary.3

@zhiqingchen
Copy link
Member

https://wuba.github.io/react-native-echarts/docs/migration/v2

@liamjones
Copy link
Author

Thank you zhiqingchen and 李志豪 for your work on this! I'm hoping to be able to try it out on our app soon.

@troberts-28
Copy link

Same here! Thank you both 🙏

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

No branches or pull requests

3 participants