Improve Glide usage for Image Loading on Android #5198

Redth · 2022-03-10T16:48:58Z

This makes use of the Glide API's for loading into the ImageView directly instead of returning a Drawable.

src/Core/src/ImageSources/UriImageSourceService/UriImageSourceService.Android.cs

The old one was a png and blurry at the resizes, this one is svg to get the right density versions to make it clear we don't have a scaling issue in the code.

Redth · 2022-03-14T18:26:38Z

src/Controls/samples/Controls.Sample/Pages/Controls/ImagePage.xaml

@@ -13,24 +13,24 @@
                    Style="{StaticResource Headline}"/>
                <Image 
                    Source="https://aka.ms/campus.jpg"
-                    HeightRequest="200" 


We can make requests here, but if we just leave the default, Glide will try and make the imageview bigger which seems to play well with layouts so far...

src/Core/src/ImageSources/Android/Glide/RequestBuilderExtensions.cs

Redth · 2022-03-14T18:28:24Z

src/Core/src/ImageSources/FontImageSourceService/FontImageSourceService.Android.cs

+				var glyph = fontImageSource.Glyph;
+
+				var size = FontManager.GetFontSize(fontImageSource.Font);
+				var unit = fontImageSource.FontAutoScalingEnabled ? ComplexUnitType.Sp : ComplexUnitType.Dip;


This uses SP units if we have font auto scaling enabled, otherwise just DP... The SP ones will scale with font accessibility settings on the device.

src/Core/src/ImageSources/FontImageSourceService/FontImageSourceService.Android.cs

jonathanpeppers · 2022-03-14T18:29:15Z

src/Core/src/ImageSources/FileImageSourceService/FileImageSourceService.Android.cs

+					var builder = glide
+						.Load(fileImageSource.File, imageView.Context!)
+						.AddListener(listener);


I should add the benchmarks from here:

main...jonathanpeppers:ImageBenchmarks

The problem I was seeing was that ImageView.Set* was so much faster than Glide:

Method Mean Error StdDev Gen 0 Allocated

SetImageResource 7.631 µs 0.0165 µs 0.0146 µs - -

SetImageDrawable 55.096 µs 1.2665 µs 3.6135 µs 0.1221 736 B

GlideWithTarget 209.044 µs 4.3352 µs 12.2275 µs 0.2441 1,544 B

It could be there is just way more interop from C# to Java? The example above would many extra JNI calls.

We would potentially have to write the Glide code in Java so we can do a single interop call?

Yeah that was my thought too... we really just want to throw the filename/url/font info/stream over the fence to java, in a single interop call and then wait for the callback.

@jonathanpeppers I moved most of the logic into the android aar project... Would you be able to test the timings here again?

src/Core/src/ImageSources/IImageSourceService.cs

src/Core/src/ImageSources/IFontImageSource.cs

src/Core/src/ImageSources/IImageSourceService.cs

jonathanpeppers

I can probably add my benchmark to this branch and get an idea of the time here:

main...jonathanpeppers:ImageBenchmarks

But I will be FTO tomorrow and back on Thursday.

jonathanpeppers · 2022-03-16T01:38:18Z

src/Core/src/ImageSources/Android/ImageLoaderCallback.cs

+	internal class ImageLoaderCallback : Java.Lang.Object, IImageLoaderCallback
+	{
+		TaskCompletionSource<IImageSourceServiceResult<bool>> tcsResult = new();
+
+		public Task<IImageSourceServiceResult<bool>> Result
+			=> tcsResult.Task;
+
+		public void OnComplete(Java.Lang.Boolean? success, Java.Lang.IRunnable? dispose)
+		{
+			var s = success?.BooleanValue() ?? false;
+
+			Action? disposeWrapper = null;
+			if (dispose != null)
+				disposeWrapper = dispose.Run;
+
+			tcsResult.TrySetResult(new ImageSourceServiceResult(s, disposeWrapper));
+		}
+	}
+
+	internal class ImageLoaderDrawableCallback : Java.Lang.Object, IImageLoaderDrawableCallback


