Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Enhancement] Convert all default image handling in Android over to using Glidex #3577

Closed
PureWeen opened this issue Aug 15, 2018 · 8 comments
Labels
e/7 🕖 7 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested m/high impact ⬛ p/Android proposal-accepted roadmap t/enhancement ➕ up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!

Comments

@PureWeen
Copy link
Contributor

PureWeen commented Aug 15, 2018

Description

Handling images in Android has always been problematic. We've made a number of improvements for a few use cases but overall it still isn't great.

Natively on Android google recommends using Glide for everything image related and we should do the same thing

@jonathanpeppers has already started the work over here
https://github.com/jonathanpeppers/glidex

Ideally we can take that work and pull it into the Android Platform core.
Glide can also handle animating gifs and efficiently caching all various sources of images

Implementation Details

Considerations

  • Performance impact of including the glide.jar (compile times, start times, package size)
  • Test with ProGuard, MultiDex, and both enabled. see if we need ProGuard rules

Performance Analysis

@jsuarezruiz performance tests are here https://github.com/jsuarezruiz/xamarin-forms-perf-playground

ImagePerformance.zip

The result:
glidex-perf

ImagePerformance|GridImagesView Memory, Used: 150360064 (143MB), Peak: 150622208, Lowest: 148054016, MaxConsumed: 2568192
ImagePerformance|GridImagesView Memory, Used: 343740416 (327MB), Peak: 344121344, Lowest: 147951616, MaxConsumed: 196169728

Sharing some data like these in the Spec will help to easily understand the idea and why we want this change.

NOTE: Using 400 images. Tested with OnePlus 6.

Difficulty: Low

Peppers has already done all the heavy lifting

@BillyMartin1964
Copy link

Not only pull it in, but make it a priority!

@ederbond
Copy link
Contributor

Any news on this ?

@psp589
Copy link

psp589 commented May 8, 2019

Also interested in this!

@daniel-luberda
Copy link
Contributor

@ederbond @psp589 While you're waiting for Glide support, maybe this would helpful:

You can now easily use FFImageLoading with standard views like Xamarin.Forms.Image. Just call this after Xamarin.Forms.Init call:

  • CachedImageRenderer.InitImageViewHandler() on Android
  • CachedImageRenderer.InitImageSourceHandler() on iOS / Mac / others

... and that's all. Also SVG files do work when providing SvgImageSource as a Source

@samhouts samhouts added this to the 4.3.0 milestone Jun 4, 2019
@SagarPanwala
Copy link

so for iOS, what we need to use ?
As of now , I don't have any problem with FFImageLoading, Instead of using glide for Android only, can we not think for Android and iOS both ?

Due to curiosity, I'm asking, can we not use FFImageLoading directly for Android and iOS both for Image view in built ?

@samhouts samhouts modified the milestones: 4.3.0, 4.4.0 Aug 29, 2019
@PureWeen
Copy link
Contributor Author

@kingces95

GlideX has a sample app here and instructions about how to init
https://github.com/jonathanpeppers/glidex

It's basically just an Init call

Android.Glide.Forms.Init (this);

That replaces our ImageHandlers for use with GlideX. If you look at the readme on that repository you'll see there are a number of tests @jonathanpeppers has done around Memory usage benefits but I'm also curious how adding the GlideX dependency influences startup performance.

Here are the viable scenarios I see to test

  • how much startup overhead does just adding the GlideX nuget and the init call cause? So with a blank forms app that has no images how does the startup performance differ with and without GlideX? I think for this test we'll need to test with the GlideX nuget completely removed vs with it added to the project since the extra binaries will affect load time
  • Startup performance when loading a single image with GlideX vs without GlideX?
  • Startup performance when loading 5 or 10 of the same images with GlideX vs without GlideX?
  • Startup performance when loading 5 or 10 different images with GlideX vs without GlideX?

With the image tests I'm curious if we will start seeing better load times with GlideX as the number of images that have to be processed increases.

@PureWeen PureWeen changed the title [Enhancement] Convert all default image handling in Android over to using Glide [Enhancement] Convert all default image handling in Android over to using Glidex Aug 30, 2019
@kingces95
Copy link
Contributor

kingces95 commented Sep 1, 2019

Sure. I'll validate the performance aspect of a private branch of main. Just cut the branch, prepare the PR, and I'll provide a profile against main. Right now I'm profiling the startup time of template apps. So what's the sartup time of zero images. It's not that the startup time of n images is not interesting, it's just that customers can always delay load images whereas there's nothing they can do about the template startup time -- that all us and only us.

That said, there's nothing stopping a customer from injecting sampling points into their app (or trying the Android profiler if/when we can get accurate statup times). Given an app, I can demonstrate how to inject the sampling points. Tho, I'm fairly sure the advice I'd give would still be to delay load the images.

like the PR will literally be as simple as just modifying the nuspec and platform to include https://www.nuget.org/packages/glidex.forms and then using his Init to replace our handlers

@kingces95 kingces95 removed their assignment Sep 1, 2019
@PureWeen PureWeen removed their assignment Sep 2, 2019
@samhouts samhouts modified the milestones: 4.4.0, 4.5.0 Nov 20, 2019
@samhouts samhouts removed this from the 4.5.0 milestone Feb 11, 2020
@samhouts samhouts added inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! up-for-grabs We welcome community contributions to any issue, but these might be a good place to start! labels Feb 28, 2020
@jfversluis
Copy link
Member

This will not happen anymore for Xamarin.Forms and I think we're already doing this or at least making great improvements in this area for .NET MAUI. Be sure to track the developments there :) Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
e/7 🕖 7 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested m/high impact ⬛ p/Android proposal-accepted roadmap t/enhancement ➕ up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
Projects
None yet
Development

No branches or pull requests

9 participants