1

I have the below working code to create a Bitmap from the source pixels array. The source array is of type byte[ ] and it's the output of a library function and it is a flatten array 3D => 1D.

My code to create the Bitmap:

var bmp = new Bitmap(width, height, PixelFormat.Format24bppRgb);
for (int j = 0; j < height; j++) {
    for (int i = 0; i < width; i++) {
        // unflattening 1D array using 3D coordinates
        //      index = z *   yMax *  xMax + y *  xMax + x
        var rv = pixels[0 * height * width + j * width + i];
        var gv = pixels[1 * height * width + j * width + i];
        var bv = pixels[2 * height * width + j * width + i];
        bmp.SetPixel(i, j, Color.FromArgb(rv, gv, bv));
    }
}

The above code works, but it is slow. For a 2048 x 1364 bitmap it takes about 2 seconds to create. I checked stackoverflow for similar cases, there are solutions using BitmapData and Marshal.Copy but those are the cases when source array is not a 3D=>1D flattened array. Of course I tried to use BitmapData and copy from pixels to BitmapData.Scan0 but got a wrong bitmap. Is there any trick, how to speedup the bitmap creation?

Update: Based on comments below I ended up with this code, which works much faster:

var bytes = new byte[pixels.Length];
var idx = 0L;
var area = height * width;
var area2 = 2 * area;
var bmp = new Bitmap(width, height, PixelFormat.Format24bppRgb);
var bmd = bmp.LockBits(new Rectangle(0, 0, bmp.Width, bmp.Height), ImageLockMode.ReadWrite, bmp.PixelFormat);
for (int j = 0; j < height; j++) {
    for (int i = 0; i < width; i++) {
        var Unflatten1D = j * width + i;
        bytes[idx]      = pixels[Unflatten1D];
        bytes[idx + 1]  = pixels[Unflatten1D + area];
        bytes[idx + 2]  = pixels[Unflatten1D + area2];
        idx += 3;
    }
}
Marshal.Copy(bytes, 0, bmd.Scan0, bytes.Length);
bmp.UnlockBits(bmd);
Ladislav
  • 320
  • 3
  • 10
  • 1
    What if you "unflatten" the array first, but not call the `SetPixel()` for each pixel - store it in a temp array instead; then use `BitmapData` / `Marshal.Copy` approach? Basically, I think it's the `SetPixel()` call that's slow, not the calculations. The temp array will be in the correct format, so you'll get correct bitmap in the end. – CoolBots Aug 15 '20 at 19:41
  • 3
    `Of course I tried to use BitmapData and copy from pixels to BitmapData.Scan0 but got a wrong bitmap.` You should ask a question about how to fix that instead. LockBits is the correct way to do it. Show us your code of how you tried to use it and we can help you fix that. – Scott Chamberlain Aug 15 '20 at 19:59
  • Either use unsafe code (below) or LockBits – TaW Aug 15 '20 at 20:17
  • 1
    Thank you @CoolBots, your idea worked perfectly ! The SetPixel( ) method was slow in the code - obviously. Goind to update my post with a working and fast solution. – Ladislav Aug 16 '20 at 09:26
  • I posted a complete solution for dumping a byte array into an image some years ago, on a related question: https://stackoverflow.com/a/43967594/395685 The flattening thing can of course simply be done in advance, in a byte array. – Nyerguds Aug 25 '20 at 14:55

1 Answers1

2

You can compile you application with /unsafe flag (in visual studio right click project, then properties then allow unsafe code)

Next make the method code is unsafe. For example:

public unsafe void CreateImage()

Then change the code as follows.

var bmp = new Bitmap(width, height, PixelFormat.Format24bppRgb);
var bitmapData = bmp.LockBits(new Rectangle(0, 0, width, height), ImageLockMode.ReadWrite, PixelFormat.Format24bppRgb);


byte* bits = (byte*)bitmapData.Scan0;
fixed (byte* p = pixels)
{
    // Avoid having those 2 multiplications in the loop
    int wtimesh1 = height * width * 1;
    int wtimesh2 = height * width * 2;
    for (int j = 0; j < height; j++)
    {
        // Avoid multiplication in the loop
        int jtimesw = j * width;
        for (int i = 0; i < width; i++)
        {
            // unflattening 1D array using 3D coordinates
            //      index = z *   yMax *  xMax + y *  xMax + x
            int pixel = j * bitmapData.Stride + i * 3;
            // Avoid recalculation
            int jtimeswplusi = jtimesw + i;
            bits[pixel + 2] = p[jtimeswplusi];
            bits[pixel + 1] = p[wtimesh1 + jtimeswplusi];
            bits[pixel] = p[wtimesh2 + jtimeswplusi];
        }
    }

}

On my PC, this optimization caused creation time to go down from 2.1s to 0.05s

Of course the caveat here is that the code is unsafe (as the compiler flag implies), therefore extra care must be taken for the correctness of the code to avoid crashes, undefined behavior and/or security issues.

Sherif Elmetainy
  • 4,034
  • 1
  • 13
  • 22
  • 1
    I would avoid unsafe contexts, whenever possible, anyway now I have the solution, which is fast and safe. – Ladislav Aug 17 '20 at 11:36
  • 1
    Great! But also note that Marshal.Copy is (by definition) also not safe since unmanaged memory pointers do not contain size information, and therefore cannot be validated. But of course your solution is "safer" since only one line of code need to be checked for bugs versus multiple lines in my solution. But the Marshal.Copy solution is a little bit slower, since you write the bytes to an array (with boundary checks), then copy them all again to Scan0 where in my solution I write directly to Scan0 without boundary checks. So it's a very small tradeoff between safety and performance. – Sherif Elmetainy Aug 17 '20 at 11:53