CenterCrop is not always working

[yrizk]

Glide Version/Integration library (if any): 3.6.0
Device/Android Version: any
Issue details/Repro steps/Use case background:

How to repo:
I have a demo app with this bug here: https://github.com/yrizk/glide-centercrop-bug

but essentially...
no scale type declared in the xml
centerCrop() when loading image into an imageview.
make sure the app is cleared from memory

The image will start w/o the center crop, then if you scroll past or above it, then sometimes it comes back with the correct scale type.

on a separate note, the library is really good! thank you guys for building and supporting this

[yrizk]

I found a hack that fixes this issue:

Glide.with(itemView.getContext()).load(url).asBitmap().centerCrop().diskCacheStrategy(DiskCacheStrategy.ALL).into(new BitmapImageViewTarget(imageView) {
            @Override
            public void onResourceReady(Bitmap bitmap, GlideAnimation anim) {
                super.onResourceReady(bitmap, anim);
                Glide.with(itemView.getContext()).load(url).centerCrop().into(imageView);
            }
        });

EDIT: this fix prevents cardview:cardPreventCornerOverlap="false" from working correctly , and I'm almost certain that since its fixed with a bit of off-screen scrolling, the issue is related to a recycler view.

[TWiStErRob]

Didn't run your code, but one thing worth trying is changing that wrap_content to match_parent or 0dp on the custom ImageView.

Also try it without the custom ImageView altogether, to see if Glide is compatible with your ImageView. In case it works with stock ImageView you could try to generalize the aspect ratio thing into a custom layout (ViewGroup) and have a fill-fill ImageView inside that.

[yrizk]

hello @TWiStErRob thanks for the reply. Actually the code in my project is using a std ImageView, I accidentally added a custom imageview to that sample app . Also I have given the image a set height (150dp) and width (300dp) . Just update the github project to reflect these things.

[TWiStErRob]

Is it possible that the images you load (imgurImage.getLink()) are smaller than that so there's nothing to crop?

[TWiStErRob]

I'm sorry for guessing, I don't have computer access to compile and debug atm.

[yrizk]

I don't thikn so but lets see. the imageview itself is set to 300 * 150 dp , the image that's being loaded is 615 * 425 px (maybe not on the imgur link, but it is with the real data source). On a htc one m8 the width is 1080 pixels with a densityDPI of 480 which means the the ImageView is actually
900 px * 450 px ?? that's definetly not right, cz the device itself is 1080 px wide, and there grid is 2 columns wide, which is impossible if each is 900 px wide. But that's what the math says...

anyways, if that there was the case, then why is it applying it after the ImageView comes back in the viewport ?
I don't know if this is related, but you guys mention in the wiki that you use Android's CENTER_CROP for your centerCrop() if you apply them side you'll see that Android's ImageView.ScaleType.CENTER_CROP is actually cropping around a slightly different part of the image. Perhaps the CenterCrop algorithm itself isn't deterministic, and you guys are both doing the same thing.

Hope this is helpful. I think if you follow the repo steps you'll see what I mean.

[gintechsystems]

This also happens to me. I will go into my activity and the images will load. If I hit back and then go back into the activity again, they scale correctly the second time around? Also @yrizk I tried your hack and it seems to work but then it no longer animates.

EDIT:

Moving super.onResourceReady after setting my imagedrawable keeps the animation.

[gintechsystems]

Also from playing around if I set my src and have it set to centerCrop on the imageview, it seems glide will display the new image the same exact size on the initial load, if you go leave and go back so it displays it again, it displays correctly. Maybe layout issue of some sort or how the image takes cropping?

EDIT:

One new note, if you set the imageview resource on SetResource that is when it happens, it seems to work fine using SetResourceReady.

                           @Override
                            protected void setResource(GlideDrawable resource) {
                            }

                            @Override
                            public void onResourceReady(GlideDrawable resource, GlideAnimation<? super GlideDrawable> glideAnimation) {
                           view.setImageDrawable(resource);                                   
                           super.onResourceReady(resource, glideAnimation);

                            }

[TWiStErRob]

Sooo, I ran your example app. Sorry for the delay, I was AFK. Here's my investigation:

First run: buggy, symptoms are: first screen of images load correctly, along with partially visible list items. The next row below that doesn't seem to apply centerCrop, but rather fitCenter, neither the next one, but after that centerCrop is correctly applied again, implication: the problem has to do with recycling the item views.

The layout and is too complex to debug, simplify and gather more info:

• remove TextView from item layout: still buggy
• .dontAnimate(): still buggy, faster repro
  implication: not related to .crossFade() or TransitionDrawable
