Skip to content

Commit

Permalink
[android] new IImageViewHandler API (xamarin#3045)
Browse files Browse the repository at this point in the history 
* [android] new IImageViewHandler API

Context:
https://github.com/bumptech/glide
https://github.com/jonathanpeppers/glidex

Currently the way my "proof-of-concept" GlideX library works by
completely bypassing `IImageSourceHandler`. GlideX provides its own
custom `ImageRenderer` and `ImageCellRenderer`. This was required due
to how *opinionated* the Glide library is. Glide's approach is to
never allow a developer access to a `Android.Graphics.Bitmap` or
`Android.Graphics.Drawable` because we would likely do it wrong... and
developers do all the time!

To evolve Xamarin.Forms to where images could be better handled down
the road, I've introduced a new interface:

    public interface IImageViewHandler : IRegisterable
    {
        Task LoadImageAsync(ImageSource imageSource, ImageView imageView, CancellationToken cancellationToken = default(CancellationToken));
    }

The idea is that we can query for `IImageViewHandler` and fall back to
`IImageSourceHandler`. This would allow GlideX to just be an
`IImageViewHandler` and not mess with creating custom renderers.

We can also implement `IImageViewHandler` for `FileImageSource`, to
get some general performance improvements around loading files from
disk:

    string file = ((FileImageSource)imagesource).File;
    if (File.Exists(file))
    {
        //Load with imageView.SetImageURI(Android.Net.Uri)
    }
    else
    {
        //Load with imageView.SetImageResource(int)
    }

I tested this change with new performance tests in `ImageScenarios`:
- Load 100 images using `AndroidResource`
- Load 100 images from disk
- I conveniently prefixed these with `.`, so they appeared first in
  the scenario list

Here are the results from three runs, using `IImageSourceHandler`
versus `IImageViewHandler`, in a HAXM emulator:

`IImageSourceHandler` - 100x `AndroidResource`
- 6059.899
- 3297.885
- 3015.179

`IImageSourceHandler` - 100x from disk
- 12398.71
- 14146.41
- 16060.88

`IImageViewHandler` - 100x `AndroidResource`
- 6748.766
- 2817.975
- 2456.197

`IImageViewHandler` - 100x from disk
- 7326.745
- 4799.001
- 5411.515

There is not going to be as much as an improvement for
`AndroidResource` (maybe not any), since Xamarin.Forms looks for
`Drawable` since: xamarin#1973

NOTE: that these scenarios are probably too slow to keep, as it seems
these performance tests are geared to fail if they take longer than
250ms. I can remove these before we merge this PR.

Other changes to make this happen:
- `Registrar` was failing with `InvalidCastException` instead of
  returning `null` when querying for `IImageViewHandler`. I switched
  to using `as` instead of a hard cast. I am not sure why it was not
  already setup this way, since
  `Registrar.Registered.GetHandlerForObject<T>` appears to be called
  and checked for `null` everywhere.

* Changes to get this puppy "mergeable"

- Removed code from xamarin#1973 that is obsolete, we should not be able to hit that codepath anymore
- Restored: https://github.com/xamarin/Xamarin.Forms/blob/e8660383b03b4010724b36c6eef471a2e74e7c98/Xamarin.Forms.Platform.Android/Extensions/ImageViewExtensions.cs#L55
- Used `SetImageDrawable` and `Drawable` so that `Bugzilla51173_InvalidImage` should pass now
- Made adjustments to the perf scenarios
  • Loading branch information
@jonathanpeppers @PureWeen
jonathanpeppers authored and PureWeen committed Jul 15, 2018
1 parent 0c1127a commit fd73303
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 23 deletions.
47 changes: 47 additions & 0 deletions ...ssues/Xamarin.Forms.Controls.Issues.Shared/PerformanceGallery/Scenarios/ImageScenarios.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.IO;
using System.Linq;
using Xamarin.Forms.Internals;

Expand All @@ -23,4 +24,50 @@ public ImageScenario2()
View = new Image { Source = "coffee.png" };
}
}

[Preserve(AllMembers = true)]
internal class ImageScenario3 : PerformanceScenario
{
const int count = 5;

public ImageScenario3()
: base($"[Image] {count}x AndroidResource")
{
var source = ImageSource.FromFile("bank.png");
var layout = new StackLayout();
for (int i = 0; i < count; i++)
{
layout.Children.Add(new Image { Source = source, HeightRequest = 20 });
}
View = layout;
}
}

[Preserve(AllMembers = true)]
internal class ImageScenario4 : PerformanceScenario
{
const int count = 5;
static readonly string tempFile;

static ImageScenario4()
{
//NOTE: copy image to disk in static ctor, so not to interfere with timing
tempFile = Path.Combine(Path.GetTempPath(), $"{nameof(ImageScenario4)}.png");
using (var embeddedStream = typeof(ImageScenario4).Assembly.GetManifestResourceStream("Xamarin.Forms.Controls.GalleryPages.crimson.jpg"))
using (var fileStream = File.Create(tempFile))
embeddedStream.CopyTo(fileStream);
}

public ImageScenario4()
: base($"[Image] {count}x from disk")
{
var source = ImageSource.FromFile(tempFile);
var layout = new StackLayout();
for (int i = 0; i < count; i++)
{
layout.Children.Add(new Image { Source = source, HeightRequest = 20 });
}
View = layout;
}
}
}
16 changes: 8 additions & 8 deletions Xamarin.Forms.Core/Registrar.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,36 +53,36 @@ internal TRegistrable GetHandler(Type type, params object[] args)
return (TRegistrable)DependencyResolver.ResolveOrCreate(handlerType, args);
}

public TOut GetHandler<TOut>(Type type) where TOut : TRegistrable
public TOut GetHandler<TOut>(Type type) where TOut : class, TRegistrable
{
return (TOut)GetHandler(type);
return GetHandler(type) as TOut;
}

public TOut GetHandler<TOut>(Type type, params object[] args) where TOut : TRegistrable
public TOut GetHandler<TOut>(Type type, params object[] args) where TOut : class, TRegistrable
{
return (TOut)GetHandler(type, args);
return GetHandler(type, args) as TOut;
}

public TOut GetHandlerForObject<TOut>(object obj) where TOut : TRegistrable
public TOut GetHandlerForObject<TOut>(object obj) where TOut : class, TRegistrable
{
if (obj == null)
throw new ArgumentNullException(nameof(obj));

var reflectableType = obj as IReflectableType;
var type = reflectableType != null ? reflectableType.GetTypeInfo().AsType() : obj.GetType();

return (TOut)GetHandler(type);
return GetHandler(type) as TOut;
}

public TOut GetHandlerForObject<TOut>(object obj, params object[] args) where TOut : TRegistrable
public TOut GetHandlerForObject<TOut>(object obj, params object[] args) where TOut : class, TRegistrable
{
if (obj == null)
throw new ArgumentNullException(nameof(obj));

var reflectableType = obj as IReflectableType;
var type = reflectableType != null ? reflectableType.GetTypeInfo().AsType() : obj.GetType();

return (TOut)GetHandler(type, args);
return GetHandler(type, args) as TOut;
}

public Type GetHandlerType(Type viewType)
Expand Down
33 changes: 19 additions & 14 deletions Xamarin.Forms.Platform.Android/Extensions/ImageViewExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,30 @@ public static async Task UpdateBitmap(this AImageView imageView, Image newImage,

imageView.SetImageResource(global::Android.Resource.Color.Transparent);

bool setByImageViewHandler = false;
Bitmap bitmap = null;
Drawable drawable = null;

IImageSourceHandler handler;

if (source != null && (handler = Internals.Registrar.Registered.GetHandlerForObject<IImageSourceHandler>(source)) != null)
if (source != null)
{
if (handler is FileImageSourceHandler)
var imageViewHandler = Internals.Registrar.Registered.GetHandlerForObject<IImageViewHandler>(source);
if (imageViewHandler != null)
{
drawable = imageView.Context.GetDrawable((FileImageSource)source);
try
{
await imageViewHandler.LoadImageAsync(source, imageView);
setByImageViewHandler = true;
}
catch (TaskCanceledException)
{
imageController?.SetIsLoading(false);
}
}

if (drawable == null)
else
{
var imageSourceHandler = Internals.Registrar.Registered.GetHandlerForObject<IImageSourceHandler>(source);
try
{
bitmap = await handler.LoadImageAsync(source, imageView.Context);
bitmap = await imageSourceHandler.LoadImageAsync(source, imageView.Context);
}
catch (TaskCanceledException)
{
Expand All @@ -63,12 +70,10 @@ public static async Task UpdateBitmap(this AImageView imageView, Image newImage,
return;
}

if (!imageView.IsDisposed())
if (!setByImageViewHandler && !imageView.IsDisposed())
{
if (bitmap == null && drawable != null)
{
imageView.SetImageDrawable(drawable);
}
if (bitmap == null && source is FileImageSource)
imageView.SetImageResource(ResourceManager.GetDrawableByName(((FileImageSource)source).File));
else
{
imageView.SetImageBitmap(bitmap);
Expand Down
27 changes: 26 additions & 1 deletion Xamarin.Forms.Platform.Android/Renderers/FileImageSourceHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
using System.Threading.Tasks;
using Android.Content;
using Android.Graphics;
using Android.Widget;
using Android.Net;
using Xamarin.Forms.Internals;

namespace Xamarin.Forms.Platform.Android
{
public sealed class FileImageSourceHandler : IImageSourceHandler
public sealed class FileImageSourceHandler : IImageSourceHandler, IImageViewHandler
{
// This is set to true when run under designer context
internal static bool DecodeSynchronously {
Expand All @@ -31,5 +33,28 @@ internal static bool DecodeSynchronously {

return bitmap;
}

public Task LoadImageAsync(ImageSource imagesource, ImageView imageView, CancellationToken cancellationToken = default(CancellationToken))
{
string file = ((FileImageSource)imagesource).File;
if (File.Exists(file))
{
var uri = Uri.Parse(file);
if (uri != null)
imageView.SetImageURI(uri);
else
Log.Warning(nameof(FileImageSourceHandler), "Could not find image or image file was invalid: {0}", imagesource);
}
else
{
var drawable = ResourceManager.GetDrawable(imageView.Context, file);
if (drawable != null)
imageView.SetImageDrawable(drawable);
else
Log.Warning(nameof(FileImageSourceHandler), "Could not find image or image file was invalid: {0}", imagesource);
}

return Task.FromResult(true);
}
}
}
14 changes: 14 additions & 0 deletions Xamarin.Forms.Platform.Android/Renderers/IImageViewHandler.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System.Threading;
using System.Threading.Tasks;
using Android.Widget;

namespace Xamarin.Forms.Platform.Android
{
/// <summary>
/// The successor to IImageSourceHandler, the goal being we can achieve better performance by never creating an Android.Graphics.Bitmap instance
/// </summary>
public interface IImageViewHandler : IRegisterable
{
Task LoadImageAsync(ImageSource imagesource, ImageView imageView, CancellationToken cancellationToken = default(CancellationToken));
}
}
1 change: 1 addition & 0 deletions Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@
<Compile Include="ContextExtensions.cs" />
<Compile Include="GetDesiredSizeDelegate.cs" />
<Compile Include="IDeviceInfoProvider.cs" />
<Compile Include="Renderers\IImageViewHandler.cs" />
<Compile Include="InnerGestureListener.cs" />
<Compile Include="InnerScaleListener.cs" />
<Compile Include="IPlatformLayout.cs" />
Expand Down

0 comments on commit fd73303

Please sign in to comment.