Fog Creek Software
Discussion Board




More efficient method to brighten image?

Hi All,

I'm putting some basic image editing support into my app and I've come upon the need for a faster way to brighten an image.

I'm using .NET and have found that going through the bitmap object pixel-by-pixel and increasing the brightness on each of the r, g, & b channels (code pasted below) is very slow. Even when doing this on a thumbnail image it is very slow.

Does anyone have any reccomendation on a better way of doing this? Everything else that I am doing works fine using simple .NET methods so I don't really want to spend the time or money to implement a component like LEADTools or something just for this one function if I don't have to.

Oh yeah, this is very slow on my dev machine (XP PRO, P4 2.53GHZ, 1gb RAM, 64mb video, etc.), and terrible on slower machines. So it's not just the computer.

Thanks,

  --Josh

        Dim x, y, tmpColor
        Dim addR, addB, addG As Integer

        For y = 0 To bmp.Height - 1
            For x = 0 To bmp.Width - 1
                tmpColor = bmp.GetPixel(x, y)

                ' Work out how much can be added for each color
                If (tmpColor.r + amount) > 255 Then
                    addR = 255
                ElseIf (tmpColor.r + amount) < 0 Then
                    addR = 0
                Else
                    addR = tmpColor.r + amount
                End If

                If (tmpColor.g + amount) > 255 Then
                    addG = 255
                ElseIf (tmpColor.g + amount) < 0 Then
                    addG = 0
                Else
                    addG = tmpColor.g + amount
                End If

                If (tmpColor.b + amount) > 255 Then
                    addB = 255
                ElseIf (tmpColor.b + amount) < 0 Then
                    addB = 0
                Else
                    addB = tmpColor.b + amount
                End If

                bmp.SetPixel(x, y, Color.FromArgb(addR, addG, addB))
            Next
        Next

JWA
Thursday, June 12, 2003

I posted about this quite a while ago -- the one thing people seemed to agree with is that image manipulation is the one place where C/C++ (and pointers) really shine. Maybe you could write some unmanaged code to get it done faster?

jedidjab
Thursday, June 12, 2003

I do my image processing in C

I suspect that the GetPixel/SetPixels take the time as they need to calculate the offset into the image for each pixel and are called repeatedly.  Of course you don't know until you measure. 

I don't know enough about .net, to know if you can do this, but if you can treat the bitmap as an array (like an array of bytes) and walk thru in order since you are walking thru all the pixels.

