Applying a 'mean filter'

Question

I am just trying to apply a "mean filter" with C#. The working code below gets window size (3,5,7,9,...) from textBoxWinSize and applies the mean filter.

I am new and I want your comments and suggestions about my code so I can improve myself.

Can I make it faster? How? Is there any unnecessary or absurd part in the code? Can it be improved in any other way?

I do not care about border pixels for now.

private void buttonApplyFilter_Click(object sender, EventArgs e)
{
 int R, G, B,aveR,aveG,aveB;
 double sumR,sumG,sumB;
 Color pixel_value;
 Bitmap bmp = new Bitmap(bmpOrig);
 int WinSize = Int32.Parse(textBoxWinSize.Text);
 int col,row,n,m;
 for (col = (WinSize-1); col < (bmp.Width-WinSize); col++)
 for (row = (WinSize-1); row < (bmp.Height-WinSize); row++)
 {
 sumB = 0;
 sumG = 0;
 sumR = 0;
 for (m=-(WinSize-1)/2; m<=(WinSize-1)/2;m++)
 for (n=-(WinSize-1)/2;n<=(WinSize-1)/2;n++)
 {
 pixel_value = bmp.GetPixel(col+m, row+n);
 R = (pixel_value.R);
 G =(pixel_value.G);
 B = (pixel_value.B);
 sumR = sumR + R;
 sumG = sumG + G;
 sumB = sumB + B;
 }
 aveR = (int)(sumR / (WinSize * WinSize));
 aveG = (int)( sumG / (WinSize * WinSize)); 
 aveB = (int)( sumB / (WinSize * WinSize));
 bmp.SetPixel(col, row, Color.FromArgb(aveR, aveG, aveB));
 }
 pictureBox1.Image = bmp;
}

Answer 1

First up, I think you're doing too much in the button click handler. The logic for applying the mean filter should be moved to its own method, something like

private static Bitmap ApplyMeanFilter(Bitmap input, int windowSize)

Now it can be re-used and unit tested more easily.

---

I think this line is wrong

pixel_value = bmp.GetPixel(col+m, row+n)

in that bmp should be bmpOrig. The reasoning is that you're changing pixels in bmp as you go, which will have an effect on pixels that are processed later. That is, they won't be assigned the mean of their original surrounding pixels.

---

Assuming the bmp should be bmpOrig, we can optimise this quite a bit.

Say that instead of a bitmap, we have a 2D array of ints.

input

\begin{array}{c | c c c c} & 0 & 1 & 2 & 3 \\ \hline 0 & 1 & 2 & 3 & -1 \\ 1 & 4 & 1 & 6 & -3 \\ 2 & 4 & 5 & 9 & -3 \\ 3 & 4 & -2 & 6 & -3 \end{array}

We can compute another 2D array such that the \$(i, j)\$-th entry is the sum of entries in the submatrix \$(0, 0), (i, j)\$:

var sums = new int[height, width];
for (var i = 0; i < height; i++)
{
 var sum = 0;
 for (var j = 0; j < width; j++)
 {
 sum += input[i, j];
 sums[i, j] = i > 0 ? sums[i - 1, j] + sum : sum;
 }
}

sums

\begin{array}{c | c c c c} & 0 & 1 & 2 & 3 \\ \hline 0 & 1 & 3 & 6 & 5 \\ 1 & 5 & 8 & 17 & 13 \\ 2 & 9 & 17 & 35 & 28 \\ 3 & 13 & 19 & 43 & 33 \end{array}

Now suppose we want to find the sum of the values in the 3x3 square around entry \$(i, j) = (2, 2)\$:

\begin{array}{c | c c c c} & 0 & 1 & 2 & 3 \\ \hline 0 & 1 & 2 & 3 & -1 \\ 1 & 4 & 1 & 6 & -3 \\ 2 & 4 & 5 & \textbf{9} & -3 \\ 3 & 4 & -2 & 6 & -3 \end{array}

We can do this quickly using the matrix we just computed. The answer is given by

 sums[3, 3] - sums[3, 0] - sums[0, 3] + sums[0, 0]
= 33 -ひく 13 -ひく 5 +たす 1
=わ 16

In general, if halfWindow = (windowSize - 1) / 2 then the sum of values in the windowSize * windowSize square around entry \$(i, j)\$ is given by

sums[i + halfWindow, j + halfWindow] -
sums[i - halfWindow - 1, j + halfWindow] -
sums[i + halfWindow, j - halfWindow - 1] +
sums[i - halfWindow - 1, j - halfWindow - 1]

