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

Some images simply do not download (Black image) - Android #738

Closed
thiagoluis88 opened this issue Nov 12, 2015 · 44 comments
Closed

Some images simply do not download (Black image) - Android #738

thiagoluis88 opened this issue Nov 12, 2015 · 44 comments

Comments

@thiagoluis88
Copy link

Hi,
In some devices (Moto X) do not download correctly, like the screenshot attached.
When i skipMemoryCache(true) it works, because Glide download the image again.
I already use asBitmap() with imageDecoder() and the problem still occur.

But, in my other device (Nexus 4) this problem doesn't occur. Glide works perfectly

I tried to config listener() to log Exceptions but never called onException() method. In other words, no Exceptions occurs. Its weird.

Best,
Thiago Luis.

screenshot_2015-11-12-16-45-43

@TWiStErRob
Copy link
Collaborator

Hi there, thanks for reporting. Please follow the instructions and answer the questions below to help us diagnose further. (Alternatively you can provide GitHub mini project that reproduces this issue).

Try enabling internal logging to see what's going on in the logs.

When you say no listener.onException is being called, is onResourceReady called? Try to debug onResourceReady to see if the Bitmap resource value passed is a valid bitmap (AS/IDEA has "View Bitmap" in debugger or I think you can dump it to SD card via a oneliner: resource.compress(PNG, 0, new FileOutputStream("/sdcard/temp.png"))).

What does your imageDecoder do? Does it work correctly without that?

What are the installed Android versions for each of these devices?

Since you say it works on one phone, but not on an other, I rule out the possiblity that the view is invisible when you try to load, which may lead to no actual load happening and no callbacks called.

What happens if (instead of skipMemoryCache) you turn it off via a GlideModule? (I expect it works)

@Override public void applyOptions(Context context, GlideBuilder builder) {
    builder.setMemoryCache(new MemoryCacheAdapter());
}

@thiagoluis88
Copy link
Author

Hi,
I enabled the logging e nothing weird appear. Just logs about downloading from URL.

Yes, the onResourceReady was called. I saw the Bitmap resource and it was a valid bitmap, like the others that were downloaded. Same size, etc.

My imageDecoder was:

public static StreamBitmapDecoder getBitmapDecoder(Context context){
    BitmapPool bitmapPool = Glide.get(context).getBitmapPool();
    return new StreamBitmapDecoder(Downsampler.AT_LEAST, bitmapPool,      DecodeFormat.PREFER_ARGB_8888);
}

I did like this because the other way(with meta-data android:name="com.inthecheesefactory.lab.glidepicasso.GlideConfiguration") brought me the black image with more frequency.

My Android versions are 4.1.2(Razr), 5.1(Moto x) and 5.1.1(Nexus 4).

With skipMemoryCache(true) works, because Glide download again when the black image occur, but i don't want with this approach.

I will try the resource.compress() to see if the black image appear on the sdcard.

Best,
Thiago Luis

@thiagoluis88
Copy link
Author

Hi,
I did on more test here: Remove de imageDecoder() and leave the Glide configuration as default:

DrawableTypeRequest<String> request = Glide.with(context)
        .load(NetworkUtils.DEAL_IMAGE_URL + pictures.get(position));
//BitmapRequestBuilder<String, Bitmap> request = Glide.with(context)
//        .load(NetworkUtils.DEAL_IMAGE_URL + pictures.get(position))
//        .asBitmap()
//        .imageDecoder(GlideUtils.getBitmapDecoder(context));
if( width > 0 && height3x4 > 0 ) {
    request.override(width, height3x4).fitCenter();
}
request.placeholder(R.drawable.no_picture).into(imgGallery);

And works. No more black image. But the default configuration is RGB_565, right? Because of this a did this configuration of imageDecoder to get ARGB_8888. But, no matter how i will configure this approach (via meta-data or imageDecoder()) brought me some black image.

Best,
Thiago Luis.

@TWiStErRob
Copy link
Collaborator

So, to sum up (please confirm if this is correct): if you use 8888 on a Moto X (5.1) you get a black bitmap, but this doesn't happen on Nexus 4 (5.1.1) nor on Razr (4.1.2); or if memory cache is skipped. Bitmap contents are yet unconfirmed (with .compress()), but highly likely all black pixels.

This is a duplicate of #599, but that wasn't fixed due to no repro.

