0
votes

I am working on WriteableBitmaps and I have a method that:

  1. Copies WriteableBitmap from parameter to variable that is outside method
  2. Works on first bitmap
  3. Adds copied bitmap to UndoStack

The point of this method is to make changes on bitmap and after that add to undo stack bitmap before changes.

If this method is called very often it causes OutOfMemoryException.

Variables

private WriteableBitmap _oldBitmap;
private Image _oldImage;

Method

public Layer ExecuteTool(Layer layer, Coordinates startingCoords, Color color,int toolSize, ToolType tool)
{
    if (toolSize < 1) return null;
    Layer cLayer = layer;

    _oldBitmap = new WriteableBitmap(layer.LayerBitmap);
    _oldImage = layer.LayerImage;
    _oldImage.Source = _oldBitmap;

    switch (tool)
    {
        case ToolType.Pen:
            cLayer.LayerBitmap = DrawPixel(cLayer.LayerBitmap, startingCoords, toolSize,color);
            break;
        case ToolType.Bucket:
            cLayer.LayerBitmap = FloodFill(cLayer.LayerBitmap, startingCoords, color);
            break;
        case ToolType.Line:
            if (_toolIsExecuting == false)
            {
                LineAsync(cLayer, startingCoords, color, toolSize);
            }
            break;
        case ToolType.Circle:
            if(_toolIsExecuting == false)
            {
                CircleAsync(cLayer, startingCoords, color);
            }
            break;
        case ToolType.Rectangle:
            if(_toolIsExecuting == false)
            {
                RectangleAsync(cLayer, startingCoords, color);
            }
            break;              
        case ToolType.Earser:
            cLayer.LayerBitmap = DrawPixel(cLayer.LayerBitmap, startingCoords, toolSize, Colors.Transparent);
            break;
        case ToolType.Lighten:
            if(Mouse.LeftButton == MouseButtonState.Pressed)
            {
                cLayer.LayerBitmap = Lighten(cLayer.LayerBitmap, startingCoords);
            }
            else if(Mouse.RightButton == MouseButtonState.Pressed)
            {
                cLayer.LayerBitmap = Darken(cLayer.LayerBitmap, startingCoords);
            }
            break;
        default:
            break;
    }
    if (tool != ToolType.ColorPicker)
    {
        UndoManager.RecordChanges("ActiveLayer", new Layer(_oldBitmap, _oldImage), cLayer, string.Format("{0} Tool.", tool.ToString()));
    }

    return cLayer;
}

PS. It doesn't work without copying bitmap

1
So, why is this a memory leak? Seems like your code is working as designed. You may need to consider a different architecture, such as one where you store an initial image and a series of incremental modifications, them implement "Undo" by redoing from when the last complete version of the image was cached in the undo stream. Or, you could move items from your "Undo" stream onto disk via serialization when you have more than, say, 10 undo steps saved. - dbc

1 Answers

0
votes

Bitmaps are notoriously big in size. You keep adding old states to a undo stack, preventing the GC from cleaning them up. That is exactly what you should not be doing with bulk data like images.

I think you just accidentally made an example on what not to do there :)

Solution:

Stop keeping lots of unused images! Put some sensible limits to the undo stack size in place. Or solve this problem in a way that does not need MiB's worth of ram.

One thing to keep in mind in particular is that images in memory are entirely uncompressed. Compression only applies to images on disks or being transfered over a network. In order to display or process a image, every comrpession step has to be undone. And they take space equal to Height X Widht X color depth, as a uncompressed Bitmap image would.