Problem with Thumbnail Loading from a custom ImageViewFactory

[KirkBushman]

Hi, it's me again.

I've noticed an error while trying to make a custom ImageViewFactory, let me explain:
-You are loading a thumbnail in a detail view of the image inside BIV (Second activity)
-You already loaded the image on the First Activity, you should hit cache.
-This does not happen if you used RequestOptions in your call the first time.
-The thumbnail loading takes a little bit too long depending on the image, and you have lag.

In this case you do have to make a custom ImageViewFactory, but when I did, the loadThumbnailContent() function did not get called.
This happens because there are 2 functions overloads, when I override both, the call gets called twice, and messes up my image transition.

Basically in certain configurations we are loading Thumbnail images more than one time.

Some questions:

Do we need to use ImageLoader to load both thumbnail and image?
Do we need 2 interface (ImageLoader & ImageViewFactory) to manage external calls?
Do we need to manage cache, since both glide and fresco already manages cache on their own?
(Maybe we can just interface some calls to fresco and glide)

I would try to test something, but my fork is occupied by #179 at the moment...
Can you merge that for the moment while I test some stuff out?

[Piasy]

Hey @KirkBushman , thanks for your work!

Here are some answers to your question:

Do we need to use ImageLoader to load both thumbnail and image?

I think yes, if the thumbnail is also an online image, what do you suggest to do if not usingImageLoader?

Do we need 2 interface (ImageLoader & ImageViewFactory) to manage external calls?

ImageViewFactory is added to handle image view creation, mainly for gif support, because I don't want to mess view stuff into ImageLoader, which should focus on image loading.

Do we need to manage cache, since both glide and fresco already manages cache on their own?

Yes we need for Fresco, because we will use SSIV to show the main image, SSIV need the File of image in cache. We didn't handle cache stuff for Glide, right?

As for the PR, I don't want to merge it since it's still working in progress. You can create another branch on your fork, and working on the new things.

Thank you for your contribution again!

[KirkBushman]

I think yes, if the thumbnail is also an online image, what do you suggest to do if not usingImageLoader?

As of now, If one has to give an option to the request, you have to override everything, both ImageLoader and ImageViewFactory, and since you cannot give different params to the thumb call and source image call, you get in Glide duplicated calls, because of different keys on the cache.
I think we should rework those components to make them simple, I would not keep the call onStart() and only use one.

ImageViewFactory is added to handle image view creation, mainly for gif support, because I don't want to mess view stuff into ImageLoader, which should focus on image loading.

Since they are both an abstraction on imaging lib those could have really a couple of methods, I think it would be better to merge them.

mmm... I really don't know if my use case really fits this lib...

[Piasy]

and since you cannot give different params to the thumb call and source image call, you get in Glide duplicated calls, because of different keys on the cache.

You mean when enable DelayMainImage for transition, the thumbnail image will be loaded twice?

But why? Isn't glide use url as cache key?

[KirkBushman]

Nope, cache key includes sizes, effects and other params in request options... Even if you have it in cache it will download it twice during .downloadOnly() call...

[Piasy]

Okay, if what you said is true, what do you suggest?

I think we should rework those components to make them simple, I would not keep the call onStart() and only use one.

You can make a more detailed proposal here, then we can discuss about it.

[KirkBushman]

This is why I am reluctant, there is lot I would change in the interfaces, I would only have one call to load from thumbnail and source, would remove the onStart() calls and take them after loading the URLs...
I would set the interfaces so that the user, can build the request, how he wants, and match the cache he already has...
I would not control the cache but listen to the cache of glide and fresco...

In the end you would have only a few methods in the interface like: loadThumbnail() loadImage() onImageLoaded() and so on.

[KirkBushman]

I am experimenting locally, to find a simpler solution

[Piasy]

Thanks, but I'm still not very clear about what you propose, could you please make a more detailed prototype?

[KirkBushman]

Since I think my use case has reached a level that needs it's own solution, in the weekend I've been working on this:
https://github.com/KirkBushman/LargeImageView

This is working for me in every case and I think is simple enough to pick up on the go.

The problem:

Since LruCache, uses a lot of params as the cache key,

Glide
.with(context)
.load(uri)
.into(imageView)

and

Glide
.with(context)
.load(uri)
.override(300, 300)
.into(imageView)

will produce 2 different calls, caches and all.

If you have already the image in cache, and you want to use it in another view, you replicate the same call
(That's the reason, in my projects, I use Kotlin extensions to load images in the same way in different parts of the app).

To do this in BigImageViewer it's kind of difficult, because you need to override everything, and I was getting duplicate thumbnails images, because we are sharing the same call from onStart between the thumb and the source image.
Maybe there is a way to do this, but my dumb head could not find it easily.

The Logic:

We need a solution the completely exposes to the user the call and the views. This can be done with less code, since SSIV takes care of all thing once you have the image file, and you imaging-lib (Glide) takes care of everything until that, you only need to have a carousel view that handles the views and switch between them.

Take a look at this:
https://github.com/KirkBushman/LargeImageView/blob/master/lib/src/main/java/com/kirkbushman/largeimageview/LargeImageView.kt
https://github.com/KirkBushman/LargeImageView/blob/master/sampleapp/src/main/java/com/kirkbushman/largeimageview/sampleapp/activities/BasicActivity.kt

The main view never needs to worry about the url or caching calls, just simply managing the views.

[KirkBushman]

I'm probably going to continue with my own solution, but maybe we can take this as a chance to update BIV, which is used by a lot of people.

What do you think?

[Piasy]

Thanks for sharing, but I'm very busy these days, glad that you have your own solution, I'll check it later when I got time.