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

Avoid creating loading drawables when there are no images/videos to load #983

Merged
merged 1 commit into from
May 10, 2022

Conversation

mchowning
Copy link
Contributor

@mchowning mchowning commented May 6, 2022

Improves performance by only creating the placeholder drawables for images and videos when there are images or videos, instead of doing it every time we call fromHtml. This makes a big difference in performance when there are a lot of AztecText views on the screen at once (i.e., in Gutenberg-Mobile).

Before this change, scrolling from the top to the bottom of a Gutenberg post containing 500 paragraph blocks could cause the device to allocate and deallocate over a gigabyte of data to create VectorDrawables and Bitmaps for image and video placeholders for every block. After this change, there is no memory allocated or deallocated for that purpose when there are no image or video spans in the HTML passed to Aztec.

Please read the PR description in the WPAndroid repo for more information.

Test

  1. Verify that the Aztec demo app still works fine
  2. Verify that Gutenberg still works fine via the tests described in the WPAndroid PR

Make sure strings will be translated:

  • If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

@mchowning
Copy link
Contributor Author

👋 @planarvoid and @AmandaRiu ! I don't think there should be any issues, but would either of you want to take a look at this change and let me know if you have any concerns from a Day One perspective? 🙇

@mchowning mchowning requested review from SiobhyB and jhnstn May 6, 2022 18:12
@mchowning mchowning marked this pull request as ready for review May 6, 2022 18:12
Copy link
Member

@jhnstn jhnstn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran through the testing instructions on wordpress-mobile/WordPress-Android#16490 and everything passed as expected.

The changes here look simple and appropriate 💯
Nice work @mchowning !

@mchowning mchowning merged commit 7a13d6c into trunk May 10, 2022
@mchowning mchowning deleted the avoid_creating_unnecessary_video_image_drawables branch May 10, 2022 16:53
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

Successfully merging this pull request may close these issues.

2 participants