• .diskCacheStrategy(ALL): still buggy, faster repro, no network, even with resized images
• .skipMemoryCache(true): still buggy. I expected this to always repoduce the issue, even on second scrolling over the buggy images, but it behaved the same.
  implication: not related to disk or memory cache hit
• .centerCrop() -> scaleType="centerCrop": AOK
  implication: I'm confused!?
• StaggeredGridLayoutManager->LinearLayoutManager: AOK
  StaggeredGridLayoutManager->GridLayoutManager: still buggy
  implication: probably related to two columns
• RelativeLayout->FrameLayout: AOK
  RelativeLayout->LinearLayout: AOK
  implication: it's a layout sizing issue

What's up with that .centerCrop() -> scaleType="centerCrop" fixing the issue?
The issue manifestation looks like fitCenter. It's hard to imagine Glide would fail to apply an explicitly asked for .centerCrop(). Moving the explicit cropping into XML, makes it implicit and also overrides the default fitCenter. implication: the Bitmap must be cropped and then shown with fit when the issue is present.

Test image: http://i.imgur.com/jMvECu2.jpg, original size: 604x453
Add .listener() to log (only excerpt shown here):

http://i.imgur.com/G5CSRw1.png ImageView{528e56cc 0,0-476,450} onResourceReady(false, true) bitmap: 476, 450 // first screen loaded
http://i.imgur.com/jMvECu2.jpg ImageView{528a4510 0,0-476,450} onResourceReady(false, true) bitmap: 900, 450 // first load: buggy image
http://i.imgur.com/jMvECu2.jpg ImageView{528a5b38 0,0-476,450} onResourceReady(false, true) bitmap: 476, 450 // later load: AOK image

So, indeed the bitmap is loaded with bigger than the view size, fitting applies.

Why is Glide loading a 900x450 image into a clearly 476x450 size ImageView?
I'm using a Galaxy S5 emulator with FullHD display, 480dpi -> xxhdpi/3x. Screen width: 1080
Let's debug ViewTarget.SizeDeterminer#getSize: when populating the ViewHolder I set the tag:

imageView.setTag(R.id.image, imgurImage.getLink());

this helps to identify which hits to the getSize method I need. Next, add a conditional breakpoint in getSize:

"http://i.imgur.com/jMvECu2.jpg".equals(view.getTag(com.amg.android.projectt.R.id.image))

App startup: no hits as expected, scroll down, breakpoint hit. currentWidth=900, hmm, @yrizk was confused about the item size too. Let's see where it comes from:

view.getWidth(): 0
view.getLayoutParams().width: 900

Aah, makes sense, since the XML says layout_width="300dp". But then why did it work with Frame and Linear layouts? They're much simpler than RelativeLayout and they probably ask for how big they can be, before asking their children to measure themselves.

TL;DR and summary
Setting the layout width to 300dp (900px) for a 2-column grid on a 1080px screen confuses Glide, because it tries to read the sizing information from the laid out image and then the layout params, if neither is available it waits for a layout, see ViewTarget.SizeDeterminer#getViewWidthOrParam. Since the hardcoded value is there, it just loads the image 900px wide. But the grid sizing constrians the view sizes to 476px, so ImageView has to display a larger image in a smaller space with the default fitCenter, that's why it looks like centerCrop wasn't applied.

Then that item scrolls off screen and gets reused. When scrolled back the same image is bound to a recycled item which means it was already laid out and now view.getWidth() will give the right size. The grid items are identical even with staggered grid layout. Their width doesn't change even when the TextView contains more text. Now the SizeDeterminer figures 476px wide image is needed and crop can properly do it's work which is then shown in a perfectly sized ImageView, so the fitCenter's effect is not visible.

Solutions

• The simplest is to not use hardcoded values, so Glide waits for a proper layout:

  <CardView
      ...
      android:layout_width="match_parent"
      android:layout_height="wrap_content"
      >
      <RelativeLayout
          android:layout_width="match_parent"
          android:layout_height="match_parent"
          >
          <ImageView
              ...
              android:layout_width="match_parent" <!-- this makes Glide wait -->
              android:layout_height="150dp"
              />
          ...
      </RelativeLayout>
  </CardView>

• moving .centerCrop() -> scaleType="centerCrop" is not a good solution, because it still makes Glide load 900px images. The original is cropped to 900px by Glide and then cropped by the ImageView to the real size. It's double work and even more, because when scrolled out and back even the disk cache misses, because the second time the original needs to be cropped to the right size by Glide, resulting in double network requests and double caching for the same image in RESULT cache.

• using another layout may be ok, but it still hides the problem like above.

I still suggest to move centerCrop() into XML, because it's a view thing, not logic:

    <ImageView
        ...
        android:layout_width="match_parent" <!-- this makes Glide wait -->
        android:layout_height="150dp"
        android:scaleType="centerCrop"
        />

