- Looking at what your code actually does, it's resizing an image, not cropping it - so you should rename this method to
Resize
, not Crop
.
- Only the "owner" of an
IDisposable
object should call .Dispose()
or use a using
block.
- By "owner" I mean a parent scope or lifetime that controls the lifetime of an
IDisposable
object.
- This can be a class (if the
IDisposable
is stored as a field) or it an be a method (if the IDisposable
should not outlive a method, or scope within the method).
- Because I assume you don't intend for your
Crop
method to limit the lifetime of Bitmap img_crop
(the source input image) your Crop
method must not call bmp_crop.Dispose()
!
- When a method creates a new
IDisposable
object and returns it, then the method "owns" the object until it returns it - which means if an exception is thrown inside the method then the method must dispose of the object - this is done with a try/catch
immediately after creating the object.
- It's very important to dispose of the
Graphics
object immediately after you no-longer need it. The disposable types in System.Drawing
are wrappers around native Windows GDI objects which use-up GDI Handles in your process - leaking GDI handles is bad because eventually you'll run-out and then all graphics operations will suddenly stop working (this is why a lot of badly written WinForms programs start having graphical glitches after a while).
- Also,
img_crop
and bmp_crop
are a poor choice for variable names because the name doesn't describe what the variable is, and Hungarian Notation is bad.
- I'm using
source
for the input image and cropped
for the output image in my example below.
Here's how I'd do it:
public static Bitmap Resize( Bitmap source, int w, int h, int x, int y )
{
Bitmap resized = new Bitmap( w, h, PixelFormat.Format24bppRgb );
try
{
cropped.SetResolution(80, 60);
using( Graphics g = Graphics.FromImage( cropped ) )
{
g.SmoothingMode = SmoothingMode.AntiAlias;
g.InterpolationMode = InterpolationMode.HighQualityBicubic;
g.PixelOffsetMode = PixelOffsetMode.HighQuality;
g.DrawImage( source, new Rectangle( 0, 0, w, h ), x, y, w, h, GraphicsUnit.Pixel );
}
// Pass the image to the other class/method/etc:
Class2( resized );
return resized; // Return the image to the caller, who now "owns" the image and its their responsibility to dispose of it.
}
catch( Exception ex )
{
resized.Dispose();
throw;
}
}
If you don't intend to return cropped
to the caller after calling your Class2
method then you should remove the try/catch
in the method and instead use a using
block, like so:
public static Bitmap Resize(Bitmap source, int w, int h, int x, int y)
{
using( Bitmap resized = new Bitmap( w, h, PixelFormat.Format24bppRgb ) )
{
cropped.SetResolution(80, 60);
using( Graphics g = Graphics.FromImage( resized ) )
{
g.SmoothingMode = SmoothingMode.AntiAlias;
g.InterpolationMode = InterpolationMode.HighQualityBicubic;
g.PixelOffsetMode = PixelOffsetMode.HighQuality;
g.DrawImage( source, new Rectangle( 0, 0, w, h ), x, y, w, h, GraphicsUnit.Pixel );
}
// Pass the image to the other class/method/etc:
Class2( resized );
}
}