Closed
Description
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
Activity
yrizk commentedon Sep 9, 2015
I found a hack that fixes this issue:
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 commentedon Sep 9, 2015
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 commentedon Sep 9, 2015
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 commentedon Sep 11, 2015
Is it possible that the images you load (
imgurImage.getLink()
) are smaller than that so there's nothing to crop?TWiStErRob commentedon Sep 11, 2015
I'm sorry for guessing, I don't have computer access to compile and debug atm.
yrizk commentedon Sep 11, 2015
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 commentedon Sep 28, 2015
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 commentedon Sep 28, 2015
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.
TWiStErRob commentedon Sep 30, 2015
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:
TextView
from item layout: still buggy.dontAnimate()
: still buggy, faster reproimplication: not related to
.crossFade()
orTransitionDrawable
.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"
: AOKimplication: I'm confused!?
StaggeredGridLayoutManager
->LinearLayoutManager
: AOKStaggeredGridLayoutManager
->GridLayoutManager
: still buggyimplication: probably related to two columns
RelativeLayout
->FrameLayout
: AOKRelativeLayout
->LinearLayout
: AOKimplication: 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):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 theViewHolder
I set the tag:this helps to identify which hits to the
getSize
method I need. Next, add a conditional breakpoint ingetSize
: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: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 thanRelativeLayout
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 a1080px
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, seeViewTarget.SizeDeterminer#getViewWidthOrParam
. Since the hardcoded value is there, it just loads the image900px
wide. But the grid sizing constrians the view sizes to476px
, soImageView
has to display a larger image in a smaller space with the defaultfitCenter
, that's why it looks likecenterCrop
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 theTextView
contains more text. Now theSizeDeterminer
figures476px
wide image is needed and crop can properly do it's work which is then shown in a perfectly sizedImageView
, so thefitCenter
's effect is not visible.Solutions
The simplest is to not use hardcoded values, so Glide waits for a proper layout:
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:it also simplifies the loading to:
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 commentedon Sep 30, 2015
@gintechsystems
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 commentedon Sep 30, 2015
Thanks twister. So for now just do these workarounds, will this possibly be fixed in the future?
TWiStErRob commentedon Sep 30, 2015
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 tomatch_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 commentedon Sep 30, 2015
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 commentedon Sep 30, 2015
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 commentedon Sep 30, 2015
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.
26 remaining items