0

I'm working in a WPF application where I show my images in two places which means the same image gets loaded in two places. In one of the place the image will be shown along with other few images in a slider where it will be able to edit and save. If there is no image available in the location I should be showing a separate image Image not found which is not editable.

When I started working on the functionality I got the Used by another process exception during edit and save. So after searching I came up with a solution and now at a rare time I get the Out of memory exception when I click the Next or Previous or First or Last in slider. The slider is just an Image control with 4 buttons. When the buttons are clicked the below method is called. I'm not sure if there is any memory leaks.

bool NoImage = true;
private static readonly object _syncRoot = new object();
private BitmapSource LoadImage(string path)
{
    lock (_syncRoot) //lock the object so it doesn't get executed more than once at a time.
    {
        BitmapDecoder decoder = null;

        try
        {
            //If the image is not found in the folder, then show the image not found.
            if (!File.Exists(path) && (path != null))
            {
                System.Drawing.Bitmap ss = XXXX.Resources.ImageNotFound;
                var stream = new System.IO.MemoryStream();
                if (!File.Exists(Path.GetTempPath() + "ImageNotFound.jpg"))
                {
                    FileStream file = new FileStream(Path.GetTempPath() + "ImageNotFound.jpg", FileMode.Create, FileAccess.Write);
                    ss.Save(stream, ImageFormat.Jpeg);
                    stream.Position = 0;
                    stream.WriteTo(file);
                    file.Close();
                    stream.Close();
                }

                path = Path.Combine(Path.GetTempPath(), "ImageNotFound.jpg");
                NoImage = false;
            }
            else
            {
                if (!EnableForEdit)
                    NoImage = false;
                else
                    NoImage = true;
            }

            if (!string.IsNullOrEmpty(path) && (!NoImage || File.Exists(path)))
            {
                using (var stream = new FileStream(path, FileMode.Open, FileAccess.Read))
                {
                    decoder = BitmapDecoder.Create(stream, BitmapCreateOptions.None, BitmapCacheOption.OnLoad);

                }
                return decoder.Frames.FirstOrDefault();

            }
            else
                return null;
        }
        catch (OutOfMemoryException ex)
        {
            MessageBox.Show("Insufficient memory to handle the process. Please try again later.", "Application alert");                  

            return null;
        }
        catch (Exception ex)
        {
            // Error handling.
            throw new ApplicationException(ex.Message);
        }
        finally
        {
            decoder = null;
            GC.WaitForFullGCComplete(1000);
            GC.Collect(0, GCCollectionMode.Forced);
        }
    }
}


<Image x:Name="viewImage" Grid.Row="2" Height="100" Width="135" Source="{Binding DisplayImage, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged, NotifyOnSourceUpdated=True}" />

If my approach is wrong, let me know where should I do the change or if there is any simpler way to do. Kindly help.

Note: The images which are loaded is above 5Mb

A Coder
  • 3,039
  • 7
  • 58
  • 129
  • 3
    first thing i would suggest is to shift to lazy loading also dispose your streams by wrapping them in a using statement – MikeT Jul 19 '16 at 10:57
  • `The images which are loaded is above 5Mb` - as in around 5MB or as in could also be 500MB (*which is also above 5mb*)? – Igor Jul 19 '16 at 11:04
  • i would also suggest using binding rather than code behind, when you specify a bound value the binding gives you 2 option for alternate values (FallbackValue and TargetNullValue) if you set one or both of these to a predefined Not Found image resource then most of this work has been done for you – MikeT Jul 19 '16 at 11:05
  • Also is `_syncRoot` a static or instance variable? If its instance do you have mulitple instances of the class being loaded at once or is the class only loaded 1x. – Igor Jul 19 '16 at 11:05
  • @MikeT Lazy loading in the slider means? `_synRoot` is for, at times the user is too fast to slide up the images. Multiple clicks on the buttons in second. Updated ques for that. – A Coder Jul 19 '16 at 11:34
  • @Igor both the images are between 5-10Mb. – A Coder Jul 19 '16 at 11:34
  • Lazy loading means to not load the image before it is needed, that way if you have 500 items in the slider you don't load them until just before the item is displayed, this will minimise the memory footprint of your app – MikeT Jul 19 '16 at 11:46
  • @MikeT Max i load only 20 images at a time and so i think its isn't needed. Is that right? – A Coder Jul 19 '16 at 11:58
  • I've stuck an example in the answers to show you what i mean – MikeT Jul 19 '16 at 13:02

1 Answers1

4

Firstly when ever you create a stream you need to dispose of it once you are finished with it (Note Close does not Dispose, but Dispose does close), if not then the stream stays in memory consuming resources

