Imaging with getPixel() and loops I wrote perfectly working code that reads an image, and then does a kind of threshold on it by zone. If the zone has too much white then it turns all the zone into white, otherwise it turns into black.

The problem is that the code was originally from a C++ source with pointers and all instead of GetPixel() etc. Speed was just instant, nothing more. Now that I rewrote "safely" (no pointers thingy), it take like 20 seconds per images, so instant versus 20 seconds = insane difference.

Could any of you help me improve that small code, even if you use back unsafe pointers code etc? I don't really care about that. The part I absolutely need is speed, and it has to be quick as possible because I will deal with huge images library. Instead of taking 30 minutes, it will literally takes days.

    private Bitmap doti(Bitmap imag)
    {
        int threshold = 7;
        int distance = 9;
        int rows, cols, row, col, x, y, count;
        int dhalf = (distance / 2) + 1;
        int Sqdistance = SQ(distance);
        rows = imag.Height;
        cols = imag.Width;
        Bitmap bitmap0 = (Bitmap)imag.Clone();
        Bitmap bitmap = new Bitmap(cols, rows);
        Bitmap outmap = new Bitmap(cols, rows);

        //convert to grayscale of a single byte
        for (row = 0; row &lt; rows; row++)
            for (col = 0; col &lt; cols; col++)
            {
                Color pixel = bitmap0.GetPixel(col, row);
                byte grayish = (byte)Math.Floor((decimal)(pixel.R + pixel.G + pixel.B) / 3);
                bitmap.SetPixel(col, row, Color.FromArgb(grayish, grayish, grayish));
            }

        //check our threshold to set black or white by checking each pixels in a square defined by distance.
        for (row = 0; row &lt; rows; row++)
            for (col = 0; col &lt; cols; col++)
            {
                count = 0;
                //could optimize here heavily maybe, by only checking inside a circle rather than square+dist.
                for (x = Math.Max(col - dhalf, 0); x &lt; Math.Min(col + dhalf, cols); x++)
                    for (y = Math.Max(row - dhalf, 0); y &lt; Math.Min(row + dhalf, rows); y++)
                    {
                      //if inside the square and pixel color is not black count one more not black pixel
                        if ((Sqdistance &gt; DISTANCE(col, row, x, y)) &amp;&amp; ((bitmap.GetPixel(x, y).R) &gt; 0)) //this second condition is killing a lot the speed to begin.
                            count++;
                    }

                //if too much count of not black pixel, set it white.
                if (count &gt;= threshold)
                    outmap.SetPixel(col, row, Color.FromArgb(255, 255, 255));
                else
                    outmap.SetPixel(col, row, Color.FromArgb(0, 0, 0));

            }

        return outmap;
    }


SQ() and DISTANCE():

private int SQ(int a) { return ((a) * (a)); }
private int DISTANCE(int a, int b, int c, int d) { return (SQ(a - c) + SQ(b - d)); }
The ´var´ keyword:

From the C# Programming Guide:


  The var keyword can also be useful when the specific type of the variable is tedious to type on the keyboard, or is obvious, or does not add to the readability of the code.


So lines like:

int threshold = 7;
Bitmap bitmap0 = (Bitmap)imag.Clone();


would become:

var threshold = 7;
var bitmap0 = (Bitmap)imag.Clone();


Variable names:

Names like bitmap, bitmap0, Sqdistance don't have a useful meaning, not for you, nor for others reading/reviewing your code. Use meaningful names for your variables, better for readability and maintainability.

Curly braces:

Although you're not writing "wrong" code by omitting the braces on the outer loops, it seriously decreases readability. Following is only 2 lines longer and is much easier to understand:

for (row = 0; row &lt; rows; row++)
{
    for (col = 0; col &lt; cols; col++)
    {
        //Code...
    }
}


Other:

if (count &gt;= threshold)
    outmap.SetPixel(col, row, Color.FromArgb(255, 255, 255));
else
    outmap.SetPixel(col, row, Color.FromArgb(0, 0, 0));


can be rewritten to:

var colorBit = count &gt;= treshold ? 255 : 0;
outmap.SetPixel(col, row, Color.FromArgb(colorBit, colorBit, colorBit));


Following code contains too much braces in my opinion:

private int SQ(int a) { return ((a) * (a)); }


Rewrite it as follows:

private int Square(int a)
{ 
    return a * a;
}


or a different approach:

private int Square(int a)
{ 
    return (int)Math.Pow(a, 2);
}


Also for those two last methods, don't make them upper case. Method names in .NET are Pascal case (Capitalization Conventions).

Performance:

Regarding the performance. I didn't take a deep dive in the code or test it, but is there a reason why you perform the loops twice? Isn't there a possibility to execute all the code inse the double for-loop?

I did a performance analysis on your method and it is indeed the GetPixel method that is slowing things done. I did a Google search and apparently a lot of people are dealing with this. Here's an interesting question on StackOverflow that has a possible solution: C# - Faster Alternatives to SetPixel and GetPixel for Bitmaps for Windows Forms App.

Seems that an important part of this approach is the LockBits() method call. More reading about it on MSDN: Bitmap.LockBits Method (Rectangle, ImageLockMode, PixelFormat)