AppIntro icon indicating copy to clipboard operation
AppIntro copied to clipboard

[Discussion] Image Handling Improvements

Open AnuthaDev opened this issue 5 years ago • 6 comments

Issue details / Repro steps / Use case background: In this library, handling the memory used by images is not managed at all, which leads to OOMs on several devices(e.g. #551 )I would like to discuss, how we can improve the image handling on our library. I have a few ideas:

  1. Scaling drawables(as Bitmaps) before setting them to their respective imageviews.***

  2. For even better support you can take a look at these links:

a) https://developer.android.com/topic/performance/graphics/manage-memory

b) https://developer.android.com/topic/performance/graphics/cache-bitmap

c) https://blog.booking.com/android-reuse-bitmaps.html

OR

  1. We provide separate modules that use Image loading libraries like Glide, Picasso for loading images as optional dependencies along with our library (probably the easiest).

OR

  1. We could provide a way for the user to get imageviews so that an image loading library can be used(probably not recommended).

***: Highly recommended regardless of approach taken.

Tell me your thoughts on which approach is better and if you know of any other alternative.

AnuthaDev avatar May 25 '19 16:05 AnuthaDev

@cortinico @paolorotolo What do you think should be the preferred approach?

AnuthaDev avatar May 25 '19 16:05 AnuthaDev

@cortinico What do you think?

AnuthaDev avatar Jun 01 '19 07:06 AnuthaDev

@cortinico Please respond

AnuthaDev avatar Jun 04 '19 14:06 AnuthaDev

Hey @AnuthaDev I'll probably go for either 3 or 4.

Can you please justify why you would go for option 1 rather than the others?

My point of view is: 3 - I would generally like the idea of having some sort of abstraction layer that takes care of loading images. Then we can publish an appintro-image-glide or app-image-picasso to provide binding with the common libraries.

4 - That's the quick and dirty solution. Just expose an ImageView to the user, he will take care of loading the content inside. It will anyway break the encapsulation principle and is generally not the best solution.

cortinico avatar Jun 05 '19 00:06 cortinico

@cortinico Oh, I actually thought it was the easiest and quickest (for me😝) to implement. Anyways, I guess option 3 looks the most viable option. Let's hear @paolorotolo on this.

AnuthaDev avatar Jun 05 '19 02:06 AnuthaDev

Having some layer that takes care of image loading is ok for me. I'm not sure if we want to add anyway a method to get the ImageView from the default intro: it's true that some devs could use other libaries or internal solutions to handle image loading but it's also true they can simply write a slide fragment and handle resources themself.

paolorotolo avatar Jun 05 '19 15:06 paolorotolo