it also simplifies the loading to:

Glide.with(itemView.getContext()).load(imgurImage.getLink()).into(imageView);

and all the benefits of resource directories are unleashed, for example you can use fitCenter and different size on tablet sized screens, all just defined in XML.

[TWiStErRob]

@gintechsystems

set to centerCrop on the imageview, it seems glide will display the new image the same exact size on the initial load

This is the second solution I described, it just hides the issue more, please review your layout and try to make it more dynamic.

[gintechsystems]

Thanks twister. So for now just do these workarounds, will this possibly be fixed in the future?

[TWiStErRob]

It cannot be fixed as I see it. Reading the LayoutParams.width is a really good way to speed up image loading, in most cases, and only when the layout agrees. Try to replace the width to match_parent, which is not a workaround, it is the correct way to allow Glide bring it's best performance; In a way Glide forces you to make better layouts, because it may misbehave if you half-ass it. Double-loading and custom target hacks will just bite you later.

[gintechsystems]

Thanks, understood. I currently use match_parent but I also use match_parent for my height because I want aspect ratio and I measure on imageview which creates a perfect square. Not sure if this is bad practice though.

[TWiStErRob]

I use a simple layout around the ImageView to make it square: https://gist.github.com/TWiStErRob/f8190f1f55ec1e7777fc. It plays real nice with Glide so far.

[gintechsystems]

Thanks, that is how mine is setup and is working great with glide. Was just checking to make sure what I was doing was ok.

[TWiStErRob]

This also looks like a duplicate of #291.

[gintechsystems]

Yes I agree.

[yrizk]

@TWiStErRob thank you so much for that very detailed investigation. Seriously, I'm impressed. That 900px certainly makes more sense light of what you just said. I understand that waiting the layout traversal so that dimensions are finalized removes the confusion, but I'm still confused as to why the fitCenter as opposed to centerCrop . if Glide mistook the ImageView to be 900px wide instead of what it was allowed to be (476px) , isn't that still a larger image loading into a smaller view? Shouldn't a scaling type still be a valid operation? does it somehow lose its scaling type because the bitmap itself was resized from 900px to 476px?

[TWiStErRob]

thank you so much for that very detailed investigation.

That's what you get for providing a sample project... ;)

@yrizk the centerCrop disappears because the upscaled centerCropped image is fitCentered (even if you say .centerCrop() the default on the ImageView is still fitCenter) into the small ImageView:

1. Original size: 604x453 (upper left)
2. View size in layout: 900x450 (blue), which is upscaled (humula emulates centerCrop)
3. Which is then fitted into 476x450 (red): notice the height is already good, which means it has to fit the width meaning it has to resize 900 to 476 keepig aspect
4. The result is a 476x238 image displayed in a 476x450 sized box (red), which means top and bottom padding.

When you have match_parent, Glide receives 475x450 resulting in the perfect size (green) where it also doesn't matter which scaleType is used, since none of them can alter the image in a way that will distort it.

Feel free to ask more if this doesn't clear it up!

[yrizk]

yes I understand exactly what you mean now. and wow you keep raising the bar!! Seriously thank you again this is all very illuminating. I'd buy you a drink , but I just looked into int'l fund transfer and its discouraging. if you've had success with paypal lmk :) Glide takes the cake for best support in my book

[lingyfh]

when I change placeholder image size, it's ok. OMG!!!!!!

[SarvagyaVaish]

My ImageView has a layout_width="0dp" since i am using a layout_weight="2". Do you have any suggestions for when I cannot use match_parent?

[gintechsystems]

@SarvagyaVaish What happens when you don't use match_parent. can you show us the issue from a screenshot?

[SarvagyaVaish]

After further testing I found out that the problem was with setting a placeholder image. I was setting a square placeholder image. Once Glide fetched the actual rectangular image it replaced the placeholder with it, but ended up forcing it to be square.
For now my workaround is to use a color as a placeholder instead of the square png placeholder. If you know of a way to get glide to respect the aspect ratio of the image it is fetching, please let me know. Also, if you want I can create a sample app that demonstrates this oddity.

Thanks!

[lucazin]

I still suggest to move centerCrop() into XML, because it's a view thing, not logic:

<ImageView
    ...
    android:layout_width="match_parent" <!-- this makes Glide wait -->
    android:layout_height="150dp"
    android:scaleType="centerCrop"
    />

it also simplifies the loading to:

Glide.with(itemView.getContext()).load(imgurImage.getLink()).into(imageView);

this fix my problem !

[Mkurbanov]

i just removed placheldor from glide and it's worked for me :)

[madisadyk]

this bug is still in version 4.13.0