CODEWITHSUNDEEP
c#.netwinformspicturebox
Kai
Kai

Reputation: 41

Out of memory in image viewer despite using Image.Dispose

When adding a Next and Previous navigation option to my Image viewer coded in C#, when I press Next about 20 or so times, Visual Studio tells me the process ran out of memory. It does this in any folder with many even if the image file sizes for them all are tiny

I get:

An unhandled exception of type 'System.OutOfMemoryException' occurred in System.Drawing.dll Additional information: Out of memory.

This is the code I am using

private void next_Click(object sender, EventArgs e)
{
    string[] foldernm = Directory.GetFiles(Path.GetDirectoryName(lfoto_file.FileName));
    _pictureIndex++;
    if (_pictureIndex >= foldernm.Length)
    {
        _pictureIndex = 0;
    }
    ibread_img.Image.Dispose();
    ibread_img.Image = Image.FromFile(foldernm[_pictureIndex]);
}

Now as you can see, I have ibread_img.Image.Dispose(); there because I have searched about this and other people said to use that, but it doesn't work and I still get the same problem, a break-point confirms the code is being ran so I am confused to why its still running out of memory. The images I am cycling through are not large. I have tried everything I could find including nulling the previously loaded image, manually calling the garbage collector and nothing seems to work. I am not the best at C# so there might be a horrible mistake or flaw in that code but I don't know, any ideas on how to fix this?

Upvotes: 2

Views: 230

Answers (2)

Ňɏssa Pøngjǣrdenlarp
Ňɏssa Pøngjǣrdenlarp

Reputation: 38865

There are a couple of things you could do to improve your viewer. First, you are recreating the list of image files every time; you are loading all of them each time just to access the next one and you dont have to create an image in order to show it.

// class level vars
int picIndex = 0;
IEnumerable<string> files;
int filesCount;
string picPath;
static string[] imgExts = {".png", ".jpg",".gif"};

Since you mentioned a Next and Previous button, you must have almost the same code elsewhere. This will eliminate that duplication, Next:

ShowImage(picIndex);

picIndex+=1;
if (picIndex >= filesCount)
    picIndex = 0;

Then a method to show the desired image:

private void ShowImage(int Index)
{ 
    // create image list if needed (once)
    if (files == null)
    { 
        files = new DirectoryInfo(picPath).EnumerateFiles().
            Where(q => imgExts.Contains(q.Extension.ToLowerInvariant())).
            Select( z => z.FullName);

        filesCount = files.Count();
    }

    string thisFile = files.ElementAt(Index);

    // no need to dispose an image if you never create one          
    pb2.ImageLocation = thisFile;
    lblImgName.Text = Path.GetFileName(thisFile);
}

Rather that create the list of files each time (in 2 places) this does it once ever, and instead of loading a List of all of them, this leaves it as IEnumerable to get them as needed. It also works off FileInfo, is case insensitive mainly to illustrate a different way which would allow you to sort them (OrderBy) by the created date, if you wanted.

Finally, given the full path and file name, you can use the .ImageLocation property and avoid creating and disposing of Images.

The main thing is to minimize the amount of repeated code so you Dont Repeat Yourself. The code for Next and Previous is going to be almost identical.

Upvotes: 1

Kai
Kai

Reputation: 41

Thanks to LarsTech and Plutonix for pointing out my mistake. This new code works fine now:

private void next_Click(object sender, EventArgs e)
    {
        var filteredFiles = Directory.EnumerateFiles(Path.GetDirectoryName(lfoto_file.FileName))
            .Where(file => file.ToLower().EndsWith("jpg") || file.ToLower().EndsWith("png") || file.ToLower().EndsWith("gif") || file.ToLower().EndsWith("bmp") || file.ToLower().EndsWith("tiff") || file.ToLower().EndsWith("ico"))
            .ToList();
        _pictureIndex++;
        if (_pictureIndex >= filteredFiles.Count)
        {
            _pictureIndex = 0;
        }
        ibread_img.Image.Dispose();
        ibread_img.Image = Image.FromFile(filteredFiles[_pictureIndex]);
        init();
    }

I just needed to filter out the correct formats.

Upvotes: 0

Related Questions