Determine if an image is opaque or transparent

Question

I wrote an extension method defined for the Image Class (System.Drawing) — which returns a Boolean value telling you if the image is opaque or transparent.

The color of each pixel in the image is represented as a 32-bit number: 8 bits each for alpha, red, green, and blue (ARGB). Each of the four components is a number from 0 through 255, with 0 representing no intensity and 255 representing full intensity.

The alpha component specifies the transparency of the color.

If any pixel in the image has a value not equal to 255 in its alpha channel, then the image is not opaque, and therefore transparent.

    public static bool IsOpaque(this Image image)
    {
        var bitmap = new Bitmap(image);
        var bitmapData = bitmap.LockBits(new Rectangle(0, 0, bitmap.Width, bitmap.Height), ImageLockMode.ReadOnly,
            PixelFormat.Format32bppArgb);
        unsafe
        {
            var p = (byte*)bitmapData.Scan0;
            for (var x = 0; x < bitmap.Width; x++)
            {
                for (var y = 0; y < bitmap.Height; y++)
                {
                    if (p[x * 4 + y * bitmapData.Stride + 3] == 255) continue;
                    bitmap.UnlockBits(bitmapData);
                    return false;
                }
            }
        }
        bitmap.UnlockBits(bitmapData);
        return true;
    }

Concerns:

• Can performance be improved at all?
• Is it possible to improve readability and reusability without affecting performance?

Answer 1

Performance can definitely be improved, but it's a case of how far you want to take the tradeoff.

First, though, note that Bitmap is IDisposable. If you create a new one, you're responsible for disposing it, typically with a using.

Bearing in mind Heslacher's point about casting being faster than copying, and correcting it to account for the fact that not all images are bitmaps, we could refactor:

    private static bool IsOpaque(Bitmap bitmap)
    {
        ...
    }

    public static bool IsOpaque(this Image image)
    {
        var bitmap = image as Bitmap;
        if (bitmap != null)
        {
            return IsOpaque(bitmap);
        }

        using (bitmap = new Bitmap(image))
        {
            return IsOpaque(bitmap);
        }
    }

---

Now, taking into account memory locality we want to swap the loops, as pointed out by Roland:

            for (var y = 0; y < bitmap.Height; y++)
            {
                for (var x = 0; x < bitmap.Width; x++)
                {
                    if (p[x * 4 + y * bitmapData.Stride + 3] == 255) continue;
                    ...
                }
            }

But multiplication is slow, so replace with addition:

            int off = 3; // NB apply Roland's observation about endianness here
            int gap = bitmapData.Stride - bitmap.Width * 4;
            for (var y = 0; y < bitmap.Height; y++, off += gap)
            {
                for (var x = 0; x < bitmap.Width; x++, off += 4)
                {
                    if (p[off] == 255) continue;
                    ...
                }
            }

---

Now we come to the big tradeoff of speed vs complexity of code. Image.PixelFormat and LockBits. If the images are already in 32-bit ARGB then LockBits is relatively cheap. If they're in a different format, it can be quite expensive.

• If the format doesn't support alpha (i.e. (image.PixelFormat & (PixelFormat.Indexed | PixelFormat.Alpha | PixelFormat.PAlpha)) == PixelFormat.Undefined) then you can return false without even looking at the data.
• If the format is indexed, you can do a quick-reject by scanning the palette. (Or even a quick-accept if every palette index is translucent, although I imagine that case would be rare).
• Failing those, you can replicate the scan on a per-format basis, locking the data in its native format and then scanning appropriately. For indexed images, you can further split on whether the palette contains only one index with transparency (so you just need to look for that index) vs whether you need to actually look up the alpha for each pixel.

Note that if you take the final option, it might be worth generating the code with T4 so that you only have to edit one place in the .tt file to fix bugs rather than having to fix them method by method.

Further note that although I said this was a tradeoff, strictly speaking your code is buggy: if the original image is in Format64bppArgb then the conversion to Format32bppArgb might turn an almost opaque pixel completely opaque.

Answer 2

I don't know how good the C# compiler and runtime optimize this code out of the box, so here is what I would try:

• Load bitmap.Width, bitmap.Height and bitmapData.Stride into local variables.
• Swap the two for loops, making x the inner loop. Memory accesses are now linear instead of jumping around (to see this, print the array index in your current code).

