views:

1108

answers:

4

I'm using the System.Drawing classes to generate thumbnails and watermarked images from user-uploaded photos. The users are also able to crop the images using jCrop after uploading the original. I've taken over this code from someone else, and am looking to simplify and optimize it (it's being used on a high-traffic website).

The previous guy had static methods that received a bitmap as a parameter and returned one as well, internally allocating and disposing a Graphics object. My understanding is that a Bitmap instance contains the entire image in memory, while Graphics is basically a queue of draw operations, and that it is idempotent.

The process currently works as follows:

  • Receive the image and store it in a temporary file.
  • Receive crop coordinates.
  • Load the original bitmap into memory.
  • Create a new bitmap from the original, applying the cropping.
  • Do some crazy-ass brightness adjusting on the new bitmap, maybe (?) returning a new bitmap (I'd rather not touch this; pointer arithmetics abound!), lets call this A.
  • Create another bitmap from the resulting one, applying the watermark (lets call this B1)
  • Create a 175x175 thumbnail bitmap from A.
  • Create a 45x45 thumbnail bitmap from A.

This seems like a lot of memory allocations; my question is this: is it a good idea to rewrite portions of the code and reuse the Graphics instances, in effect creating a pipeline? In effect, I only need 1 image in memory (the original upload), while the rest can be written directly to disk. All the generated images will need the crop and brightness transformations, and a single transformation that is unique to that version, effectively creating a tree of operations.

Any thought or ideas?

Oh, and I should probably mention that this is the first time I'm really working with .NET, so if something I say seems confused, please bear with me and give me some hints.

+2  A: 

The process seems reasonable. Each image has to exist in memory before it is saved to disk - so each version of your thumbnails will be in memory first. The key to making sure this works efficiently is to Dispose your Graphics and Bitmap objects. The easiest way to do that is with the using statement.

using( Bitmap b = new Bitmap( 175, 175 ) )
using( Graphics g = Graphics.FromBitmap( b ) )
{
   ...
}
Paul Alexander
My real question goes more like: will it give me an increase in performance if I reused the `Graphics` object?
Daniel Schierbeck
A: 

I am only going to throw this out there casually, but if you wanted a quick 'guide' to best practices for working with images, look at the Paint.NET project. For free high-proformance tools for doing image manipulation, look at AForge.NET.

The benefit of AForge is to allow you to do alot of these steps without creating a new bitmap every time. If this is for a website, I can almost guarentee that the code you are working with will be the performance bottleneck for the application.

JasonRShaver
+1  A: 

I completed a similar project a while ago and did some practical testing to see if there was a difference in performance if I reused the Graphics object rather than spin up a new one for every image. In my case, I was working on a steady stream of large numbers of images (>10,000 in a "batch"). I found that I did get a slight performance increase by reusing the Graphics object.

I also found I got a slight increase by using GraphicsContainers in the Graphics object to easily swap different states into/out of the object as it was used to perform various actions. (Specifically, I had to apply a crop and draw some text and a box (rectangle) on each image.) I don't know if this makes sense for what you need to do. You might want to look at the BeginContainer and EndContainer methods in the Graphics object.

In my case, the difference was slight. I don't know if you would get more or less improvement in your implementation. But since you will incur a cost in rewriting your code, you might want to consider finishing the current design and doing some perf tests before rewriting. Just a thought.

Some links you might find useful:

Using Nested Graphics Containers
GraphicsContainer Class

TMarshall
+2  A: 

Reusing Graphics objects will probably not result in significant performance gain.

The underlying GDI code simple creates a device context for the bitmap you have loaded in RAM (a Memory DC).

The bottleneck of your operation appears to be in loading the image from disk.

Why reload the image from disk? If it is already in a byte array in RAM, which it should be when it is uploaded - you can just create a memory stream on the byte array and then create a bitmap from that memory stream.

In other words, save it to the disk, but don't reload it, just operate on it from RAM.

Also, you shouldn't need to create a new bitmap to apply the watermark (depending on how it'd done.)

You should profile the operation to see where it needs improvement (or even if it needs to be improved.)