View sizes are hard.

[sjudd]

V4.1 Logic

It's actually difficult to describe the logic in ViewTarget in Glide 4.1, but it's something like:

1. Use the View width/height - padding if either the layout parameters are non-zero and greater than the padding or if no layout is currently requested.
2. Use Target.SIZE_ORIGINAL if no layout is currently requested and the layout params are set to wrap_content
3. Use the layout parameter width and height if they're non-zero and greater than the padding.

Where it works

This logic works for a couple of cases:

1. match_parent layout parameters in RecyclerView and elsewhere (hits case 1 above after layout)
2. wrap_content layout parameters with views that actually want the original image (hits case 2 above)
3. Fixed size views (ie layout width and height are set to values > 0).

Where it breaks

It introduces two significant problems:

1. In RecyclerView/ListView images may flash if the child Views use the match_parent layout parameters because even if the Views are re-used and have valid sizes that won't change, they're typically pending a layout when they're re-added to the RecyclerView (if for no other reason than Glide calls setImageDrawable() for the placeholder when the load is cleared and/or started).
2. wrap_content uses Target.SIZE_ORIGINAL, which causes OOMs.

Where it doesn't work

It also doesn't solve one case that it was meant to solve:
3. The View uses fixed layout parameters, but the parent arbitrarily resizes the child ignoring the exact parameters.

I've gone back and forth on this a few times, and the following seems clear:

1. It's not possible to accurately determine the View size in every case.
2. Loading original size images is usually not what callers want.

vNext Logic

I believe I can improve on the v4.1 logic by:

1. If the View's layout parameter dimensions are > 0 and > padding, use the layout parameters
2. If the View dimensions are > 0 and > padding, use the view dimensions
3. If the View layout parameters are wrap_content and at least one layout pass has occurred, Log a warning suggesting to use Target.SIZE_ORIGINAL or some other fixed size via override() and use the screen dimensions.
4. Otherwise (layout parameters are match_parent, 0, or wrap_content and no layout pass has occurred), wait for a layout pass, then go back to 1.

As a result, there are a few things I expect not to work:

1. Child Views in RecyclerView where the layout parameters are set to match_parent but where the LayoutManager will set different parent sizes for different positions.
2. Using wrap_content and expecting an image that can be zoomed/panned.

I can fix 1 by adding an option that will force a request to wait for layout if it's loaded into a View. I think 2 is mostly fixed by a warning log that suggests SIZE_ORIGINAL or another fixed layout parameter.

[Tolriq]

@sjudd how does this address ConstraintLayout ?

For the record in the XML often the sizes will be 0dp with constraints that will resolve the size at layout.

Reading the logic it's not handled. And getting all the parents layout to find if there's a constraint layout does not seems good either.

[sjudd]

Thanks for reviewing and pointing that out @Tolriq.

I've updated the steps a bit to better reflect what actually happens in the code. I will make one change though, which is to wait for at least one layout pass before using the screen dimensions for wap_content.

After that change I expect constraint layout will work. 0dp layout parameter and view sizes for ConstraintLayout before the layout pass will make Glide wait for an onPreDraw() callback and then check again. At that point the layout parameters will probably still be 0, but the View sizes should be valid, which will trigger a load. Subsequent loads will see the pre-existing View size and start a new load anyway.

As a fallback, there's also a new constructor argument for ViewTarget that will force Glide to wait for a layout pass always (in case the View size is invalid or the layout parameters can't be trusted):

Glide.with(fragment)
  .load(url)
  .into(new BitmapImageViewTarget(view, true /*waitForLayout*/));

Which reminds me that I need to pass that boolean through to ViewTarget subclasses. I'll make those changes shortly.