More questions (sorry, it's not a trivial issue):

  • Did you have a chance to look at the actual pixels of the bitmap resource?
    Are they uniformly all black?
  • Is the .override().fitCenter() code executed?
  • Do you use a custom ImageView here or a custom Target anywhere in your app?
    Sadly, one misbehaving little piece of code (e.g. a custom target) can poison the whole application, that's why it's important to know if you have it anywhere in your app.
  • Does it work if you configure 8888 and add .dontAnimate() (unlikely to help)
  • Since you didn't mention .placeholder() before I'm assuming it shows up and then it changes to black after a little time, is that correct?
  • Would you be able to create a minimal app that reproduces this issue?

Just logs about downloading from URL.

Skipping memory cache shouldn't result in a download, only a disk cache hit.


skipMemoryCache(true) works [...] but i don't want with this approach.

Of course, that's understandable, but trying those cases helps us figure out what the problem might be.

@thiagoluis88
Copy link
Author

Yes, that is right. When i use ARGB_8888 and only on Moto X, that i know.

My ImageView isn't a custom View
I use override() and placeholder()

Yes, first show the placeholder image and, after show the black image.

When i compressed the Bitmap, the image really had all black pixels (see attached image).
screenshot_2015-11-13-11-18-32

In the Activity i sent before:
I tested with dontAnimate() and fitCenter() and continues show the black image
I tested with dontAnimate() without fitCenter() and continues show the black image
I tested without dontAnimate() and fitCenter() and continues show the black image

All those tests above i did with into(imageView). When i changed to into(ImageViewTarget(){setResource()} no more black images were downloaded
But, only works when i remove fitCenter() and put dontAnimate()

In my ImageViewTarget i just do this:

@Override
protected void setResource(GlideDrawable resource) {
    getView().setImageDrawable(resource.getCurrent());
}

In other Activity that i use with centerCrop():
I tested with centerCrop() and dontAnimate() and into(ImageViewTarget(){setResource()} and continues show the black image
I tested without centerCrop() and into(ImageViewTarget(){setResource()} and no more black images were downloaded

So, if i do not use centerCrop() or fitCenter() works good. No more problem. But i need to use dontAnimate()

Could i help you with this informations?

Best,
Thiago Luis

@TWiStErRob
Copy link
Collaborator

I tried to reproduce it like this, in a list:

Glide
    .with(imageView.getContext())
    .load(getItem(position))
    .asBitmap()
    .imageDecoder(new StreamBitmapDecoder(imageView.getContext(), DecodeFormat.PREFER_ARGB_8888))
    .override(400, 300)
    .fitCenter()
    .placeholder(android.R.drawable.ic_menu_revert)
    .into(imageView)
;

Based on your description it should've loaded a black image, but it didn't. The problem is that I have a Galaxy S4 (4.4.2). Can you please check if this code will reproduce your issue on your Moto X? So we'll have a known short repro.

@sjudd do you have any ideas? See my prev comment for a summary. It looks like a pooling issue.

@thiagoluis88
Copy link
Author

Hi,

I tried like you said but still brought me the black image.
But, when i changed to into(new BitmapImageViewTarget(imageView)) and removed fitCenter() it worked perfectly and a little bit faster.

I'm testing in 3g.

Maybe some network issue? Any idea?

Well, with this approach i'm satisfied. I will use BitmapImageViewTarget() and about scaleType i will use on ImageView itself.

Best,
Thiago Luis.

@TWiStErRob TWiStErRob added the bug label Nov 13, 2015
@TWiStErRob
Copy link
Collaborator

Thanks for confirming that the code I gave you reproduces the problem. Hopefully we can reproduce it on some device in the future and debug what's going on. I'm glad you found a workaround yourself!

@mullender
Copy link

@TWiStErRob I can reproduce the problem with the Gallery sample app on the master branch.
I am also using a Moto X 2nd gen on android 5.1
To reproduce I simply need to scroll. The specific images that fail are differ for different clean runs.
But once they fail they seem to get cached in the failed state.

@mullender
Copy link

When the bitmappool is disabled (by always returning a new bitmap from the pool) the issues still reproduces.

@TWiStErRob
Copy link
Collaborator

@mullender please try with BitmapPoolAdapter, the pool shouldn't create new objects. All call-sites are prepared to receive null and will allocate a Bitmap themselves if needed.
By trying the adapter you can confirm that the problem may be in BitmapFactory when providing an inBitmap thorugh options. Also try to play with MemoryCacheAdapter.

@mullender
Copy link

I have added both adapters and the issue remains, which means the memory cache and the bitmappool can be ruled out.

@TWiStErRob
Copy link
Collaborator

Hmm, so it's not reuse then.

Can you try to enable logging: https://github.com/bumptech/glide/wiki/Debugging-and-Error-Handling#more-logging maybe there's a hint there.

I understand it's "different for different clean runs", but can you try to load the same or similar image through BitmapFactory with 8888 and see if it's black?

@mullender
Copy link

I narrowed it down to the fitCenter transformation (and probably also the other transformations):
after the transformation the pixels are black 0xff000000.
As a workaround I check a few pixels and if they are all black I repeat the transform. (which typically succeeds) and if that fails I return the original.
Can you say hack :)

@TWiStErRob
Copy link
Collaborator

hack :)

@mullender
Copy link

This looks very similar:
hdodenhof/CircleImageView#31

@mullender
Copy link

@TWiStErRob I created a fix, that resolves the issue. See the PR.

The fix has two parts:

  • not setting the target density (which seems to have values that are not related to the screen density)
  • adding synchronization on the canvas editing
    Together they resolve the problem, either one alone does not.

@mullender
Copy link

Perf impact of adding a lock to:

result = BitmapFactory.decodeStream(is, null, options);

result without lock (decoding 1K images), super fast scrolling

D(21747:21789) --> count: 1000
D(21747:21789) -->   0     0.00
D(21747:21789) -->  10     0.00
D(21747:21789) -->  20     0.00
D(21747:21789) -->  30     1.00
D(21747:21789) -->  40     5.00
D(21747:21789) -->  50     7.00
D(21747:21789) -->  60    17.00
D(21747:21789) -->  70    50.00
D(21747:21789) -->  80   230.00
D(21747:21789) -->  90   278.00
D(21747:21789) --> 100   450.00

result WITH lock (decoding 1K images), super fast scrolling

D(25639:25701) --> count: 1001
D(25639:25701) -->   0     0.00
D(25639:25701) -->  10     2.00
D(25639:25701) -->  20    23.00
D(25639:25701) -->  30    52.00
D(25639:25701) -->  40   132.00
D(25639:25701) -->  50   215.00
D(25639:25701) -->  60   254.00
D(25639:25701) -->  70   306.00
D(25639:25701) -->  80   446.00
D(25639:25701) -->  90   560.00
D(25639:25701) --> 100  3792.00

This doesn't seem acceptable to enable unconditionally, especially since we only have a single device affected (that we know about)

mullender pushed a commit to mullender/glide that referenced this issue Nov 20, 2015
…on (affects only Moto X 2nd gen on api 22)

this addresses a threading bug on this specific device, the bug manifests itself by displaying
black images instead of resized images.
mullender pushed a commit to mullender/glide that referenced this issue Nov 20, 2015
…on (affects only Moto X 2nd gen on api 22)

this addresses a threading bug on this specific device, the bug manifests itself by displaying
black images instead of resized images.
mullender pushed a commit to mullender/glide that referenced this issue Nov 20, 2015
…on (affects only Moto X 2nd gen on api 22)

this addresses a threading bug on this specific device, the bug manifests itself by displaying
black images instead of resized images.
mullender pushed a commit to mullender/glide that referenced this issue Nov 20, 2015
…on (affects only Moto X 2nd gen on api 22)

this addresses a threading bug on this specific device, the bug manifests itself by displaying
black images instead of resized images.
@sjudd sjudd closed this as completed in 59f3d9c Nov 24, 2015
sjudd added a commit that referenced this issue Nov 24, 2015
fix for issue #738 Some images simply do not download (Black image) -…
@TWiStErRob TWiStErRob added this to the 4.0 milestone Nov 24, 2015
@TWiStErRob TWiStErRob reopened this Jan 28, 2016
@Mauker1
Copy link

Mauker1 commented Feb 3, 2016

Perhaps this bug is related to this question?

@TWiStErRob
Copy link
Collaborator

Yep, looks like the same issue, but no solution there either. @mullender's workaround seems the best option sadly.

@Mauker1
Copy link

Mauker1 commented Feb 29, 2016

Not sure if entirely related to the issue, but every time I try to load the image and I get that black canvas, this is popping up on my logcat:

02-29 13:20:57.216: D/skia(29612): --- SkImageDecoder::Factory returned null

@RikNorakomi
Copy link

Running into this same issue as well on a Samsung GP-p7510 running Android 4.0.4 - API 15.
Might you have a code example of mullender's workaround on checking the transform for black pixels and repeating the transform with all black pixels? :-)

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Sep 26, 2016

It might be better if we added a condition for your device as well at 59f3d9c#diff-bcf1e10e8c7c676afd4d4cb151781112R44 (feel free to PR)

@RikNorakomi If you really want to try, I would imagine it went down like this:

// Glide.....bitmapTransform(new HackyBlacky(new CenterCrop(context)))....;
class HackyBlacky implements Transformation<Bitmap> {
    private final Transformation<Bitmap> realTransform;
    public HackyBlacky(Transformation<Bitmap> realTransform) { this.realTransform = realTransform; }
    @Override public String getId() { return realTransform.getId(); }

    @Override public Resource<Bitmap> transform(Resource<Bitmap> resource, int outWidth, int outHeight) {
        Resource<Bitmap> original = realTransform.transform(resource, outWidth, outHeight);
        if (!isBlack(original.get())) {
            return original;
        }
        Resource<Bitmap> secondTry = realTransform.transform(resource, outWidth, outHeight);
        if (!isBlack(secondTry.get())) {
            // retry worked, let's use that
            original.recycle();
            return secondTry;
        }
        // oh well, both black, we're unlucky; or the image is really black
        secondTry.recycle();
        return original;
    }

    private static boolean isBlack(Bitmap check) {
        Random rnd = new Random();
        for (int i = 0; i < 10; i++) {
            int x = rnd.nextInt(check.getWidth());
            int y = rnd.nextInt(check.getHeight());
            if (check.getPixel(x, y) != Color.BLACK) {
                return false;
            }
        }
        return true;
    }
}

@KatrinYakimova
Copy link

Have this issue on Moto G.
using
ImageView.setImageDrawable(resource.getCurrent()); in onResourceReady instead .into(ImageView) solved the problem for me.
Don't khow how correct is it, but i've tested and it works.

@TWiStErRob
Copy link
Collaborator

@KatrinYakimova I think you have a false-positive fix: if I read it correctly, you used .into(Target) instead of .into(ImageView). If you take a look at the code of those two methods you'll notice that one of them automatically applies a transformation and the other one doesn't. When you use a custom target you have to manually say .fitCenter()/.centerCrop().

Since you removed the transformation step from your Glide request, there's no chance of having the canvas synchronization issue. I guess you got that code from #738 (comment) which also said:

So, if i do not use centerCrop() or fitCenter() works good.

@sjudd sjudd modified the milestones: Next, 4.0 Apr 1, 2017
@sjudd
Copy link
Collaborator

sjudd commented Apr 1, 2017

I'm going to mark this closed, if more new models come up, we can add them to the lock whitelist as well.

@sjudd sjudd closed this as completed Apr 1, 2017
sjudd added a commit to sjudd/glide that referenced this issue Apr 1, 2017
@sjudd sjudd modified the milestones: 4.0, Next Apr 2, 2017
@rsousabluebeck
Copy link

This issue still happens on Moto X 2nd Gen as per version 4.1.1. It can be fixed as a workaround by adding .apply(new RequestOptions().dontTransform()) to your builder.

@sjudd
Copy link
Collaborator

sjudd commented Oct 2, 2017

@rsousabluebeck what is the model version from Build.MODEL? There's a manual blacklist of device models that can't be transformed which we can update if we get the right string.

sjudd added a commit to sjudd/glide that referenced this issue Oct 5, 2017
More progress towards bumptech#738.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170938515
@strooooke
Copy link
Contributor

Also applies to XT1032.

sjudd added a commit to sjudd/glide that referenced this issue Dec 11, 2017
@strooooke
Copy link
Contributor

Wow, that was fast! I could only verify with the XT1032, because I have one of those here; I'd assume that XT1031, XT1033, XT1034, XT1035, XT1008, XT1028, XT939G, XT1039, XT1040, XT1042 and XT1045 are also affected.

@strooooke
Copy link
Contributor

strooooke commented Dec 12, 2017

Just for general information: I'm dealing with an incarnation of this bug that is independent on transformations, so the dontTransform workaround doesn't help - adding the affected device to the "really lock" list helps through the Downsampler codepath.

And I have an affected device (an XT1068) that runs on API level 23.

@mullender
Copy link

With the growth of the list, we could probably switch: private static final List<String> MODELS_REQUIRING_BITMAP_LOCK over to a Set<String>
But realistically it won't matter that match for a List this small :)

@sjudd
Copy link
Collaborator

sjudd commented Dec 14, 2017

Yeah a set is somewhat better, but I agree it's not a big deal.

@strooooke mind sending a pull request with the affected models?

strooooke added a commit to strooooke/glide that referenced this issue Dec 14, 2017
Yet more progress on bumptech#738.

Observations aren't confined to API level 22 anymore.
sjudd pushed a commit that referenced this issue Dec 15, 2017
* Update list of models that require locks around Canvas

Observations aren't confined to API level 22 anymore.

Yet more progress on #738.
@i-m-aman
Copy link

I want to point out one more thing for the upcoming person. I ran the code with glide on the emulator with Gapps not installed and images were not loading and then I bumped into this thread. Then I update the GApps and the images start fetching. So future people, please check on the emulator that you are running the code with Gapps running on it.

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