Resizing image but keeping aspect ratio

Question

I wrote some code that resizes an image to fit in a given box. I think that some parts of the code are not needed (for example if it's a box image). Is there a more efficient way to do this? Is there something wrong with my code (from the results it looks ok)? Is there a "shorter" version to do this?

Bitmap original, resizedImage;

try
{
    using (FileStream fs = new System.IO.FileStream(imageLabel.Text, System.IO.FileMode.Open))
    {
        original = new Bitmap(fs);
    }

    int rectHeight = BOXHEIGHT;
    int rectWidth = BOXWIDTH;

    //if the image is squared set it's height and width to the smallest of the desired dimensions (our box). In the current example rectHeight<rectWidth
    if (original.Height == original.Width)
    {
        resizedImage = new Bitmap(original, rectHeight, rectHeight);
    }
    else
    {
        //calculate aspect ratio
        float aspect = original.Width / (float)original.Height;
        int newWidth, newHeight;

        //calculate new dimensions based on aspect ratio
        newWidth = (int)(rectWidth * aspect);
        newHeight = (int)(newWidth / aspect);

        //if one of the two dimensions exceed the box dimensions
        if (newWidth > rectWidth || newHeight > rectHeight)
        {
            //depending on which of the two exceeds the box dimensions set it as the box dimension and calculate the other one based on the aspect ratio
            if (newWidth > newHeight)
            {
                newWidth = rectWidth;
                newHeight = (int)(newWidth / aspect);
            }
            else
            {
                newHeight = rectHeight;
                newWidth = (int)(newHeight * aspect);
            }
        }
        resizedImage = new Bitmap(original, newWidth, newHeight);
    }
}
catch (Exception ex)
{
    MessageBox.Show(ex.Message);
}

Answer 1

Your algorithm to calculate the final image dimensions could be improved by simplifying the logical process that you use to determine the final size.

Fundamentally, there are two dimensions you are interested in, the height, and width of the final image. There are two possible outcomes for the scaling, one where the scaling produces the target height, and the other where it produces the target width. The scaling that is used is the one which produces the smallest image. The size of the final image is determined by the scaling factor, and there are two factors:

float scaleHeight = (float)BOXHEIGHT / (float)original.Height;
float scaleWidth = (float)BOXWIDTH / (float)original.Width;

Now, we have two scaling ratios, which one produces the smaller image? The one that has the smallest scaling factor.

float scale = Math.Min(scaleHeight, scaleWidth);

Now you can simply do the adjustment:

resizedImage = new Bitmap(original,
         (int)(original.Width * scale), (int)(original.Height * scale));

Answer 2

Here's how you can refactor your code a bit:

namespace ConsoleApplication1
{
    using System;
    using System.Drawing;
    using System.IO;

    public class Program
    {
        public static void Main(string[] args)
        {
        }

        private const int BOXWIDTH = 1;

        public static Bitmap Foo()
        {    
            try
            {
                Bitmap original = LoadOriginalImage(imageLabel.Text);

                if (ImageIsBox(original))
                    return new Bitmap(original, BOXWIDTH, BOXWIDTH);

                var newDimensions = CalculateNewDimensionsForImage(original);

                return new Bitmap(original, newDimensions.Width, newDimensions.Height);
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
        }

        private static Bitmap LoadOriginalImage(string imagePath)
        {
            using (FileStream fs = new FileStream(imagePath, FileMode.Open))
            {
                return new Bitmap(fs);
            }
        }

        private static bool ImageIsBox(Bitmap original)
        {
            return original.Height == original.Width;
        }

        private static ImageDimensions CalculateNewDimensionsForImage(Bitmap original)
        {
            //calculate aspect ratio
            float aspect = original.Width / (float)original.Height;
            int newWidth, newHeight;

            //calculate new dimensions based on aspect ratio
            newWidth = (int)(BOXWIDTH * aspect);
            newHeight = (int)(newWidth / aspect);

            //if one of the two dimensions exceed the box dimensions
            if (newWidth > BOXWIDTH || newHeight > BOXWIDTH)
            {
                //depending on which of the two exceeds the box dimensions set it as the box dimension and calculate the other one based on the aspect ratio
                if (newWidth > newHeight)
                {
                    newWidth = BOXWIDTH;
                    newHeight = (int)(newWidth / aspect);
                }
                else
                {
                    newHeight = BOXWIDTH;
                    newWidth = (int)(newHeight * aspect);
                }
            }

            return new ImageDimensions()
            {
                Height = newHeight,
                Width = newWidth
            };
        }
    }

    public class ImageDimensions
    {
        public int Height { get; set; }
        public int Width { get; set; }
    }
}

I've written a console app that tries to illustrate how you could simplify your code a bit. The end result is that a lot of variables that were using are now gone e.g. resizedImage, rectHeight, rectWidth.

This might be useful for you, if not, no worries, I enjoyed writing it :)

Answer 3

Style

• Declaring multiple variables on the same line should be avoided for the benefit of readability

• Using braces {} for single if statements also is good

• Using meaningful names for variables is good

General

This code is tightly coupled to Windows Forms or WPF (imageLabel) which isn't needed. Also it is coupled to BOXWIDTH constant. The method should be static as it doesn't change any member variables.

Your method signature should look like

private static Bitmap GetResizedImage(String fileName, Int32 maxWidth, Int32 maxHeight)
{

}   

and be called like

Bitmap resized = GetResizedImage(imageLabel.Text, BOXWIDTH, BOXHEIGHT);

Answer 4

Your code looks pretty neat to me. I have a few remarks listed below.

Variable names:

Don't mix up names, this makes things confusing:

int rectHeight = BOXWIDTH;
int rectWidth = BOXWIDTH;

Change it to:

int boxHeight = BOXWIDTH;
int boxWidth = BOXWIDTH;

Now you know you're talking about the box and not mistaking it for some possible rectangle.

Don't uppercase field names, use pascalCase. Also, aspect is not a very good name, choose a name like ratio or aspectRatio.

var keyword:

Use var instead of declaring your varibales explicitly, let the compiler decide the type. Your code will also look cleaner:

var aspect = original.Width / (float)original.Height;