dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.21k stars 1.74k forks source link

Image caching should be configurable for all ImageSources #9773

Open ClemensFischer opened 2 years ago

ClemensFischer commented 2 years ago

Description

When using StreamImageSource on Android, image cache files are written to a folder named image_manager_disk_cache in FileSystem.CacheDirectory.

Looking for a way to disable this behavior, I found that in src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/PlatformInterop.java the methods loadImageFromFile, loadImageFromStream and loadImageFromFont always pass true to the cachingEnabled argument of the loadInto method.

This seems wrong. Caching should be configurable, as it is implemented for loading an image from an URI string.

Public API Changes

Add a CachingEnabled and a CacheValidity property to the classes FileImageSource, StreamImageSource and FontImageSource, similar to the respective properties in UriImageSource. Perhaps move those properties into their common base class.

Usage:

var imageSource = new StreamImageSource
{
    Stream = ...,
    CachingEnabled = false
};

Alternatively, disable caching for those ImageSources. It seems pointless to cache an image file that is loaded from a local file or stream or font symbol.

Intended Use-Case

In my map control library I am downloading map tiles images via HTTP from map content providers like OpenStreetMap or Bing Maps. It uses StreamImageSource to create ImageSource objects from the HTTP response stream or buffer.

The library implements its own map tile image caching mechanism and does not need any further caching.

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

DavidPressman commented 2 years ago

I would really like to see this issue addressed whenever possible. My app contains a number of images streamed from byte arrays, none of which images can be visibly deleted on user request, apparently because of caching. Only when the page is closed and re-opened is it apparent a deletion occurred. Even worse, after a deletion, when the user tries to add back an image, the old cached one appears. He then has to try the add again to get it replaced. Setting image source to null or to an image source from a stream of byte[0] has no effect. A very disgusting situation!!

DavidPressman commented 2 years ago

To clarify: What I just described is as seen on Android. Not sure about other platforms.

ClemensFischer commented 2 years ago

I am using StreamImageSource to load map tile images from different content providers for a map control. My observation was that (on Android) sooner or later the control began to show wrong images from the cache. Generation of cache keys is obviously broken, and should be fixed.

My current workaround is to always clear the image cache after an ImageSource was created:

var imageManagerDiskCache = Path.Combine(FileSystem.CacheDirectory, "image_manager_disk_cache");

if (Directory.Exists(imageManagerDiskCache))
{
    foreach (var imageCacheFile in Directory.EnumerateFiles(imageManagerDiskCache))
    {
        Debug.WriteLine($"Deleting {imageCacheFile}");
        File.Delete(imageCacheFile);
    }
}
AHComp commented 2 years ago

Why it takes so long to fix such a big bug ? What's the workaround to disable ?

rentanadviser commented 1 year ago
H-A-Hristov commented 1 year ago

This is like p0, maybe p1. Definitely not something for the backlog.

Mielesgames commented 1 year ago

This explains why my local profile picture system keeps loading older profile pictures

rbrettj commented 1 year ago

This is a pretty big issue. Looks like @ClemensFischer gave you the code to change, what's the holdup? Android image caching is messed up right now and needs addressing.

stiaan-netup commented 1 year ago

Very big issue! @jfversluis this is a breaking issue - an upgrade might be needed

jfversluis commented 1 year ago

Looks like we have #13111 already merged. Does that look like it would cover this case as well?

ghost commented 1 year ago

Hi @ClemensFischer. We have added the "s/needs-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

ClemensFischer commented 1 year ago

@jfversluis That is for StreamImageSource only. It still seems pretty pointless to have caching enabled for FileImageSource and FontImageSource.

Symbai commented 1 year ago

File caching isn't pointless. Its a good feature. Especially when a mobile device suddenly lost internet connection. Or if you want to limit your bandwith usage. However this issue requested to make caching customizable so it can be turned off when needed doesn't change existing behavior. I dont think that like #13111 caching here should be turned off as well without giving devs the choice to turn it back on. These are two different scenarios.

ClemensFischer commented 1 year ago

@Symbai That is not true, Loading an image via FileImageSource means that it is loaded from a local image file. Creating another local image file in the cache makes no sense at all. When you say "lost internet connection" you are talking about remote image resources, which are loaded via UriImageSource.

hartez commented 1 year ago

It still seems pretty pointless to have caching enabled for FileImageSource and

Caching a file image source in memory saves you disk I/O time. The underlying library that handles the caching (Glide) does multiple types of caching; it's not just caching to disk. It will also cache images in memory where appropriate.

I agree with your overall point - you should be able to disable caching if you want for a given image source. And we should have overloads which let you customize the keys which are used for caching*. But that would require new APIs, so it's not likely to happen until a major version release.

