14

I have a small paint program that I am working on. I am using SetPixel on a bitmap to do that drawing of lines. When the brush size gets large, like 25 pixels across there is a noticeable performance drop. I am wondering if there is a faster way to draw to a bitmap. Here is a bit of the background of the project:

  • I am using bitmaps so that I can utilise layers, like in Photoshop or The GIMP.
  • Lines are being drawn manually because this will eventually use graphics tablet pressure to alter the size of the line over its length.
  • The lines should eventually be anti-aliaced/smoothed along the edges.

I'll include my drawing code just in case it is this that is slow and not the Set-Pixel bit.

This is in the windows where the painting happens:

    private void canvas_MouseMove(object sender, MouseEventArgs e)
    {
        m_lastPosition = m_currentPosition;
        m_currentPosition = e.Location;

        if(m_penDown && m_pointInWindow)
            m_currentTool.MouseMove(m_lastPosition, m_currentPosition, m_layer);
        canvas.Invalidate();
    }

Implementation of MouseMove:

    public override void MouseMove(Point lastPos, Point currentPos, Layer currentLayer)
    {
        DrawLine(lastPos, currentPos, currentLayer);
    }

Implementation of DrawLine:

    // The primary drawing code for most tools. A line is drawn from the last position to the current position
    public override void DrawLine(Point lastPos, Point currentPos, Layer currentLayer)
    {
        // Creat a line vector
        Vector2D vector = new Vector2D(currentPos.X - lastPos.X, currentPos.Y - lastPos.Y);

        // Create the point to draw at
        PointF drawPoint = new Point(lastPos.X, lastPos.Y);

        // Get the amount to step each time
        PointF step = vector.GetNormalisedVector();

        // Find the length of the line
        double length = vector.GetMagnitude();

        // For each step along the line...
        for (int i = 0; i < length; i++)
        {
            // Draw a pixel
            PaintPoint(currentLayer, new Point((int)drawPoint.X, (int)drawPoint.Y));
            drawPoint.X += step.X;
            drawPoint.Y += step.Y;
        }
    }

Implementation of PaintPoint:

    public override void PaintPoint(Layer layer, Point position)
    {
        // Rasterise the pencil tool

        // Assume it is square

        // Check the pixel to be set is witin the bounds of the layer

            // Set the tool size rect to the locate on of the point to be painted
        m_toolArea.Location = position;

            // Get the area to be painted
        Rectangle areaToPaint = new Rectangle();
        areaToPaint = Rectangle.Intersect(layer.GetRectangle(), m_toolArea);

            // Check this is not a null area
        if (!areaToPaint.IsEmpty)
        {
            // Go through the draw area and set the pixels as they should be
            for (int y = areaToPaint.Top; y < areaToPaint.Bottom; y++)
            {
                for (int x = areaToPaint.Left; x < areaToPaint.Right; x++)
                {
                    layer.GetBitmap().SetPixel(x, y, m_colour);
                }
            }
        }
    }

Thanks a lot for any help you can provide.

Pyro
  • 339
  • 1
  • 4
  • 12

5 Answers5

14

You can lock the bitmap data and use pointers to manually set the values. It's much faster. Though you'll have to use unsafe code.

public override void PaintPoint(Layer layer, Point position)
    {
        // Rasterise the pencil tool

        // Assume it is square

        // Check the pixel to be set is witin the bounds of the layer

        // Set the tool size rect to the locate on of the point to be painted
        m_toolArea.Location = position;

        // Get the area to be painted
        Rectangle areaToPaint = new Rectangle();
        areaToPaint = Rectangle.Intersect(layer.GetRectangle(), m_toolArea);

        Bitmap bmp;
        BitmapData data = bmp.LockBits(new Rectangle(0, 0, bmp.Width, bmp.Height), ImageLockMode.ReadWrite, PixelFormat.Format24bppRgb);
        int stride = data.Stride;
        unsafe
        {
            byte* ptr = (byte*)data.Scan0;
            // Check this is not a null area
            if (!areaToPaint.IsEmpty)
            {
                // Go through the draw area and set the pixels as they should be
                for (int y = areaToPaint.Top; y < areaToPaint.Bottom; y++)
                {
                    for (int x = areaToPaint.Left; x < areaToPaint.Right; x++)
                    {
                        // layer.GetBitmap().SetPixel(x, y, m_colour);
                        ptr[(x * 3) + y * stride] = m_colour.B;
                        ptr[(x * 3) + y * stride + 1] = m_colour.G;
                        ptr[(x * 3) + y * stride + 2] = m_colour.R;
                    }
                }
            }
        }
        bmp.UnlockBits(data);
    }