so your code should look as follows

using(var stream = new System.IO.MemoryStream())
{
    if (!File.Exists(Path.GetTempPath() + "ImageNotFound.jpg"))
    {
        using(FileStream file = new FileStream(Path.GetTempPath() + "ImageNotFound.jpg", FileMode.Create, FileAccess.Write))
        {
            ss.Save(stream, ImageFormat.Jpeg);
            stream.Position = 0;
            stream.WriteTo(file);
        }
    }
}

Second you need to reduce your apps memory impact

to do that i would suggest leveraging the functionality already in WPF here is a quick example of how you should do this

Your Model

public class ImageItem
{
    public Uri URI{ get; set; }

    private BitmapSource _Source;

    public BitmapSource Source
    {
        get
        {
            try
            {
                if (_Source == null) _Source = new BitmapImage(URI);//lazy loading

            }
            catch (Exception)
            {
                _Source = null;
            }
            return _Source;
        }
    }
    public void Save(string filename)
    {
        var img = BitmapFrame.Create(Source);
        var encoder = new JpegBitmapEncoder();

        encoder.Frames.Add(img);
        using(var saveStream = System.IO.File.OpenWrite(filename))
            encoder.Save(saveStream)

    }


}

Your View Model

public class ImageList : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;
    public ObservableCollection<ImageItem> Images { get; } = new ObservableCollection<ImageItem>();

    private int _SelectedIndex;
//  c# >= 6
    public static readonly PropertyChangedEventArgs SelectedIndexProperty = new PropertyChangedEventArgs(nameof(SelectedIndex));
//  c# < 6
//  public static readonly PropertyChangedEventArgs SelectedIndexProperty = new PropertyChangedEventArgs("SelectedIndex");

    public int SelectedIndex
    {
        get { return _SelectedIndex; }
        set
        {
            _SelectedIndex = value;
//          c# >= 6
            PropertyChanged?.Invoke(this, SelectedIndexProperty);
            PropertyChanged?.Invoke(this, CurrentImageProperty);
//          c# < 6
//          var handler = PropertyChanged;
//          if(handler !=null)
//          {
//              handler (this, SelectedIndexProperty);
//              handler (this, CurrentImageProperty);
//          }
        }
    }

//  c# >= 6
    public static readonly PropertyChangedEventArgs CurrentImageProperty = new PropertyChangedEventArgs(nameof(CurrentImage)); 
//  c# < 6
//  public static readonly PropertyChangedEventArgs CurrentImageProperty = new PropertyChangedEventArgs("CurrentImage"); 

    public ImageItem CurrentImage => Images.Count>0 ?  Images[SelectedIndex] : null;

    public void Next()
    {
        if (SelectedIndex < Images.Count - 1)
            SelectedIndex++;
        else
            SelectedIndex = 0;
    }
    public void Back()
    {
        if (SelectedIndex == 0)
            SelectedIndex = Images.Count - 1;
        else
            SelectedIndex--;
    }

    public void Add(string Filename)
    {
        Images.Add(new ImageItem() { URI= new Uri(Filename) });
//      c# >= 6
        PropertyChanged?.Invoke(this, CurrentImageProperty);
//      c# < 6
//      var handler = PropertyChanged;
//      if(handler !=null)
//      {
//          handler (this, CurrentImageProperty);
//      }
    }
}

and Finally your View

<Window x:Class="ImageDemo.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:ImageDemo"
        mc:Ignorable="d"
        Title="MainWindow" Height="350" Width="525">
    <Window.Resources>
        <BitmapImage x:Key="NotFound" UriSource="C:\...\NotFound.png"/>
    </Window.Resources>
    <Window.DataContext>
        <local:ImageList/>
    </Window.DataContext>
    <DockPanel>
        <Button Content="&lt;" Click="Back_Click"/>
        <Button DockPanel.Dock="Right" Content="&gt;" Click="Next_Click"/>
        <Image Source="{Binding CurrentImage.Source, Mode=OneWay, 
               TargetNullValue={StaticResource NotFound},
               FallbackValue={StaticResource NotFound}}"/>
    </DockPanel>
</Window>

public partial class MainWindow : Window
{
    public MainWindow()
    {
        InitializeComponent();
    }

    //c# >= 6
    private ImageList list => DataContext as ImageList;
    //c# < 6
    //private ImageList list {get{ return DataContext as ImageList;}}

    private void Next_Click(object sender, RoutedEventArgs e)
    {
        list.Next();
    }

    private void Back_Click(object sender, RoutedEventArgs e)
    {
        list.Back();
    }
}