One obvious optimization with your code is to reduce the number of adds (does tmpColor.r have enough range to go over/under 255/0 ? Again don't know enough about .net, but if does,

If (tmpColor.r + amount) > 255 Then
                    addR = 255
                ElseIf (tmpColor.r + amount) < 0 Then
                    addR = 0
                Else
                    addR = tmpColor.r + amount
                End If

becomes

tmpColor.r = min( 255, max( 0, tmpColor.r + amount ) ) ;

S. Tanna
Thursday, June 12, 2003

Hi Guys,

Yeah, I simply overlooked reducing the addition load. Changing it to:

                addR = tmpColor.r + amount
                If addR > 255 Then
                    addR = 255
                ElseIf addR < 0 Then
                    addR = 0
                End If

speeds things up. Now at least the thumbnails process more smoothly.

I'd really prefer to keep this in managed code and not have to dig into C or C++, so I just want to streamline it as much as possible. I don't think that .NET would let you treat it as an array of bytes, but I'll look into that.

JWA
Thursday, June 12, 2003

Try this:

From Max Raskin  at  http://www.planet-source-code.com/vb/scripts/ShowCode.asp?txtCodeId=165&lngWId=10

  Public Shared Function BrightnessFilter(ByVal BrightnessValue As Integer, ByRef b As Bitmap) As Bitmap
        '===================================================================================='
        ' This function is quite simple, it adds/subtracts the BrightnessValue to each byte  '
        ' and so it increases/decreases the brightness of the pixels.                        '
        '===================================================================================='
        If b.PixelFormat = PixelFormat.Format8bppIndexed Then
            MsgBox("256 colors bitmap are not supported.", MsgBoxStyle.Critical Or MsgBoxStyle.ApplicationModal, "Error")
            Return Nothing
        End If
        If BrightnessValue = 0 Then Return Nothing
        bmData = b.LockBits(New Rectangle(0, 0, b.Width, b.Height), ImageLockMode.ReadWrite, PixelFormat.Format24bppRgb)
        ptr = bmData.Scan0
        nOffset = bmData.Stride - b.Width * 3
        For y = 0 To b.Height - 1
            For x = 0 To (b.Width * 3) - 1
                Dim bByte As Integer = Marshal.ReadByte(ptr, 0)
                bByte += BrightnessValue
                ' Make sure that the byte doesn't go beyond 255 or beneath 0
                If bByte > 255 Then bByte = 255
                If bByte < 0 Then bByte = 0
                ' Write the new byte and loop until all bytes are modified
                Marshal.WriteByte(ptr, 0, CByte(bByte))
                ' In C++ we would simply do ptr += 3; but here it VB.NET
                ' it gets ugly, but it works (op_Explicit converts from Int32 to an IntPtr
                ' pointer).
                ' We increase the pointer by 3 (3 bytes for BGR):
                ptr = IntPtr.op_Explicit(ptr.ToInt32 + 1)
            Next
            ' Advance an offset
            ptr = IntPtr.op_Explicit(ptr.ToInt32 + nOffset)
        Next
        b.UnlockBits(bmData)
        Return b
    End Function

DJ
Thursday, June 12, 2003

JWA: not that it will speed up your code - but those If...Else's are getting on my nerves ;-) how about using Math.Min() and Math.Max()?

Duncan Smart
Thursday, June 12, 2003

DJ,

That works beautifully. Lot's of good things to learn from that. Fast and smooth, thanks.

Duncan,

I tried the Math.Min and Math.Max originally, and once again after S.Tanna reccomended it, but it was a lot slower, almost twice as slow. I went to the If..ElseIf to at least try to minimize the total number of comparisons needed.

I would have thought that the math comparison functions would have been better too, and I'm not really sure why they were so much slower.

--Josh

JWA
Thursday, June 12, 2003

The comparison functions are slow because they require a function call for each compare. The if statements don't, so they're faster.

Chris Tavares
Thursday, June 12, 2003

Like I say, I use C, don't know about .net, but I guessed 3X adding is bad.

What about eliminating ALL the adding from the loop.

Precalculate an array of the added values, then use that in the per pixel loop.  In pseudo-C :-

int add[256] ;
for ( int i = 0 ; i <= 255; i++ )
{
add[i] = min( 255, max( i + amount, 0 ) ) ; // or the IF version
}

for ( int x = 0 ; x < width ; x++ )
{
for ( int y = 0 ; y < height ; y++ )
{
rgb = GetPixel(x,y) ;
SetPixel( x, y, RGB( add[r], add[g], add[b] ) ) ;
}
}

S. Tanna
Thursday, June 12, 2003

Even in C, replacing an addition with a table lookup is not a particularly good idea. (For the C-crowd - remember that table[index]==*(table+index) so you're still adding)

Addition is pretty much the cheapest operation that can be had.

The link that DJ posted is right on the money - convert the whole bitmap into an array of bytes, operate on that and gain a LOT of speed. Calling GetPixel/SetPixel is rather expensive, especially if you do it for every single pixel in the picture.

Two more minor improvements:

Pointer management: Don't add to ptr in the inner loop. Instead, use 'x' as the offset for ReadByte/WriteByte. After all, you're passing in an offset anyways, and you need x to loop-count. Only increment ptr by stride once you go to the next scanline.

Alternatively, you can try incrementing the ptr and only using ReadByte(ptr) (and WriteByte, of course) without specifying the offset. One of the two approaches is going to net some speedup but I'm afraid you have to measure.

Common Subexpressions: If you really need speed, don't trust your compiler to eliminate common subexpressions.  You potentially compute width*3 for every scanline - do it once, outside the loop. The compiler /should/ do that for you, but you should either check on the compiler output or do it yourself if you need extra speed.

Or, you might resort to using Color matrices which are available in GDI+ - you can fairly easily scale or translate your whole color space.  Look for ColorMatrix for more info on that.  Here's a quick (untested) code fragment that should do the trick. It's a bit C-ish, sorry - my VB is more than rusty
Public Shared Function BrightnessFilter(ByVal BrightnessValue As Integer, ByRef b As Bitmap) As Bitmap

    Dim newBitmap = New Bitmap(b)
    Dim g = Graphics.FromImage(newBitmap)
    Dim imageAttributes As New Imaging.ImageAttributes()
    Dim width As Integer = b.Width
    Dim height As Integer = b.Height

    Dim colorMatrixElements As Single()() = { _
      New Single() {1, 0, 0, 0, 0}, _
      New Single() {0, 1, 0, 0, 0}, _
      New Single() {0, 0, 1, 0, 0}, _
      New Single() {0, 0, 0, 1, 0}, _
      New Single() {BrightnessValue / 255.0, BrightnessValue / 255.0, BrightnessValue / 255.0, 0, 1}}

    Dim colorMatrix As New Imaging.ColorMatrix(colorMatrixElements)

    imageAttributes.SetColorMatrix( _
      colorMatrix, _
      Imaging.ColorMatrixFlag.Default, _
      Imaging.ColorAdjustType.Bitmap)


    ' overdraw the bitmap with brightened colors
    g.DrawImage(newBitmap, _
      New Rectangle(0, 0, newBitmap.Width, newBitmap.Height), _
      0, 0, newBitmap.Width, newBitmap.Height, _
      GraphicsUnit.Pixel, imageAttributes)
    Return newBitmap
End Function


Hope this helps,
- Robert

Groby
Thursday, June 12, 2003

> Even in C, replacing an addition with a table lookup is not a particularly good idea. (For the C-crowd - remember that table[index]==*(table+index) so you're still adding)

You are eliminating the 2 IFs for <0 >255 and possibly 1 assignment all X3 per pixel.

It also has the advantage the same update loop can be used for other modifications such as contrast enhancement

But the GetPixel/SetPixel is what kills as they probably expand into something like
offset = ( y * 3 * height ) + ( x * 3 )

S. Tanna
Thursday, June 12, 2003

Don't think it'll help much, but it would probably be a good idea to cache the height and width before your For loops. As it is right now, your bmp object has to determine its width and height on each iteration. The difference there really depends on how the object determines those values internally, though.

Good luck.

Chris
Friday, June 13, 2003

Hi Chris,

That's interesting. Do most languages calculate the terms of a for statement on each loop? I guess that I'd always assumed that it would be set on the first pass and then remain static. It makes more sense to calculate on each loop to account for a changinf target, but I just always assumed it was a one shot thing.

How is this typically done in most languages?

  --Josh

P.S. Thanks to everyone for their insightful and extremely helpful comments to this question. I learned quite a bit of technique just going through all of this.

JWA
Friday, June 13, 2003

> But the GetPixel/SetPixel is what kills as they probably
> expand into something like
> offset = ( y * 3 * height ) + ( x * 3 )

They expand into a call into the O/S and video driver: maybe thousands, not dozens, of opcodes.

Christopher Wells
Friday, June 13, 2003

I don't know know if we're talking C or dot not, and a memory bitmap or one on screen.  But even in the best case, it's a nasty calculation to do in a loop

Oddly enough I had the exact same problem in one of my programs.

S. Tanna
Friday, June 13, 2003

*  Recent Topics

*  Fog Creek Home