* (Keys are why #13111 had to turn off caching for streams entirely. URIs and files have natural useful keys; streams don't, so Glide was always re-using images when it shouldn't have been because we weren't giving it decent keys to work with. Until we can design and implement an API that allows users to pass in usable keys for caching streams, we have to turn off caching for them or they won't work at all.)

ClemensFischer commented 1 year ago

@hartez We are not talking about memory cache here. This issue is about cache files in a folder named image_manager_disk_cache in FileSystem.CacheDirectory. This is what just should not be done.

The suggested API change would be easy and compatible: add a CachingEnabled and a CacheValidity property to the classes FileImageSource, StreamImageSource and FontImageSource. Can't see anything complicated with that.

However, I do not really care anymore since I gave up on MAUI, at least for now. May try again next year or so to see if it has evolved into a more mature platform.

idiidi commented 1 year ago

Is there a workaround for this on Android? I am loading an image from file, the image is then changed (keeps the same filename), but the Image is still showing the old image.

Symbai commented 1 year ago

Workaround is to save image as temporary file to disk and give it a unique(!) name. Then load the image from this unique filename.

edgedj commented 1 year ago

wondering if there's any update on this? We have a product mid way through UAT testing for a client and this is an issue for images we generate from static SVG data in our database via API. We're generating ~110 small PNG files from the SVG data each time a change is detected in our static tables using the excellent SkiaSharp library . Giving each one a unique name such as DevOps pipeline build number would be ok but doesn't feel the right thing to do to allow end users to slowly fill up their app folder with old images they won't use anymore. This is only an issue for Android, iOS seems to pick up the image changes immediately.

albilaga commented 1 year ago

This is also breaks our app. We have Image with FontImageSource that FontFamily can be changed. Basically as of right now MAUI is enabled cache by default this make FontImageSource still use old FontFamily. Like now, workaround is doing this https://github.com/dotnet/maui/issues/9773#issuecomment-1236550801 but I do hope this getting fixed. Xamarin.Forms app I believe don't have this issue (maybe Xamarin.Forms not enable caching by default for FontImageSource)

hartez commented 1 year ago

wondering if there's any update on this? We have a product mid way through UAT testing for a client and this is an issue for images we generate from static SVG data in our database via API. We're generating ~110 small PNG files from the SVG data each time a change is detected in our static tables using the excellent SkiaSharp library . Giving each one a unique name such as DevOps pipeline build number would be ok but doesn't feel the right thing to do to allow end users to slowly fill up their app folder with old images they won't use anymore. This is only an issue for Android, iOS seems to pick up the image changes immediately.

Image caching is disabled for images loaded from a stream in the latest versions of MAUI. So you could load those images from streams rather than from files - that would avoid caching.

ClemensFischer commented 1 year ago

Image caching is disabled for images loaded from a stream in the latest versions of MAUI.

Caching should still also be configurable for image files. This is such a ridiculously simple change, why don't you just implement it instead of letting this discussion go on for months? @hartez

blmiles commented 1 year ago

@ClemensFischer 100% agree, complete screws so many apps but no update. @hartez We have to release an Android app by Nov 1 with this bug because there is not other option. MS have dropped the ball on this one. Such a large part of the community need this! Sorry, no justification for it considering so many Xamarin apps are being ported over to Maui and THIS breaks things. Beyond annoyed. I struggle not to be insulting here.

blmiles commented 1 year ago

What a useful library for Maui Apps and this Image caching debacle: https://github.com/Redth/FFImageLoading.Compat Hope that helps a few here!!

Install the nuget package the add this to the MauiProgram.cs:

.UseFFImageLoading() Add this to the xaml page where you might have you Image control(s):

xmlns:ffimageloading="clr-namespace:FFImageLoading.Maui;assembly=FFImageLoading.Compat.Maui"

Substitute the Image xaml for this:

          <ffimageloading:CachedImage CacheType="None" Source="{Binding ImagePath}" Aspect="AspectFit">
              <ffimageloading:CachedImage.HeightRequest>
                  <OnIdiom x:TypeArguments="x:Double" Phone="200" Tablet="375"  />
              </ffimageloading:CachedImage.HeightRequest>                                    
          </ffimageloading:CachedImage>

Amend to suit you reqs...

iamlawrencev commented 11 months ago

Bump, really need this as a workaround using Image streams is not ideal.

Eves101 commented 10 months ago

Looks like we have #13111 already merged. Does that look like it would cover this case as well?

is not fixed yet, saving images to disk with the same filename but different content serves the old file.

RoyM33-T commented 9 months ago

This is still not fixed, I am loading a file editing it and resaving it, It is still serving the original File.

SirJohnK commented 8 months ago

I have the same issue. The ability to set Caching properties for all ImageSources would be much appreciated! 👍

unamed000 commented 8 months ago

We've having the same issue.

There is no clear official API about caching Images and this is involving such a large part of our current Xamarin app if we want to port them over to MAUI!

And for now, we can't use FFImageCompat since it has so many issues and not maintained by anybody. Such fundamental features like this is extremely important for a modern nowadays app.

ThiagoFrancischini commented 8 months ago

Having problems here too...

kallal commented 8 months ago

As noted, one just always have to use a stream.

Remember, MANY mobile applications are for inspection, such as before and after pictures.

For pipe inspections, for accident inspections such as a break in and picture of a broken door, then a fixed door.

Or a medial application in which a picture of a broken leg is taken, and then the after treatment picture is taken. Or as noted a simple picture edit in which some text annotations are injected into the image. So, from pipe inspections, medical applications, insurance applications? Such as a before and after door being repaired?

The "act" of using FromFile for a picture? It should NEVER cache such pictures. From a web based URL, sure I get this goal.

However, NEVER should a simple picture "from file" EVER in ANY use case EVER show the wrong picture PERIOD! In other words, if news gets out that Maui pictures can't be trusted from a legal point of view, then you have a real fire storm on your hands here - you just do! This puts the whole Maui platform at an incredible legal risk here.

Sounds simple, but I can think of boatloads of applications where the assumption of loading a picture using FromFile can't be used, nor relied upon. It is not expected behavior, but worse opens up many issues for inspection and legal type of applications.

As noted, the current workaround is to always load such files from a stream.

I can chalk this up to a learning curve on my part, but loading a picture from a local file should NEVER give rise to any kind of surprise here, and it does.

mariomoramontero commented 7 months ago

Please you have to fix this for the files too

if I save a file with file= "camera_id.png" and read it, and after that I refresh the picture saved with the same name but another content...you would expect that: "ImageSource.FromFile( file );" retrieves the new content NOT the old one, that means that this is somehow cached in memory, and we don't have any way to force a new read unless we try a lot of things like getting the stream, This it's not a expected behaviour in any case, completely nonintuitive and lead us to loose large amount of times investigating this kind of issues...

we have to take full control of the caching of our apps, if not at least you have to provide a way to bypass it.

PD: I'm wondering if this may be the same root error cause than the toolbarIcons not being refreshed, there are many more issues like this

ClemensFischer commented 7 months ago

@hartez It is unbelievable that this is still an open issue.

kallal commented 7 months ago

Please you have to fix this for the files too

if I save a file with file= "camera_id.png" and read it, and after that I refresh the picture saved with the same name but another content...you would expect that: "ImageSource.FromFile( file );" retrieves the new content NOT the old one, that means that this is somehow cached in memory, and we don't have any way to force a new read unless we try a lot of things like getting the stream, This it's not a expected behaviour in any case, completely nonintuitive and lead us to loose large amount of times investigating this kind of issues...

we have to take full control of the caching of our apps, if not at least you have to provide a way to bypass it.

PD: I'm wondering if this may be the same root error cause than the toolbarIcons not being refreshed, there are many more issues like this

You can work around this issue by using a file stream and NOT using ImageSource.FromFile("test.png")

Hence you use this:

        var bytesPic = File.ReadAllBytes(sFile);
       MyImage3.Source = ImageSource.FromStream(() => new MemoryStream(bytesPic));

R Albert

mariomoramontero commented 7 months ago

Yes, what I did in the end is

return ImageSource.FromStream( () =>
 {
    return File.OpenRead( fileName );
 } );

and from within a Converter, so I can use it on my image bindings when I need a real reload.

Image = "{ Binding Path=Image, Converter={StaticResource ImageSourceForceRefresh} }"

This approach works for me

hartez commented 7 months ago

@hartez It is unbelievable that this is still an open issue.

I don't disagree, but I also don't work on MAUI anymore.

This is such a ridiculously simple change, why don't you just implement it instead of letting this discussion go on for months? @hartez

It's harder than you seem to think, but if you want to have a go at it I believe the MAUI team still accepts PRs.

jurganson commented 3 months ago

I also just wanted to bump this thread and let it be known that I would also very much appreciate this feature!

BillFulton commented 3 months ago

Seems to apply also to App Icon and Splash Image in dev context - unclear how to use stream in these two cases as they are declarative.

petarkrastev commented 2 weeks ago

The issue is still persisting! What is going on? It was raised before more than 2 years and still not fixed!!! This is not serious...