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

Placeholder cache with values and values-night resources #3751

Open
willgzn opened this issue Jul 11, 2019 · 7 comments
Open

Placeholder cache with values and values-night resources #3751

willgzn opened this issue Jul 11, 2019 · 7 comments

Comments

@willgzn
Copy link

willgzn commented Jul 11, 2019

Glide Version: 4.8.0
Integration libraries: OkHttp -> 3.9.1
Device/Android Version: Galaxy S8 | Android 9.0 Pie

Issue details / Repro steps / Use case background:

I'm working on a Day/Night application and I have two different placeholders for each mode.

(To understand how values and values-night works, see this : https://medium.com/androiddevelopers/appcompat-v23-2-daynight-d10f90c83e94)

To simplify, I'm using the same resource with two background color :

placeholder.xml
<?xml version="1.0" encoding="utf-8"?> <shape xmlns:android="http://schemas.android.com/apk/res/android" android:shape="rectangle"> <solid android:color="@color/placeholder_background" /> </shape>

values/colors.xml
<color name="placeholder_background">@color/white</color>

values-night/colors.xml
<color name="placeholder_background">@color/black</color>

Glide request
..... new GlideOptions() .placeholder(R.drawable.placeholder) .error(R.drawable.placeholder) .transform(transformation) ......

My issue here is when the user is in the day mode, a white placeholder will be loaded and displayed, but if the user change to night mode, the placeholder will stay in cache and won't change to the night version of the placeholder.

I know there is a glide cache for images but I don't know how to clear the placeholder cache.

If you have any idea to clear this resource and display the right one.

@stale
Copy link

stale bot commented Jul 18, 2019

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

@stale stale bot added the stale label Jul 18, 2019
@stale stale bot removed the stale label Jul 22, 2019
@sjudd
Copy link
Collaborator

sjudd commented Jul 22, 2019

Do you have a sample app that shows how this works?

Glide doesn't actually cache placeholders, we just call Resources#getDrawable. It is possible though that the request itself is being cached:

if (request.isEquivalentTo(previous)

@michaelbukachi
Copy link

@skylive Checkout my answer in issue #3778

@sjudd
Copy link
Collaborator

sjudd commented Aug 14, 2019

Internal bug is b/139004348

@sjudd
Copy link
Collaborator

sjudd commented Aug 14, 2019

Oh I take it back, there is no internal bug for this. The issue here is probably that we use the application context to load the placeholders, rather than the Activity context. Still need to investigate that.

@sjudd
Copy link
Collaborator

sjudd commented Oct 10, 2022

There's a workaround for this here: #3778 (comment). It won't work when actually loading a resource Drawable with Glide, only for placeholder/fallback/error.

sjudd added a commit to sjudd/glide that referenced this issue Oct 11, 2022
Previously we'd use the application Context, which would not use the
Activity theme. That would in turn mean we would not customize Drawables
for light / dark mode.

I've also added a bunch of emulator tests. It turns out these changes
aren't quite sufficient, even with bumptech#4842, to get asynchronous loading
of resources passed to load() working. I've left a pretty extensive
comment in the new test along with ignored tests about what additional
changes we'd need.

Progress towards bumptech#3751.
sjudd added a commit to sjudd/glide that referenced this issue Oct 11, 2022
Previously we'd use the application Context, which would not use the
Activity theme. That would in turn mean we would not customize Drawables
for light / dark mode.

I've also added a bunch of emulator tests. It turns out these changes
aren't quite sufficient, even with bumptech#4842, to get asynchronous loading
of resources passed to load() working. I've left a pretty extensive
comment in the new test along with ignored tests about what additional
changes we'd need.

Progress towards bumptech#3751.
sjudd added a commit to sjudd/glide that referenced this issue Oct 11, 2022
Applying the Theme allows us to respect the Theme, including dark or
light mode resources. We're making the same compromise here that we've
made for other model types. Specifically we're only applying this
behavior if the non-generic version of the method is used, ie the type
of the model is known at compile time.

We could try to do this at a lower level, but there's some additional
risk of confusion about when or why the Theme is or isn't available.
While this isn't perfectly consistent, the behavior can at least be well
documented.

There's also some risk that passing the Context's Resources / Theme
classes to a background thread will result in transient memory leaks. I
don't immediately see a direct link between either class and the
enclosing Context, but it's hard to be certain.

Progress towards bumptech#3751
sjudd added a commit to sjudd/glide that referenced this issue Oct 21, 2022
Previously we'd use the application Context, which would not use the
Activity theme. That would in turn mean we would not customize Drawables
for light / dark mode.

I've also added a bunch of emulator tests. It turns out these changes
aren't quite sufficient, even with bumptech#4842, to get asynchronous loading
of resources passed to load() working. I've left a pretty extensive
comment in the new test along with ignored tests about what additional
changes we'd need.

Progress towards bumptech#3751.
sjudd added a commit to sjudd/glide that referenced this issue Dec 16, 2022
Applying the Theme allows us to respect the Theme, including dark or
light mode resources. We're making the same compromise here that we've
made for other model types. Specifically we're only applying this
behavior if the non-generic version of the method is used, ie the type
of the model is known at compile time.

We could try to do this at a lower level, but there's some additional
risk of confusion about when or why the Theme is or isn't available.
While this isn't perfectly consistent, the behavior can at least be well
documented.

There's also some risk that passing the Context's Resources / Theme
classes to a background thread will result in transient memory leaks. I
don't immediately see a direct link between either class and the
enclosing Context, but it's hard to be certain.

Progress towards bumptech#3751
sjudd added a commit to sjudd/glide that referenced this issue Dec 21, 2022
Applying the Theme allows us to respect the Theme, including dark or
light mode resources. We're making the same compromise here that we've
made for other model types. Specifically we're only applying this
behavior if the non-generic version of the method is used, ie the type
of the model is known at compile time.

We could try to do this at a lower level, but there's some additional
risk of confusion about when or why the Theme is or isn't available.
While this isn't perfectly consistent, the behavior can at least be well
documented.

There's also some risk that passing the Context's Resources / Theme
classes to a background thread will result in transient memory leaks. I
don't immediately see a direct link between either class and the
enclosing Context, but it's hard to be certain.

Progress towards bumptech#3751
sjudd added a commit to sjudd/glide that referenced this issue Dec 29, 2022
Applying the Theme allows us to respect the Theme, including dark or
light mode resources. We're making the same compromise here that we've
made for other model types. Specifically we're only applying this
behavior if the non-generic version of the method is used, ie the type
of the model is known at compile time.

We could try to do this at a lower level, but there's some additional
risk of confusion about when or why the Theme is or isn't available.
While this isn't perfectly consistent, the behavior can at least be well
documented.

There's also some risk that passing the Context's Resources / Theme
classes to a background thread will result in transient memory leaks. I
don't immediately see a direct link between either class and the
enclosing Context, but it's hard to be certain.

Progress towards bumptech#3751
@drinkthestars
Copy link

drinkthestars commented Jul 25, 2023

Hi! Was this issue fixed in v4.15.0? Looks like the release notes say:

Improved support for dark mode, RTL and other theme attributes on resources loaded with Glide (eab4c37, 04f198e, e6f5eec, a912e0f, 9668157, 31821f5, f73f003)

We are still on v4.14.2 so would love to know if we should bump version since we're also running into this issue wrt dark mode...

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

4 participants