note: because the Model is separate to the View you can show the same image in several places with no issue at all

also System.Drawing.Bitmap is not WPF compatible so you should use the WPF classes in System.Windows.Media.Imaging

MikeT
  • 5,398
  • 3
  • 27
  • 43
  • Thanks a ton for your post. Can you please post it in 4.5 framework?. I'm not aware of 6.0. – A Coder Jul 21 '16 at 04:53
  • your mixing up the .net version and the c# version, they are independent, i've added notes on how to replace the nameof keyword with old style c# – MikeT Jul 21 '16 at 08:38
  • if you can i would suggest upgrading to VS2015, especially if your using WPF, it is so much more stable. and once you have VS2015 you can use the Roslyn compiler which supports c# 6.0 – MikeT Jul 21 '16 at 08:48
  • Yes, once i started to edit the time limit is expired.Right now i'm working in VS2012. Starting working to modify my code to this sample. Will be back sooner. – A Coder Jul 21 '16 at 09:08
  • I use a custom class Collection which has the path. How can i modify this? `public ImageItem CurrentImage => Images.Count>0 ? Images[SelectedIndex] : null;` – A Coder Jul 21 '16 at 09:12
  • you will need to Implement `INotifyCollectionChanged` in your collection class otherwise any bindings wont be aware of changes otherwise a custom collection is fine, however if you create the ImageItem class with the path then you don't need to store the path elsewhere – MikeT Jul 21 '16 at 09:17
  • i suggest reading https://msdn.microsoft.com/en-gb/library/hh848246.aspx it has a good description of the theory behind MVVM coding which is the best way to use WPF – MikeT Jul 21 '16 at 09:25
  • When i hit Next or Back the `ProertyChanged` is `null`. During Add the `PropertyChanged` is ok but the handler isn't firing the propery(CurrentImageProperty). Can you please help? – A Coder Jul 26 '16 at 06:54
  • the only reason the handler should be null is that there is nothing attached to it, which means you've probably got the binding attached incorrectly, or you haven't added the null clause and your code is erroring before it can be attached so make sure you have the ? in the `PropertyChanged?.Invoke` or that you have the `if(handler !=null)` – MikeT Jul 26 '16 at 08:49
  • if the problem is the binding then i would suggest checking the following, xmlns:local="..." is pointing at the right namespace in your project, you have set the data context of the Image to the imagelist object, i did this at the window level but any level would be fine otherwise you need to specify the source as the ImageList class, finally check the binging with as debug converter https://spin.atomicobject.com/2013/12/11/wpf-data-binding-debug/ – MikeT Jul 26 '16 at 09:00
  • Altered few things but still this piece `handler(this, CurrentImageProperty);` isn't firing the property. – A Coder Jul 26 '16 at 13:38
  • afraid i can't suggest more with out seeing your code, the code as provided was working for me – MikeT Jul 26 '16 at 16:51
  • Have posted as a new question. Can you please take a look? http://stackoverflow.com/questions/38604724/wpf-image-slider-mvvm – A Coder Jul 27 '16 at 05:52
  • had a look and the problem is you removed `private ImageList list => DataContext as ImageList;` and replaced with `private ImageList list= new ImageList();` so you have 2 image lists not 1 – MikeT Jul 27 '16 at 14:58
  • Yes, Have resolved and working fine. I need a few more tests with it. Let you know if i come across any issues so that u can help. Thanks – A Coder Jul 28 '16 at 06:05
  • that's what stack overflow is here for, wpf is a very powerful tool but like all powerful tools its very easy to go wrong with and there are so many different ways to accomplish the same task it can be confusing – MikeT Jul 28 '16 at 09:39
  • When i edit the image using `mspaint` and save, i get `sharing violation`. To avoid this i have already used a method mentioned here http://stackoverflow.com/questions/19346273/cannot-delete-file-when-used-in-datacontext/19347459. But again when i use those cache, will i get the `insufficient memory exception`? Why i'm asking is its like the same process before. – A Coder Jul 28 '16 at 11:53
  • My code only deals with showing the image, if you are going to edit it then you are probably going to use an `InkCanvas`, which is very good for storing the changes you make,, but a nightmare to merge them back into the original image, i would suggest creating a VeiwModel that has the ImageItem as the original image and then a Stroke collection of changes, the save will require you to **render** the original and the strokes into a new image and then save that – MikeT Jul 28 '16 at 14:31
  • I'm not sure what is that. Will post it as a new question. Kindly help – A Coder Jul 29 '16 at 05:58
  • Posted as a new question. http://stackoverflow.com/questions/38652086/image-edit-exceptions-wpf – A Coder Jul 29 '16 at 06:42