Return to Answer

Please read are-regions-an-antipattern-or-code-smell

Please read are-regions-an-antipattern-or-code-smell

Instead of checking the PixelFormat and for every GetColor(...)/SetColor(...) and deciding if I should read 3 or 4 bytes, I just use a class that knows exactly how to read/write a color. Think about it. For a 1000x1000 image I'm saving 1000000 checks.

How about

 public FastBitmap(Bitmap bmp)
 : this(bmp, bmp.PixelFormat)
 { }
 private readonly int bytesPerPixel;
 public FastBitmap(Bitmap bmp, PixelFormat pixelFormat)
 {
 if (bmp.PixelFormat != pixelFormat) throw new FormatException("The bitmap has a invalid PixelFormat. Expected " + pixelFormat.ToString() + ".");
 bytesPerPixel = GetBytesPerPixel(pixelFormat);
 this.bmp = bmp;
 this.Width = bmp.Width;
 this.Height = bmp.Height;
 this.PixelFormat = bmp.PixelFormat;
 this.validArea = new Rectangle(0, 0, Width, Height);
 LoadBits();
 }
 protected FastBitmap(Bitmap bmp, int bytesPerPixel)
 {
 this.bytesPerPixel = bytesPerPixel;
 this.bmp = bmp;
 this.Width = bmp.Width;
 this.Height = bmp.Height;
 this.PixelFormat = bmp.PixelFormat;
 this.validArea = new Rectangle(0, 0, Width, Height);
 LoadBits();
 }
 private int GetBytesPerPixel(PixelFormat pixelFormat)
 {
 switch (pixelFormat)
 {
 case System.Drawing.Imaging.PixelFormat.Format24bppRgb:
 return 3;
 case System.Drawing.Imaging.PixelFormat.Format32bppArgb:
 return 4;
 case System.Drawing.Imaging.PixelFormat.Format16bppArgb1555:
 return 2;
 case System.Drawing.Imaging.PixelFormat.Format48bppRgb:
 return 6;
 }
 throw new NotSupportedException("The pixelformat " + pixelFormat.ToString() + " is not supported");
 }
 private void SetColorNoException(int x, int y)
 {
 index = (y * stride) + (x * bytesPerPixel);
 scan0AsIntPointer[index / bytesPerPixel] = currentColor.ToArgb();
 }
 public void SetColor(int x, int y, System.Drawing.Color c)
 {
 if (x < 0 || x >= Width) throw new ArgumentOutOfRangeException("x");
 if (y < 0 || y >= Height) throw new ArgumentOutOfRangeException("y");
 index = (y * stride) + (x * bytesPerPixel);
 scan0AsIntPointer[index / bytesPerPixel] = c.ToArgb();
 }
 private System.Drawing.Color GetColorNoException(int x, int y)
 {
 index = (y * stride) + (x * bytesPerPixel);
 return Color.FromArgb(scan0AsIntPointer[index / bytesPerPixel]);
 } 

you only have one check in the constructor and it can be used with all the pixelformats you define.

Lets assume you want to add support for Format48bppRgb you still can do it by either changing the switch in the GetBytesPerPixel()method or by extending the class and call the protected constructor with 6 as bytesPerPixel parameter.

Pixel class

• public Point Location { get; private set; } and this.Location = position; doesn't match that good. Why don't you rename either the passed in parameter to location or the property to Position?

• you have a overloaded constructor taking only a Point which calls a constructor with a default color but you only have a constructor which takes (int x, int y, Color c) and none with a no color. IMHO you should provide this also.

abstract class FastBitmap

/// <summary>
/// Used with a scan0 pointer to get/set colors.
/// </summary>
protected int index = 0; 

this variable is nowhere used in the abstract class, so it doesn't belong there. You should move it to the implementation and IMHO you won't need this at class level either. Make it a method scoped variable.

unsafe sealed class FastBitmap32

Here you have the magic number 4 in your code. You should use a const for that with a meaningful name. But you also could do better by removing the class at all and change the FastBitmap class to a non abstract one.

You can easiliy (with a simple switch) determine how many bytes is consumed by a color for a passed in PixelFormat. That would completely remove the need to have an abstract class.

Regions

Please read are-regions-an-antipattern-or-code-smell

Is there a good use for regions?

No. There was a legacy use: generated code. Still, code generation tools just have to use partial classes instead. If C# has regions support, it's mostly because this legacy use, and because now that too many people used regions in their code, it would be impossible to remove them without breaking existent codebases.

Think about it as about goto. The fact that the language or the IDE supports a feature doesn't mean that it should be used daily. StyleCop SA1124 rule is clear: you should not use regions. Never.

Pixel class

• public Point Location { get; private set; } and this.Location = position; doesn't match that good. Why don't you rename either the passed in parameter to location or the property to Position?

• you have a overloaded constructor taking only a Point which calls a constructor with a default color but you only have a constructor which takes (int x, int y, Color c) and none with a no color. IMHO you should provide this also.

abstract class FastBitmap

/// <summary>
/// Used with a scan0 pointer to get/set colors.
/// </summary>
protected int index = 0; 

this variable is nowhere used in the abstract class, so it doesn't belong there. You should move it to the implementation and IMHO you won't need this at class level either. Make it a method scoped variable.

This

Rectangle rect = new Rectangle(0, 0, bmp.Width, bmp.Height);

in the LoadBits() method isn't used at all and should be removed.

unsafe sealed class FastBitmap32

Here you have the magic number 4 in your code. You should use a const for that with a meaningful name. But you also could do better by removing the class at all and change the FastBitmap class to a non abstract one.

You can easiliy (with a simple switch) determine how many bytes is consumed by a color for a passed in PixelFormat. That would completely remove the need to have an abstract class.

Regions

Please read are-regions-an-antipattern-or-code-smell

Is there a good use for regions?

No. There was a legacy use: generated code. Still, code generation tools just have to use partial classes instead. If C# has regions support, it's mostly because this legacy use, and because now that too many people used regions in their code, it would be impossible to remove them without breaking existent codebases.

Think about it as about goto. The fact that the language or the IDE supports a feature doesn't mean that it should be used daily. StyleCop SA1124 rule is clear: you should not use regions. Never.