The beauty is, computing the sum of the surrounding windowSize * windowSize values this way is independent of windowSize.

---

Getting back to the original problem of bitmaps, let's suppose we have a struct that will let us sum up the RGB values of the pixels. I named it Clr because I'm not very imaginative today; I'm sure you can think of a better name. Let's also assume we have operators +, - and / defined in the obvious way.

Putting it all together would look something like this

var sums = new Clr[input.Height, input.Width];
for (var i = 0; i < input.Height; i++)
{
 var sum = new Clr();
 for (var j = 0; j < input.Width; j++)
 {
 sum += Clr.FromColor(input.GetPixel(j, i));
 sums[i, j] = i > 0 ? sums[i - 1, j] + sum : sum;
 }
}
var output = new Bitmap(input);
var windowArea = windowSize * windowSize;
var halfWindow = (windowSize - 1) / 2;
for (var i = windowSize - 1; i < input.Height - windowSize; i++)
{
 for (var j = windowSize - 1; j < input.Width - windowSize; j++)
 {
 var clr = sums[i + halfWindow, j + halfWindow] -
 sums[i - halfWindow - 1, j + halfWindow] -
 sums[i + halfWindow, j - halfWindow - 1] +
 sums[i - halfWindow - 1, j - halfWindow - 1];
 output.SetPixel(j, i, (clr / windowArea).ToColor());
 }
}
return output;

Running this on a 256x256 image, I got the following timings

windowSize this original
 3 00:00:00.0910750 00:00:00.4720078
 5 00:00:00.0899366 00:00:01.2674758
 7 00:00:00.0853111 00:00:02.2569419
 9 00:00:00.0938911 00:00:03.4408123
11 00:00:00.0840854 00:00:04.9614386

---

Other things to try that might improve performance:

• Use a Clr[][] instead of a Clr[,]. See Why are multi-dimensional arrays in .NET slower than normal arrays?
• Doing the above will let you move sums[i + halfWindow] and sums[i - halfWindow - 1] into variables outside the inner loop.
• Compute the first row of sums separately to remove the conditional in this line sums[i, j] = i > 0 ? sums[i - 1, j] + sum : sum;

• ffttyy: How can I create the struct Clr includes RGB?
• mjolka: @sigara I've uploaded an example here (you'll need LinqPad to run it, but it can be read with any text editor).

Answer 2

I'm not going to try optimizing this. Right now, this code is ugly for other reasons. First and foremost being the naming: In C#, local variables should be camelCase and should generally not include abbreviations. Abrvs dt mk tns esy t ndrstd. They do not make things more performant.

Next, this is C#. Defining all your variables at the top is not going to affect performance. It is generally a good idea to define variables at the scope they belong so as to avoid confusing people.

Next, m and n are very unusual loop variables. Generally, you start with i and j for those, because no one expects i to mean anything else.

I've attached the end result, which is much cleaner, easier to understand code.

It may also be beneficial to create two variables for windowSize, since you use it in more than one place using identical formulas. Those are a better idea than copying it everywhere it's needed.

Hopefully a later review will cover other concerns.

private void ApplyFilterClick(object sender, EventArgs e)
{
 var bitmap = new Bitmap(original);
 var windowSize = Int32.Parse(textBoxWinSize.Text);
 for (int column = (windowSize - 1); column < (bitmap.Width - windowSize); column++)
 {
 for (int = (windowSize - 1); row < (bitmap.Height - windowSize); row++)
 {
 double sumBlue = 0;
 double sumGreen = 0;
 double sumRed = 0;
 for (int i =- (windowSize - 1)/2; i <= (windowSize - 1)/2; i++)
 {
 for (int j =- (windowSize - 1)/2; j <= (windowSize - 1)/2; j++)
 {
 var pixelValue = bitmap.GetPixel(column + i, row + j);
 var red = pixelValue.R;
 var green = pixelValue.G;
 var blue = pixelValue.B;
 sumRed = sumRed + red;
 sumGreen = sumGreen + green;
 sumBlue = sumBlue + blue;
 }
 }
 var averageRed = (int)(sumRed / (windowSize * windowSize));
 var averageGreen = (int)(sumGreen / (windowSize * windowSize)); 
 var averageBlue = (int)(sumBlue / (windowSize * windowSize));
 bitmap.SetPixel(column, row, Color.FromArgb(averageRed, averageGreen, averageBlue));
 }
 }
 pictureBox1.Image = bitmap;
}