-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Pay meow] Display all SVG images on UI thread using native libraries #29067
Comments
ProposalPlease re-state the problem that we are trying to solve in this issue.Long render times of components created from svg assets by react-native-svg-transformer What is the root cause of that problem?Currently, all svg assets in the application are transformed by React-Native-SVG-Transformer into components. This process is very demanding and has an impact on the size of the component tree. Each nested element of the svg resource is transformed into a component. For complex assets such as the BackgroundImage on the SignIn Page, the size of the component tree is tremendous. It has a significant influence on the app performance, because the more components we need to render, the more time it will take. What changes do you think we should make in order to solve the problem?Let’s use expo-image! It will allow us to remove the react-native-svg-transformer, display .svgs without any transformations, and improve performance of the app. I’ve prepared benchmarks at three levels of complexity and in all cases I achieved better render times with expo-image than with react-native-svg-transformer. What alternative solutions did you explore?None |
@WojtekBoman Eep! 4 days overdue now. Issues have feelings too... |
@WojtekBoman 6 days overdue. This is scarier than being forced to listen to Vogon poetry! |
@WojtekBoman Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it! |
@muttmuure FYI I'm assigning @akinwale to this issue since he got assigned as C+ on the PR |
@akinwale, @WojtekBoman Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@akinwale, @WojtekBoman 6 days overdue. This is scarier than being forced to listen to Vogon poetry! |
@akinwale, @WojtekBoman 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it! |
@akinwale, @WojtekBoman 12 days overdue. Walking. Toward. The. Light... |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
This issue has not been updated in over 14 days. @akinwale, @WojtekBoman eroding to Weekly issue. |
@muttmuure sorry for confusion, was going to have you do BZ on this but I'm going to do cuz I'm doing some testing. |
@akinwale does this link work for you? |
Yes, I am able to access it without issues. |
@mallenexpensify I have also accepted the offer. |
Contributor+: @akinwale paid $500 via Upwork. I think that's the only payment do, so I'm closing. |
Problem
We’re rendering SVGs using react-native-svg library which is not intended for displaying static images from web or assets. It affects our performance hugely, as rendering big SVGs might take a long time - for BackkgroundImage on Android debug build I’ve seen times like 380ms on pretty powerful Pixel 7
Solution
We should display all SVG images on UI thread using native libraries.
Currently, we have react-native-fast-image in place for displaying PNGs. Unfortunately it does not support SVGs.
We could either migrate to another image library (like expo-image) or add SVG support for react-native-fast-image
For the sake of benchmarking, adding basic support for SVGs in react-native-fast-image on android and comparing its performance to react-native-svg shows HUGE results!
For smaller SVGs, namely for Icons on Settings page (which are mostly one path) it also reduces render time hugely, but since base time is small (2-4 ms) it’s not that huge of a deal.
The text was updated successfully, but these errors were encountered: