0

I have this class to crop an image:

public static Bitmap Crop(Bitmap img_crop, int w, int h, int x, int y)
        {
            Image img_crop = img_crop;
            Bitmap bmp_crop = new Bitmap(w, h, PixelFormat.Format24bppRgb);
            bmp_crop.SetResolution(80, 60);

            Graphics gfx = Graphics.FromImage(bmp_crop);
            gfx.SmoothingMode = SmoothingMode.AntiAlias;
            gfx.InterpolationMode = InterpolationMode.HighQualityBicubic;
            gfx.PixelOffsetMode = PixelOffsetMode.HighQuality;
            gfx.DrawImage(img_cortar, new Rectangle(0, 0, w, h), x, y, w, h, GraphicsUnit.Pixel);

            //bmp_crop.Save("test.bmp");

            // Dispose to free up resources
            img_crop.Dispose();
            //bmp_crop.Dispose();// <------------ How to use this and pass Bitmap to the next class?
            gfx.Dispose();

            Class2(bmp_crop);
            
            return bmp_crop;

        }

I need the image cropped in another class, if I use Dispose() (bmp_crop.Dispose();), in the next class I get a parameter error, how can I discard the Bitmap and still be able to use it in the next class?

public static Bitmap Class2(Bitmap bmp_crop)
    {
    ...
    }
  • 1
    what is parameter error? and `Class2` is method not class in your code – Vivek Nuna Jun 27 '20 at 05:25
  • **You shouldn't dispose `img_crop` nor `bmp_crop`** because your `Crop()` method doesn't "own" either (ownership of `img_crop` belongs to whatever calls your `Crop` method (or a caller of that), and ownershop of `bmp_crop` is transferred from `Crop()` to its caller when it returns `bmp_crop`. – Dai Jun 27 '20 at 05:25
  • It's easy to get confused about object-"ownership" and lifetime in C# because C# doesn't have any syntax to denote object-lifetime (unlike Rust, for example), this is why when you have a method or constructor that receives an `IDisposable` that you document your intent w.r.t. ownership and lifetime (e.g. passing an `IDisposable` to a constructor to transfer ownership, or just to allow the object or method to use it) – Dai Jun 27 '20 at 05:27
  • You don't need to dispose `bmp_crop` and it won't waste resources if you have it in multiple places. `bmp_crop` is just a reference to a bitmap, not the actual bitmap. so when you put `bmp_crop` into another class, only a reference is copied (which is 32 or 64 bit in size). the actual bitmap taking resources is still one and you can dispose it when you are done with it. look for `using` statement. https://stackoverflow.com/questions/212198/what-is-the-c-sharp-using-block-and-why-should-i-use-it – M.kazem Akhgary Jun 27 '20 at 06:08
  • @M.kazemAkhgary The `bmp_crop` local variable is the only reference to the Bitmap (C++-style stack allocation doesn't apply here), also notice that this method **returns** the bitmap, so it must not dispose of it nor use a `using` block at all (except in an exception handler). – Dai Jun 27 '20 at 06:27
  • Your `Class2` method returns a `Bitmap` - why? What does `Class2` actually do? What does it return, exactly? Why doesn't your `Crop` method do anything with `Class2`'s return value? – Dai Jun 27 '20 at 06:35
  • Or you could hand these operations off to the likes of ImageResizer or ImageMagick and not have to worry about it – Caius Jard Jun 27 '20 at 09:33

1 Answers1

0
  • 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 );
    }
}
Dai
  • 141,631
  • 28
  • 261
  • 374
  • Perfect, it worked. Thanks to your class above, thanks to it I managed to optimize other parts of the code. Would it be too much to ask for help with another question of mine? Thank you very much for this help and some future help. https://stackoverflow.com/questions/62488594/c-sharp-click-and-drag-or-mouse-wheel-with-postmessage-sendmessage – Maurício Morhy Jun 28 '20 at 00:40