-
Notifications
You must be signed in to change notification settings - Fork 24
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
make load image from URI #20
Comments
you don't need make load image from URI, cause you just need input image type Drawable or Bitmap :v @adityapp |
Using Bitmap in the MenuItem model makes it universal to use and this
project loosely-coupled from image loader dependencies. But the drawback is
that it's more complicated to use.
… |
Using or loading drawable or bitmap directly may cause memory issues to the app as loading them requires significant memory usage. Even if it is not necessary or there is no urgency right now, it might be good to create a utility or helper class that lets users to pass their preferred image loaders (Picasso, Glide, Fresco, UniversalImageLoader, etc.). Therefore, the "universal" term can be persisted, as users can still freely use the dependency as how they want it. Moreover, image loaders can be a good solution in case users need to load image from various options or dealing with image loading problems. |
please, attach a memory issue for us to consider @febryanasaperdana |
@adityapp As I have not tried this dependency in any of my apps or projects, I can not attach any memory issues related with the usage of this dependency right now, especially in the case of loading drawable or bitmap directly. Popular memory issues in most Android apps dealing with image loading such as the OutOfMemory (OOM) exception, high heap size, unresponsive/slow UI, may occur depending on the usage and configuration by the users: caching strategy, the bitmap (image) dimension, etc. The above comment by me was meant to be a suggestion to previous comments made by @gifff @syariefnoorp . Image loaders have such a proven mechanism that makes image loading a breeze, as well as opening the possibility to load image from URI per this issue request. I might fork and try this dependency later and see if I can produce any memory issues regarding this, as well as exploring various image loading options. |
Well, I guess there's a misunderstanding in your point of view. What I
meant is that we provide a mechanism to load a Bitmap object to the
ImageView and leave the image-loading mechanism to the users. So if they
want to use existing image loader such as Glide, they have to implement it
themselves and piped the loaded image to our interface (after transforming
to Bitmap object).
…On Fri, 3 Jan 2020, 21:21 Febryan Asa Perdana, ***@***.***> wrote:
@adityapp <https://github.com/adityapp> As I have not tried this
dependency in any of my apps or projects, I can not attach any memory issue
related with the usage of this dependency right now, especially in the case
of loading drawable or bitmap directly. Popular memory issues in most
Android apps dealing with image loading such as the OutOfMemory (OOM)
exception, high heap size, unresponsive/slow UI, may occur depending on the
usage and configuration by the users: caching strategy, the bitmap (image)
dimension, etc.
The above comment by me was meant to be a suggestion to previous comments
made by @gifff <https://github.com/gifff> @syariefnoorp
<https://github.com/syariefnoorp> . Image loaders have such a proven
mechanism that makes image loading a breeze, as well as might opening the
possibility to load image from URI as you stated.
I might fork and try this dependency later and see if I can produce any
memory issues regarding this, as well as exploring various options of
loading image.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20?email_source=notifications&email_token=ABBQHOQ64GB54YUEAAEZCXTQ35CW3A5CNFSM4J5HYM2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIBHBBQ#issuecomment-570585222>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBQHORLVJIWSQK726ZCVYTQ35CW3ANCNFSM4J5HYM2A>
.
|
@gifff Thanks for the clarification, now the air has been cleared up. I have not tried this dependency hence I apologize if my answer is very opinionated. The library is great and there is much room for improvement. |
This feature is already implemented in #24 |
No description provided.
The text was updated successfully, but these errors were encountered: