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

Consider supporting Bitmaps/Drawables for consistency #122

Closed
TWiStErRob opened this issue Sep 6, 2014 · 18 comments
Closed

Consider supporting Bitmaps/Drawables for consistency #122

TWiStErRob opened this issue Sep 6, 2014 · 18 comments

Comments

@TWiStErRob
Copy link
Collaborator

Idea

Glide.with(this).load(bitmap).into(imageView);
Glide.with(this).load(drawable).into(imageView);

This looks weird compared to ImageView.setImageBitmap(bitmap), but if you need animation and error handling (null bitmap), the above could become an easy to use, fire and forget, go-to line instead of duplicating code.
Note: you've already made a wrapper of ImageView#setImageResource and ImageView#setImageUri.

Currently they throw

Caused by: java.lang.IllegalArgumentException: Unknown type android.graphics.Bitmap@44759ab8. You must provide a Model of a type for which there is a registered ModelLoader, if you are using a custom model, you must first call Glide#register with a ModelLoaderFactory for your custom model class.
at com.bumptech.glide.RequestManager.loadGeneric(RequestManager.java:382)
at com.bumptech.glide.RequestManager.load(RequestManager.java:374)

Context

This came up when I was trying to display a captured image using Glide (oversimplified code):

startActivityForResult(new Intent(MediaStore.ACTION_IMAGE_CAPTURE), Activity.RESULT_FIRST_USER);

