HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Go through every pixel of an image and then dispose of it

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
imagepixeleverythenthroughanddispose

Problem

I'm a little confused as to whether implementing the Dispose method serves a purpose in this code. It never gets referenced by any other class in my project, so should I simply get rid of it or should I make an explicit call to Dispose when releasing the bitmap image?

```
namespace ImageProcessor
{
///
/// A bitmap class that allows fast x, y access
///
public unsafe class UnsafeBitmap
{
Bitmap source;
int width;
BitmapData bmpData = null;
IntPtr baseLine = IntPtr.Zero;
BGRA* pCurrentPixel = null;
Point size;
internal bool locked = false;

///
/// Create an instance from an existing bitmap
///
/// The source bitmap
public UnsafeBitmap(Bitmap src)
{
source = src;
GraphicsUnit units = GraphicsUnit.Pixel;
RectangleF boundsF = src.GetBounds(ref units);
size = new Point((int)boundsF.Width, (int)boundsF.Height);
}

public Bitmap Bitmap
{
get { return (source); }
set { source = value; }
}

///
/// Lock bitmap data
///
public void LockBitmap()
{
try
{
GraphicsUnit units = GraphicsUnit.Pixel;
RectangleF boundsF = source.GetBounds(ref units);
Rectangle bounds = Rectangle.Round(boundsF);

// For a standard BMP, each individual scan line needs to be a multiple
// of 4 bytes, so when we have a 24 bit image (3 bytes per pixel),
// we need to allow for padding at the end of every scan line
// to bring it up to a multiple of 4.
width = (int)boundsF.Width * sizeof(BGRA);

Solution

I think your Bitmap property shouldn't have a setter. If you use it, you don't dispose the old Bitmap and you don't reset size. Both seem to be erroneous to me.

public Bitmap Bitmap
{
    get { return (source); }
    set { source = value; }
}


Why is source in parentheses?

Also, this could be an auto-property. You would then use the (possibly private) setter instead of the field in the constructor.

catch(Exception)
{
    throw;
}


This doesn't do anything, get rid of it.

public BGRA* this[int x, int y]
{
    get
    {
        return (BGRA*)(baseLine + y * width + x * sizeof(BGRA));
    }
}


If the type of baseLine was BGRA* (and you also changed width not to include the size), then the return statement could be simplified to:

return &baseLine[y * width + x];


Also, I would consider returning BGRA directly, I think that's more natural (assuming doing that doesn't decrease performance noticeably).

Code Snippets

public Bitmap Bitmap
{
    get { return (source); }
    set { source = value; }
}
catch(Exception)
{
    throw;
}
public BGRA* this[int x, int y]
{
    get
    {
        return (BGRA*)(baseLine + y * width + x * sizeof(BGRA));
    }
}
return &baseLine[y * width + x];

Context

StackExchange Code Review Q#81867, answer score: 5

Revisions (0)

No revisions yet.