Side node: You should check whether your code works correctly on big-endian platforms. The + 3 looks suspicious. You could use a uint * instead of the byte * and test the pixel value like this: (p[x + stride * y] >= 0xFF000000.

Answer 3

Readability

The readability of the code could be slightly improved by adding a new line before the unsafe. Assigning the Rectangle which is used to call LockBits() to a local variable will make the call to LockBits() more readable as well because it doesn't sprawl over two lines.

---

Performance

• Why do you create a new Bitmap out of the passed Image if the only thing you do is using it readonly ? A simple cast to a Bitmap would be enough and faster as well.

  Just do:

  var bitmap = image as Bitmap ?? new Bitmap(image);
  

• The calculation of the index of the pointer p could be improved by adjusting the outer loop to simply increment x by 4, because addition is faster then multiplication.

  for (var x = 0; x < bitmap.Width; x += 4)
  {
      for (var y = 0; y < bitmap.Height; y++)
      {
          if (p[x + y * bitmapData.Stride + 3] == 255) continue;
          bitmap.UnlockBits(bitmapData);
          return false;
      }
  }  
  

  If we want to improve this further we should use a new variable currentY, like so

  for (var x = 0; x < bitmap.Width; x += 4)
  {
      var currentY = -bitmapData.Stride;
      for (var y = 0; y < bitmap.Height; y++)
      {
          currentY += bitmapData.Stride;
          if (p[x + currentY + 3] == 255) continue;
          bitmap.UnlockBits(bitmapData);
          return false;
      }
  }
  

  but this improvement is almost not measurable. The main performance boost comes from the simple casting instead of creating new Bitmap.

Answer 4

Always validate the arguments in a public method.

if (image == null)
{
    throw new ArgumentNullException(nameof(image));
}

As suggested by Heslacher check if the image is a Bitmap before converting it

Bitmap bitmap = image as Bitmap;

if (bitmap == null)
{
    bitmap = new Bitmap(image);
}

Use try-finally to reduce repetition and reduce the chance of a mistake

var bitmapData = bitmap.LockBits(
    new Rectangle(0, 0, width, height), 
    ImageLockMode.ReadOnly,
    PixelFormat.Format32bppArgb);

try
{
    ...
    return false;
}
finally
{
    bitmap.UnlockBits(bitmapData);
}
return true;

Put together with the suggestion by Roland Illig to scan the image with a uint*

public static bool IsOpaque(this Image image)
{
    if (image == null)
    {
        throw new ArgumentNullException(nameof(image));
    }

    Bitmap bitmap = image as Bitmap;

    if (bitmap == null)
    {
        bitmap = new Bitmap(image);
    }

    var width = bitmap.Width;
    var height = bitmap.Height;

    var bitmapData = bitmap.LockBits(
        new Rectangle(0, 0, width, height), 
        ImageLockMode.ReadOnly,
        PixelFormat.Format32bppArgb);

    try
    {
        unsafe
        {
            var pixels = (uint*)bitmapData.Scan0;
            var stride = bitmapData.Stride / sizeof(uint);

            for (var y = 0; y < height; y++)
            {
                var yOffset = y * stride;

                for (var x = 0; x < width; x++)
                {
                    if (pixels[x + yOffset] < 0xFF000000) 
                    {
                        return false;
                    }
                }
            }
        }
    }
    finally
    {
        bitmap.UnlockBits(bitmapData);
    }

    return true;
}

Answer 5

As far as performance, the best you can do is to get those nested loops as tight as possible. I think the tightest you could get them would be

var maxY = bitmap.Height * bitmapData.stride + 3;
var maxX = bitmap.Width * 4;
for (var y = 3; y < maxY; y += bitmapData.stride)
{
    for (var x = 0; x < maxX; x +=4)
    {
        if (p[y+x] == 255) continue;

        bitmap.UnlockBits(bitmapData);
        return false;
    }
}

This is somewhat like previous answers, but it does not use additional variables to keep track of gap and offset, and does absolutely no multiplications inside the loops. This limits the code to the bare minimum necessary, increment x and y to their next spot in the bit array and check that spot.

Answer 6

If performance is not the most important goal, the following code is simple and beautiful:

public static bool IsTranslucent(this Bitmap bitmap)
{
    var width = bitmap.Width;
    var height = bitmap.Height;

    for (var y = 0; y < height; y++)
        for (var x = 0; x < width; x++)
            if (bitmap.GetPixel(x, y).A != 255)
                return false;
    return true;
}

Unless you are doing massive amounts of image processing, this is the code I would go with. It is as simple as possible and expresses your requirements closely.

To be worth optimizing, this code should be responsible for at least 20% of the total run time of the program.