public void onActivityResult(int requestCode, int resultCode, Intent data) {
    if(requestCode == REQUEST_CODE_GET_PICTURE && resultCode == Activity.RESULT_OK) {
        Bitmap bitmap = (Bitmap)data.getExtras().get("data");
        Glide.with(this).load(bitmap)/*...*/.into(image);
@sjudd
Copy link
Collaborator

sjudd commented Sep 6, 2014

Interesting - I'd assume you'd expect transformations/transcoding to work as normal? It would be a little weird, but not too difficult to add a set of model loaders/data fetchers/decoders that all just pass along the bitmap or drawable.

@TWiStErRob
Copy link
Collaborator Author

TWiStErRob commented Sep 6, 2014

The use case is: I have a Bitmap and I want to present it the same way as usual.
All of this is to make the api more user friendly. You just throw anything at it and it works. :)

Something similar to having NullEncoder and its friends, but Passthrough*. but all this is handled internally, it should be weird only for someone digging the code.

Yes, transfromations, placeholder, error, transcode (wrapping Bitmap in a Drawable), animation, listener and all the framework provided stuff.

One thing I can't get hold of is caching. Because you already have a bitmap and probably the next time it'll be a different instance with other pixels I can't think of it as a key. Writing bitmap to disk cache is pointless for this reason (you need the whole bitmap to read it from cache). May be caching the Drawable has a point: if we already created a Drawable for the exact same bitmap just find it in memcache and use that.

This made me think of another thing as well which may be under the same umbrella: generated bitmaps, is it possible with the current api? It'd be best if I only had to implement one of the interfaces and the rest is figured out by the framework. In this case you're an making images out of thin air, an example is a fractal library I could imagine.

@sjudd
Copy link
Collaborator

sjudd commented Sep 7, 2014

Makes sense and you're right we could hide the complexity inside the library. I'd like to clean up how we register and pass along the various decoders/encoders before trying to tackle this, but I agree it's a reasonable use case.

Generated bitmaps should be possible, depending on what you use to generate them. You can pass in any data model you want and define any arbitrary decoder, so as long as you can generate a bitmap in the decoder from your data model (or data you fetch using your data model) it's very plausible.

@mr-gilberto
Copy link

how can resolve it?

@TWiStErRob
Copy link
Collaborator Author

@willby, sorry, here it is for you (and for posterity), a little summary and a workaround:

Using Glide for the job of calling a method is a little overkill and it only has the benefit of doing the transformations in the background, for this reason it will be considered in the future, but for now: set it directly.

Instead of

Glide.with(this).load(bitmap).....into(image);

write:

image.setImageBitmap(bitmap);

@TWiStErRob
Copy link
Collaborator Author

Alternatively it's possible to make it work through advanced Glide usage, but I don't really recommend this because we know 4.0 will be better in this regard. In case someone still wants to hack around:

None of the above are restricted to only R.drawable.* resources (which is built in to Glide as .fromResource()), these Drawable objects can come from anywhere (e.g. SVG lib).

@cristiangarciascmspain
Copy link

@TWiStErRob I think it's not overkilling to use Glide to load a Bitmap, but lets put everything on context.
Imagine you have a URI schema data ( coming from your api ) with a very small thumbnail, and you want to show it while you wait for the real picture.

Doesn't it make sense?

Not only Fesco accept URI Schema data
also is something recommended by some big companies like twitter

About 4.0 I didn't find the changelog or roadmap for it.

sorry, I probably should write this on #556 but I was trying to solve the same problem, but parsing the bitmap myself.

@TWiStErRob
Copy link
Collaborator Author

TWiStErRob commented Oct 19, 2015

I originally created this issue because I received a Bitmap in an Intent (see context in first comment). If you use Glide to decode a data Uri or apply transformations onto the Bitmap you already have, it may worth squeezing through Glide. In willby's case (he removed his comment) it was an overkill as you see from the answer, there was no need for any of that.

Data URI is in the queue as you've seen, feel free to create a PR for it.

4.0 roadmap is in the 4.0 milestone (got there by choosing Glide's Issues and selecting Milestones on top). There's also a design document linked in the "4.0 Overview" issue.

If you decoded it yourself and you already have a small thumbnail Bitmap, the easiest is to not call .thumbnail(), but to call .placeholder(new BitmapDrawable(res, bitmap)).

@cristiangarciascmspain
Copy link

Nice, thanks.

I'd like to make the PR, but this feature is not currently on my priority list. The backend doesn't send the picture, and it's not easy to be changed, I hope you will do it before it's time to use it.

@newmanw
Copy link

newmanw commented Dec 8, 2015

Not sure is this is related, but this does not work either.

Some drawable with alpha (mydrawable_transparent.xml)

<?xml version="1.0" encoding="utf-8"?>
<bitmap xmlns:android="http://schemas.android.com/apk/res/android"
    android:src="@drawable/mydrawable"
    android:alpha="200">
</bitmap>
Glide.with(context)
    .load(R.drawable.mydrawable_transparent)
    .into(iv);

@TWiStErRob
Copy link
Collaborator Author

@newmanw This issue is for .load(Drawable|Bitmap), what you want is in #350 (subscribe to that).

@pingothedoer
Copy link

pingothedoer commented Oct 3, 2016

A little workaround that I am doing to avoid this problem.

Bitmap bitmap = // Your Bitmap;
if (bitmap != null) {
    ByteArrayOutputStream stream = new ByteArrayOutputStream();
    bitmap.compress(Bitmap.CompressFormat.PNG, 100, stream);
    Glide.with(this)
        .load(stream.toByteArray())
        .placeholder(R.drawable.empty_contact)
        .bitmapTransform(new CropCircleTransformation(this))
        .into(mAvatar);
} 

@TWiStErRob
Copy link
Collaborator Author

TWiStErRob commented Oct 3, 2016

@pingothedoer +1 for the clever idea, but I think that's a bad approach. You're almost doing I/O on the UI thread. At least hogging it for a long time just to compress the image, which will be immediately decompressed. I think my Proof of Concepts above are simple enough to be preferred over blocking the UI thread for any milliseconds.

@tpfaff
Copy link

tpfaff commented Dec 7, 2016

I want this so that glide can auto handle the rotation via exif tags for me on an existing bitmap object.

@TWiStErRob
Copy link
Collaborator Author

@tpfaff Bitmap objects don't have EXIF data, so that's a wrong reason to want this.

@huanjulu
Copy link

the error happend when i use glide try set a 'no - string -image-url '
ig .lide.with(context).load(dejaProduct.getImage()).into(iamge);

the "dejaProduct.getImage" is no string image url,

@sjudd
Copy link
Collaborator

sjudd commented Oct 10, 2017

One more thought about this: Implementing this would require that you give Glide ownership of any Bitmap you pass in. Transformations would work for Bitmaps, but not BitmapDrawables or other types of Drawables.

We could also add support for transforming BitmapDrawables using our existing transformation (only modestly painful), but we can't add support for transforming arbitrary Drawables, especially if they're animated.

Given those limitations I'm tempted to close this. It's super weird to pass a Bitmap into Glide, then try to use it later and find that Glide has recycled it or drawn a different image into it. Ditto for BitmapDrawable. It's also super weird for anything other than BitmapDrawable that calling .centerCrop() or .fitCenter() or even adding a DownsampleStrategy won't work.

I guess we kind of already have the transformation problem since I added support for non-Bitmap resources here: 7614e10. We don't currently have the ownership/recycling problem though.

@sjudd sjudd removed this from the 4.3 milestone Oct 10, 2017
@sjudd
Copy link
Collaborator

sjudd commented Nov 12, 2017

... So I actually implemented this. We wanted to support loading application icons with Glide, some of which are not decodable as Bitmaps. To make those work with transformations, we had to tackle most of the problems I mentioned earlier.

It's not efficient because we can't re-use Bitmaps we're provided, but it will work. Typically it would be better to write a ModelLoader to obtain the Bitmap that Glide can own and re-use.

See 7663c21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants