Fixes some issues with GIF images in the reader.#13906
Fixes some issues with GIF images in the reader.#13906diegoreymendez merged 2 commits intodevelopfrom
Conversation
| return nil | ||
| } | ||
| return nil | ||
|
|
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
|
You can trigger an installable build for these changes by visiting CircleCI here. |
|
Fyi, @yaelirub |
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
leandroalonso
left a comment
There was a problem hiding this comment.
@diegoreymendez after the GIF is loaded the scroll is smooth. :)
Just a question, this is an issue only with big GIFs?
|
@leandroalonso - The core issue was that loading the frames was consuming a lot of CPU. The reason why the jumpy behavior never ended before was that the GIF had more frames than the buffer size (141 frames in the gif vs 50 in our buffer). So the number of frames is one factor but I believe the file size is probably a factor too, as loading larger frames would consume more cpu. Additionally we were loading all media twice and animating twice, which also consumed more memory and CPU. I’ll want to profile this a bit more before merging, so let me know if you have any other concern. |
|
Hey! I'm moving this to 14.8 because 14.7 has been cut. If you want this to make it to 14.7, please feel free to ping me. |
|
I've tested in all of my devices and haven't found any issue with the new code. I'm merging. |
Fixes #13895 (well kind of, read on).
The issue in #13895 is that the GIFs were really large, so we kept loading frames at all times. This PR changes the amount of frames in the buffer to 300, which means there will be no more jumps when scrolling the reader AFTER THE GIF IS DONE LOADING.
There will still be some jumps before the GIFs finish loading, and fixing this IMHO is not great in terms of efforts vs benefit.
Additionally, an issue in the reader was causing attachments to be loaded twice, so these GIFs were animating twice. This was caused by a strong reference to the images held by a the
mediaArraywe're removing.To test:
PR submission checklist:
RELEASE-NOTES.txtif necessary.