Jack
  • 5,680
  • 10
  • 49
  • 74
  • Thanks for this. I think I see what is going on; ptr is a lump of the bitmap data containing the red, green and blue pixel values and is setting them manually. I will give this a go today and see how it works out. Thanks a lot for this, I feel my understanding has advanced another notch :P Just one thing though, I thought a bitmap in C# has an alpha channel too. I thought this would be handled in the pixel colour info along with the RGB. – Pyro Oct 17 '11 at 08:45
  • 2
    You need to specify the http://msdn.microsoft.com/en-us/library/system.drawing.imaging.pixelformat.aspx to include an alpha component. Just change `PixelFormat.Format24bppRgb` to `PixelFormat.Format32bppArgb`. I believe it should be after R so just offset it by +3. If you do use 32 bits per pixel, you will need to change `x * 3` to `x * 4` since it will be 4 bytes per pixel now. – Jack Oct 17 '11 at 10:12
  • I have just tried this out. It is lovely and quick now on all brush sizes! However there is now some odd behaviour: The actual drawing happens at an offset to where the mouse is at. I wonder if it anything to do with this line here: BitmapData data = bmp.LockBits(new Rectangle(areaToPaint.Right, areaToPaint.Bottom), ImageLockMode.ReadWrite, PixelFormat.Format24bppRgb); There needs to be a position specified for the rectangle. I used supplied 'position' value. Is that right? – Pyro Oct 17 '11 at 13:33
  • The `position` value for the rectangle is the position of the rectangle on your picturebox or panel. You're using the position of the mouse. If your image layout is not centered, you can just use (0, 0) for the position. If it is centered and your picturebox is bigger than your image, then you'll need to adjust the offset. Also you should change `new Rectangle(areaToPaint.Right, areaToPaint.Bottom)` to `new Rectangle(0, 0, bmp.Width, bmp.Height)`. I thought `areaToPaint` was the dimensions of the entire image. – Jack Oct 17 '11 at 20:35
  • 1
    Is it necessary to use unsafe code to do this? Couldn't you use `Marshal.WriteByte` to write to the memory address of the `IntPtr` instead of converting it to a `byte*` to do unsafe pointer operations? Or would that be slower? – meustrus Jun 27 '17 at 15:50
5

SetPixel does this: is locks the whole image, sets the pixel and unlocks it

try to do that: you acquire a lock for the whole memory image with lockbits, process you update and release the lock after.

lockbits

fixagon
  • 5,506
  • 22
  • 26
3

I usually use an array to represent the raw pixel data. And then copy between that array and the bitmap with unsafe code.

Making the array of Color is a bad idea, since the Color struct is relatively large(12 bytes+). So you can either define your own 4 byte struct(that's the one I chose) or simply use an array of int or byte.

You should also reuse your array, since GC on the LOH tends to be expensive.

My code can be found at:

https://github.com/CodesInChaos/ChaosUtil/blob/master/Chaos.Image/

An alternative is writing all your code using pointers into the bitmap directly. That's a bit faster still, but can make the code uglier and more error prone.

CodesInChaos
  • 106,488
  • 23
  • 218
  • 262
  • 1
    Do you use an array the size of the target bitmap or only the size of the area that is going to be modified? – Pyro Oct 14 '11 at 14:44
  • In my application I used an array with the same size as the bitmap, but I needed to work on the whole bitmap. But even if I'd work only on part of the bitmap I'd still use the full array, but possibly only fill in the part of it I need. That way I don't need to allocate a new array when working on a differently sized section of the image. – CodesInChaos Oct 14 '11 at 14:51
1

Just an idea: Fill an offscreen bitmap with your Brush pixels. You only need to regenerate this bitmap when the brush, size or color is changed. And then just draw this bitmap onto your existing bitmap, where the mouse is located. If you can modulate a bitmap with a color, you could set the pixels in grayscale and modulate it with the current brush color.

dowhilefor
  • 10,971
  • 3
  • 28
  • 45
  • That sounds like a pretty cool idea. Thanks. I suppose if I use transparency then it would work when drawing over existing lines. – Pyro Oct 14 '11 at 14:48
0

You're calling GetBitmap within your nested for loop. It looks like that's not necessary, you should GetBitmap outside the for loops as the reference isn't going to change.

Also look at @fantasticfix answer, Lockbits nearly always sorts slow performance issues with getting/setting pixels

RichK
  • 11,318
  • 6
  • 35
  • 49