用getPixel()和循环成像

Question

我编写了工作良好的代码，读取图像，然后按区域对其执行某种阈值。如果区域有太多的白色，那么它会把所有的区域变成白色，否则就会变成黑色。

问题是，代码最初来自一个带有指针的C++源代码，而不是GetPixel()等等。速度只是瞬间的，仅此而已。现在我重写了“安全”(没有指针)，每幅图像大约需要20秒，所以瞬间与20秒=疯狂的差别。

你们中有人能帮我改进这个小代码吗，即使你们使用了不安全的指针代码等等？我不太关心这个。我绝对需要的部分是速度，它必须尽可能快，因为我将处理庞大的图像库。不需要花30分钟，而是需要几天的时间。

    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 < rows; row++)
            for (col = 0; col < 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 < rows; row++)
            for (col = 0; col < 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 < Math.Min(col + dhalf, cols); x++)
                    for (y = Math.Max(row - dhalf, 0); y < Math.Min(row + dhalf, rows); y++)
                    {
                      //if inside the square and pixel color is not black count one more not black pixel
                        if ((Sqdistance > DISTANCE(col, row, x, y)) && ((bitmap.GetPixel(x, y).R) > 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 >= threshold)
                    outmap.SetPixel(col, row, Color.FromArgb(255, 255, 255));
                else
                    outmap.SetPixel(col, row, Color.FromArgb(0, 0, 0));

            }

        return outmap;
    }

SQ()和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)); }

Answer 1

‘var’关键字：

来自C#编程指南：

当变量的特定类型单调乏味地在键盘上键入，或者是显而易见的，或者没有增加代码的可读性时，var关键字也可能很有用。

所以像这样的台词：

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

将成为：

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

变量名：

像bitmap、bitmap0、Sqdistance这样的名字没有一个有用的含义，对你来说，对其他人来说，对你的代码也没有意义。为变量使用有意义的名称，更好地提高可读性和可维护性。

花括号：

虽然您没有通过省略外部循环上的大括号来编写“错误”代码，但它严重降低了可读性。以下内容只长了两行，而且更容易理解：

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

其他：

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

可重写为：

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

在我看来，以下代码包含太多的大括号：

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

将其改写如下：

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

或者另一种方法：

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

另外，对于最后两个方法，不要将它们设为大写。.NET中的方法名是Pascal (资本化公约 )。

Performance:

关于表演。我没有深入研究或测试代码，但是有什么原因可以让您执行两次循环呢？难道没有可能在双循环中执行所有代码吗？

我对您的方法进行了性能分析，实际上是GetPixel方法减慢了所做的事情。我做了谷歌搜索，显然很多人都在处理这个问题。这里有一个关于StackOverflow的有趣问题，它有一个可能的解决方案：C# - Windows应用程序的SetPixel和GetPixel的更快替代方案。

该方法的一个重要部分似乎是LockBits()方法调用。在MSDN：Bitmap.LockBits方法(矩形、ImageLockMode、PixelFormat)上阅读更多关于它的文章

Answer 2

byte grayish = (byte)Math.Floor((decimal)(pixel.R + pixel.G + pixel.B) / 3);

对于这一点，十进制类型不是一个很好的选择，因为它需要很高的性能，而且根本不需要任何好处。你应该使用它，只有当你有大的逗号，需要很大的准确性。这里你只处理自然数，一点逗号都没有。

byte grayish = (byte)(((int)pixel.R + pixel.G + pixel.B) / 3);

不需要Math.Floor，因为int和字节除法不保留剩余的rest。

Answer 3

阿巴斯提到了这个问题，但我认为他并没有真正关注这个问题

for (row = 0; row < rows; row++) for (col = 0; col < 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 < rows; row++) for (col = 0; col < 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 < Math.Min(col + dhalf, cols); x++) for (y = Math.Max(row - dhalf, 0); y < Math.Min(row + dhalf, rows); y++) { //if inside the square and pixel color is not black count one more not black pixel if ((Sqdistance > DISTANCE(col, row, x, y)) && ((bitmap.GetPixel(x, y).R) > 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 >= threshold) outmap.SetPixel(col, row, Color.FromArgb(255, 255, 255)); else outmap.SetPixel(col, row, Color.FromArgb(0, 0, 0)); }

始终使用Curly，如果您使用它们，人们不会注意到，程序员在不使用它们时会期待它们，特别是在for循环中。

for (row = 0; row < rows; row++)
{
    for (col = 0; col < 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 < rows; row++)
{
    for (col = 0; col < 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 < Math.Min(col + dhalf, cols); x++)
        {
            for (y = Math.Max(row - dhalf, 0); y < Math.Min(row + dhalf, rows); y++)
            {
              //if inside the square and pixel color is not black count one more not black pixel
                if ((Sqdistance > DISTANCE(col, row, x, y)) && ((bitmap.GetPixel(x, y).R) > 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 >= threshold)
        {
            outmap.SetPixel(col, row, Color.FromArgb(255, 255, 255));
        }
        else
        {
            outmap.SetPixel(col, row, Color.FromArgb(0, 0, 0));
        }
    }
}

Curly Braces和If/ and语句定义了代码块，这样程序员(以及随后的程序员)就不必三思他们要添加的代码范围(如果他们需要为某些新功能添加代码或修复逻辑错误)。

如果您总是使用Curly Braces，那么您完全知道您的作用域是什么，当嵌套循环在彼此内部时，这一点特别重要。

它受到大多数程序员(C#、Java等)的强烈推荐。总是使用卷发护套。

这个规则的大多数例外都是简单的if语句，这些语句在满足简单条件时执行一些简单的单行代码。

这里显示的if语句中的条件太长了，只需在末尾添加count++;就会降低可读性。

if ((Sqdistance > DISTANCE(col, row, x, y)) && ((bitmap.GetPixel(x, y).R) > 0)) //this second condition is killing a lot the speed to begin.
    count++;

我同意，如果是更简单的事情，如：

if ( x = y ) count++; 

这是if语句推荐形式的快捷方式，它是

if ([condition])
{
     //code
}

如MSDN所述

时间语句和其他语句都可以由一个或多个语句组成，这些语句被大括号({})括起来。对于单个语句，大括号是可选的，但建议使用。

同样的情况也适用于循环，有一个标准，也有通往标准的捷径。

MSDN国家

循环的主体由语句、空语句或语句块组成，这些语句是通过大括号中的零条或多条语句来创建的。