Parallelizing bitmap methods

I created a class called QuickBitmap to quickly get/set pixels in a Bitmap object. Using intertops I'm able to safely lock/unlock the bits of the Bitmap and with the use of methods like SaveBits, LoadBits, UpdateUnderlayingBitmap, GetPixel and SetPixel. I can quickly and easily manipulate the image.

The problem:

One of my methods changes the bitdepth to 32 bits. If I use it I'm no longer able to quickly determine if the image I'm manipulating was, originally, composed of only black and white colors.

I created the following methods do it:

private IEnumerable AllPixels()
{
    for (int i = 0; i  x.IsBlackOrWhite());
}

But as you can guess, it's quite a slow method. So I googled a little and found out that I can break a parallel.for loop.
So I rewrote my method do this:

public bool IsBlackAndWhiteParallel()
{
    bool ret = true;
    Parallel.For(0, Width, (i, loopstate) =>
    {
        for(int j=0; j<Height; j++)
        {
            if(!GetPixel(i,j).IsBlackOrWhite())
            {
                ret = false;
                loopstate.Stop();
            }
        }
    });

    return ret;
}

I benchmarked the code using the benchmark class (I found it in an old question on SO) with 100 iterations:

public static class Benchmarker
{
    public static void Profile(string description, int iterations, Action func)
    {
        // warm up 
        func();

        Stopwatch watch = new Stopwatch();

        // clean up
        GC.Collect();
        GC.WaitForPendingFinalizers();
        GC.Collect();

        watch.Start();
        for (int i = 0; i < iterations; i++)
        {
            func();
        }
        watch.Stop();
        Console.Write(description);
        Console.WriteLine(" average time: {0} ms", watch.Elapsed.TotalMilliseconds/iterations);
    }
}

Results:

• First code: 1170 ms

• Second code: 99 ms

This is how the getpixe/setpixel methods are implemented for 32 bits image:

```
[MethodImpl(MethodImpl

By using BitConverter.ToInt32 you can simplify the GetPixel() method to

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Color GetPixel(int x, int y)
{
    return Color.FromArgb(BitConverter.ToInt32(bytes, y * stride + x * 4));
}

Also there is no need to place () arround the multiplications because they will be done first.

Because GetPixel and SetPixel are public you should add some validation to them. This can be done by adding a private method GetPosition() like

private int GetPosition(int x, int y)
{
    if (x > stride) { throw new ArgumentOutOfRangeException("x"); }

    int position = y * stride + x * 4;

    if (position >= bytes.Length) { throw new ArgumentOutOfRangeException("y"); }
    return position;
}

now the changed GetPixel() method looks like

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Color GetPixel(int x, int y)
{
    return Color.FromArgb(BitConverter.ToInt32(bytes, GetPosition(x, y)));
}

Basically you don't need the Color to determine if a pixel is black or white. You can just check if the argb value of that pixel matches the values for Color.Black or Color.White like

private const int black = -16777216;
private const int white = -1;

public bool IsBlackAndWhite(int x, int y)
{
    int argb = BitConverter.ToInt32(GetPosition(x, y));
    return argb == white || argb == black;
}

You should let your variables some space to breathe.

for(int j=0; j<Height; j++)

should be

for(int j = 0; j < Height; j++)

Your IDE will usually have a keyboard shortcut for formatting the code.

Declaring multiple variables in one line makes your code less readable.

StackExchange Code Review Q#78835, answer score: 4