Is there a way these two could be the same type? You pay a cost per C# type that subclasses Java or per Java type you call from C#.

Made these share a type as suggested.

jonathanpeppers · 2022-03-16T03:38:20Z

src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/ImageLoader.java

+    public static void loadFromResourceId(ImageView imageView, int resourceId, ImageLoaderCallback callback)
+    {
+        imageView.setImageResource(resourceId);
+        callback.onComplete(true, null, null);
+    }


Could this one not have a callback at all? Then the calling code would just assume it's synchronous and finished.

Oh if this is synchronous, you can probably delete this method and just call ImageView.

Removed this. I was already bypassing it but forgot one place.

So really, testing loading a file is kind of unnecessary for resource drawable files since they'll never hit this code path now, as we check for the drawable reasource id by filename and use imageView.SetImageResourceId(..) with it instead of calling out to this Image Loader.

It's really just files elsewhere on disk that will hit this path now I think, and fonts, and urls.

jonathanpeppers · 2022-03-16T03:56:48Z

...ve/maui/src/main/java/com/microsoft/maui/glide/fontimagesource/FontModelResourceDecoder.java

+import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPoolAdapter;
+import com.bumptech.glide.load.resource.bitmap.BitmapResource;
+
+public class FontModelResourceDecoder implements ResourceDecoder<FontModel, Bitmap> {


If classes like this are only used from Java, try removing:

Suggested change

public class FontModelResourceDecoder implements ResourceDecoder<FontModel, Bitmap> {

class FontModelResourceDecoder implements ResourceDecoder<FontModel, Bitmap> {

And see if that prevents us getting a C# binding for this type.

We don't get bindings for these already as I've <remove-node'd them.

Original logic was a bit incorrect from reworking this code.

If we don't run these on the main thread we can see: Java.Lang.IllegalStateException : You can't start or clear loads in RequestListener or Target callbacks. If you're trying to start a fallback request when a load fails, use RequestBuilder#error(RequestBuilder). Otherwise consider posting your into() or clear() calls to the main thread using a Handler instead.

Make Glide load into imageview directly

00f0cb8

Redth requested review from mattleibow, PureWeen, jonathanpeppers and eerhardt March 10, 2022 16:49

jonathanpeppers reviewed Mar 10, 2022

View reviewed changes

src/Core/src/ImageSources/UriImageSourceService/UriImageSourceService.Android.cs Outdated Show resolved Hide resolved

rmarinho force-pushed the main branch from 00bb62c to 41406f2 Compare March 14, 2022 12:49

Redth added 4 commits March 14, 2022 12:11

Refactor glide usage and implement listener

c4e4654

Allow image views to autosize

4997314

New test image

fa5d222

The old one was a png and blurry at the resizes, this one is svg to get the right density versions to make it clear we don't have a scaling issue in the code.

Fix image size of Glide loaded font images

25e6e4d

Redth requested a review from jonathanpeppers March 14, 2022 18:21

Redth marked this pull request as ready for review March 14, 2022 18:25

Redth commented Mar 14, 2022

View reviewed changes

src/Core/src/ImageSources/Android/Glide/RequestBuilderExtensions.cs Outdated Show resolved Hide resolved

Redth commented Mar 14, 2022

View reviewed changes

src/Core/src/ImageSources/FontImageSourceService/FontImageSourceService.Android.cs Outdated Show resolved Hide resolved

jonathanpeppers reviewed Mar 14, 2022

View reviewed changes

Redth changed the title ~~[WIP] Improve Glide usage for Image Loading on Android~~ Improve Glide usage for Image Loading on Android Mar 14, 2022

mattleibow reviewed Mar 14, 2022

View reviewed changes

src/Core/src/ImageSources/IImageSourceService.cs Outdated Show resolved Hide resolved

mattleibow reviewed Mar 14, 2022

View reviewed changes

src/Core/src/ImageSources/IFontImageSource.cs Outdated Show resolved Hide resolved

src/Core/src/ImageSources/IImageSourceService.cs Outdated Show resolved Hide resolved

Redth added 3 commits March 15, 2022 21:15

Add glide ImageLoader to android aar project

e4d04f3

Use the helpers from the android aar

d45b862

Use autoscaling property from Font

450b0d4

jonathanpeppers reviewed Mar 16, 2022

View reviewed changes

Redth added 2 commits March 15, 2022 22:55

Merge image callbacks into one

fea8794

Add some benchmark tests

a0aa7d8

jonathanpeppers reviewed Mar 16, 2022

View reviewed changes

More work on benchmark

d51a391

Redth added 2 commits March 21, 2022 11:01

Update interop project

e5d4e3b

Rework glide interop / image service

76b13de

Redth self-assigned this Mar 21, 2022

Redth added this to the 6.0.300-rc.1 milestone Mar 21, 2022

Redth mentioned this pull request Mar 21, 2022

FontImageSource displays small box instead of requested font icon #5157

Closed

Fix more tests

fc99372

This was referenced Mar 22, 2022

[Bug] ImageButtons Aspect behave diferent depending on platform #3272

Closed

[Bug] Android Image Aspect issues #2741

Closed

Redth added 3 commits March 23, 2022 10:12

Clear pending operations for image view targets

7c163ac

Only load if we are supposed to apply the source

c1d1fd7

Original logic was a bit incorrect from reworking this code.

Merge branch 'main' into glidier

8d55dec

Redth mentioned this pull request Mar 23, 2022

Images not rendering correctly #5484

Closed

Redth added 3 commits March 23, 2022 10:50

Fix rebase

fb5707a

More rebase fixes

7c720bf

Fix solution filters

6cc0ad1

Redth mentioned this pull request Mar 23, 2022

Ensure Images handle DPI changes on Windows #3250

Open

Redth linked an issue Mar 23, 2022 that may be closed by this pull request

[Bug] ImageButtons Aspect behave diferent depending on platform #3272

Closed

Redth added 6 commits March 23, 2022 16:58

Throw if request is cancelled in test

e787af4

Do not always clear first

77e048b

Invoke clear on main thread in load calls too

1450bda

Fix refactor

f43bfc1

Fix api call

6baaa73

Redth merged commit 680f1fb into main Mar 24, 2022

Redth deleted the glidier branch March 24, 2022 14:56

mattleibow mentioned this pull request Mar 31, 2022

Improve compatibility with Preview 14 #5711

Merged

mattleibow mentioned this pull request May 19, 2022

[Bug] Change Android Image Handler to Load into view instead of loading a drawable #2271

Closed

jonathanpeppers mentioned this pull request May 25, 2022

MAUI Support jonathanpeppers/glidex#106

Open

github-actions bot locked and limited conversation to collaborators Dec 23, 2023

samhouts added the fixed-in-6.0.300-rc.1 Look for this fix in 6.0.300-rc.1! label Aug 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Glide usage for Image Loading on Android #5198

Improve Glide usage for Image Loading on Android #5198

Redth commented Mar 10, 2022

Redth Mar 14, 2022

Redth Mar 14, 2022

jonathanpeppers Mar 14, 2022

Redth Mar 14, 2022

Redth Mar 16, 2022

jonathanpeppers left a comment

jonathanpeppers Mar 16, 2022

Redth Mar 16, 2022

jonathanpeppers Mar 16, 2022

jonathanpeppers Mar 16, 2022

Redth Mar 16, 2022

jonathanpeppers Mar 16, 2022

Redth Mar 16, 2022

Method	Mean	Error	StdDev	Gen 0	Allocated
SetImageResource	7.631 µs	0.0165 µs	0.0146 µs	-	-
SetImageDrawable	55.096 µs	1.2665 µs	3.6135 µs	0.1221	736 B
GlideWithTarget	209.044 µs	4.3352 µs	12.2275 µs	0.2441	1,544 B

	public class FontModelResourceDecoder implements ResourceDecoder<FontModel, Bitmap> {
	class FontModelResourceDecoder implements ResourceDecoder<FontModel, Bitmap> {

Improve Glide usage for Image Loading on Android #5198

Improve Glide usage for Image Loading on Android #5198

Conversation

Redth commented Mar 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathanpeppers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment