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

IImageSourceHandler impl #70

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions glidex.forms.sample/RandomAlphaHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ public bool Build (ImageView imageView, ImageSource source, RequestBuilder build
}
}

public void Build (ImageSource source, RequestBuilder builder, CancellationToken token)
{

}

class MyTarget : SimpleTarget
{
static readonly Random rand = new Random();
Expand Down
4 changes: 2 additions & 2 deletions glidex.forms.sample/Resources/layout/Tabbar.axml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<android.support.design.widget.TabLayout xmlns:android="http://schemas.android.com/apk/res/android"
<?xml version="1.0" encoding="utf-8"?>
<com.google.android.material.tabs.TabLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:id="@+id/sliding_tabs"
android:layout_width="match_parent"
Expand Down
2 changes: 1 addition & 1 deletion glidex.forms.sample/Resources/layout/Toolbar.axml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<android.support.v7.widget.Toolbar
<androidx.appcompat.widget.Toolbar
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/toolbar"
android:layout_width="match_parent"
Expand Down
99 changes: 98 additions & 1 deletion glidex.forms/GlideExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
using System.Threading;
using System.Threading.Tasks;
using Android.App;
using Android.Content;
using Android.Graphics;
using Android.Views;
using Android.Widget;
using Bumptech.Glide;
using Java.Util.Concurrent;
using Xamarin.Forms;
using Xamarin.Forms.Platform.Android;
using static Bumptech.Glide.Glide;
Expand Down Expand Up @@ -60,7 +63,7 @@ public static async Task LoadViaGlide (this ImageView imageView, ImageSource sou
CancelGlide (imageView);
return;
}
stream.CopyTo (memoryStream);
await stream.CopyToAsync (memoryStream, token);
builder = request.Load (memoryStream.ToArray ());
}
break;
Expand All @@ -86,6 +89,100 @@ public static async Task LoadViaGlide (this ImageView imageView, ImageSource sou
}
}

public static async Task<Bitmap> LoadViaGlide (this ImageSource source, Context context, CancellationToken token)
{
try {
if (!IsActivityAlive (context, source))
return null;

RequestManager request = With (context);
RequestBuilder builder = null;

if (source is null) {
Forms.Debug ("`{0}` is null", nameof (ImageSource));
return null;
}

switch (source) {
case FileImageSource fileSource:
var fileName = fileSource.File;
var drawable = ResourceManager.GetDrawableByName (fileName);
if (drawable != 0) {
Forms.Debug ("Loading `{0}` as an Android resource", fileName);
builder = request.AsBitmap ().Load (drawable);
} else {
Forms.Debug ("Loading `{0}` from disk", fileName);
builder = request.AsBitmap ().Load (fileName);
}
break;

case UriImageSource uriSource:
var url = uriSource.Uri.OriginalString;
Forms.Debug ("Loading `{0}` as a web URL", url);
builder = request.AsBitmap ().Load (url);
break;

case StreamImageSource streamSource:
Forms.Debug ("Loading `{0}` as a byte[]. Consider using `AndroidResource` instead, as it would be more performant", nameof (StreamImageSource));
using (var memoryStream = new MemoryStream ())
using (var stream = await streamSource.Stream (token)) {
if (token.IsCancellationRequested || stream == null)
return null;
if (!IsActivityAlive (context, source)) {
return null;
}
await stream.CopyToAsync (memoryStream, token);
builder = request.AsBitmap ().Load (memoryStream.ToArray ());
}
break;
}
Comment on lines +106 to +138
Copy link
Owner

Choose a reason for hiding this comment

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

This long switch statement is a lot of copy-paste. This should be refactored out into smaller methods that the other LoadViaGlide() method can share.


var handler = Forms.GlideHandler;
if (handler != null) {
Forms.Debug ("Calling into {0} of type `{1}`.", nameof (IGlideHandler), handler.GetType ());
handler.Build (source, builder, token);
}

if (builder is null) {
return null;
} else {
var result = await builder.Submit ().GetAsync ();
Copy link
Owner

Choose a reason for hiding this comment

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

This isn't the proper way to await an IFuture with cancellation. This should be something like:

Suggested change
var result = await builder.Submit ().GetAsync ();
var future = builder.Submit ();
var result = await Task.Run (() => future.Get (), token);

return (Bitmap) result;
Copy link
Owner

Choose a reason for hiding this comment

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

This code throws InvalidCastException because result is BitmapDrawable!

Suggested change
return (Bitmap) result;
if (result is BitmapDrawable drawable)
return drawable.Bitmap;

}

} catch (Exception exc) {
//Since developers can't catch this themselves, I think we should log it and silently fail
Forms.Warn ("Unexpected exception in glidex: {0}", exc);
return null;
}
}

static bool IsActivityAlive (Context context, ImageSource source)
{
if (context == null || context.Handle == IntPtr.Zero) {
Forms.Warn ("imageView.Handle is IntPtr.Zero, aborting image load for `{0}`.", source);
return false;
}

//NOTE: in some cases ContextThemeWrapper is Context
var activity = context as Activity ?? Forms.Activity;
if (activity != null) {
if (activity.IsFinishing) {
Forms.Warn ("Activity of type `{0}` is finishing, aborting image load for `{1}`.", activity.GetType ().FullName, source);
return false;
}
if (activity.IsDestroyed) {
Forms.Warn ("Activity of type `{0}` is destroyed, aborting image load for `{1}`.", activity.GetType ().FullName, source);
return false;
}
} else {
Forms.Warn ("Context `{0}` is not an Android.App.Activity and could not use Android.Glide.Forms.Activity, aborting image load for `{1}`.", context, source);
return false;
}

return true;
}

/// <summary>
/// NOTE: see https://github.com/bumptech/glide/issues/1484#issuecomment-365625087
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions glidex.forms/IGlideHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ public interface IGlideHandler
/// <param name="token">The CancellationToken if you need it</param>
/// <returns>True if the image was handled. Return false if you need the image to be cleared for you.</returns>
bool Build (ImageView imageView, ImageSource source, RequestBuilder builder, CancellationToken token);
void Build (ImageSource source, RequestBuilder builder, CancellationToken token);
Copy link
Owner

Choose a reason for hiding this comment

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

This should be a C# 8 default interface method, so that existing IGlideHandler implementations won't get a compilation error.

This should also use ref RequestBuilder builder so that IGlideHandler's can set the builder to null if desired.

}
}
12 changes: 10 additions & 2 deletions glidex.forms/ImageViewHandler.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System.Threading;
using System.Threading.Tasks;
using Android.Content;
using Android.Graphics;
using Android.Runtime;
using Android.Widget;
using Xamarin.Forms;
Expand All @@ -12,7 +14,7 @@
namespace Android.Glide
{
[Preserve (AllMembers = true)]
public class ImageViewHandler : IImageViewHandler
public class ImageViewHandler : IImageViewHandler, IImageSourceHandler
{
public ImageViewHandler ()
{
Expand All @@ -24,5 +26,11 @@ public ImageViewHandler ()
Forms.Debug ("IImageViewHandler of type `{0}`, `{1}` called.", GetType (), nameof (LoadImageAsync));
await imageView.LoadViaGlide (source, token);
}

public async Task<Bitmap> LoadImageAsync (ImageSource source, Context context, CancellationToken token = default)
{
Forms.Debug ("IImageViewHandler of type `{0}`, `{1}` called.", GetType (), nameof (LoadImageAsync));
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Forms.Debug ("IImageViewHandler of type `{0}`, `{1}` called.", GetType (), nameof (LoadImageAsync));
Forms.Debug ("IImageSourceHandler of type `{0}`, `{1}` called.", GetType (), nameof (LoadImageAsync));

return await source.LoadViaGlide (context, token);
}
}
}
}