Why would you create a new Bitmap to convert to BitmapImage?

Question

I have a converter in some legacy code, that is doing something that seems wrong, but I don't know bitmaps very well. It looks like it's based on https://stackoverflow.com/a/3427114/57883 with some added capabilities.

using System;
using System.Collections.Generic;
using System.Drawing.Imaging;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Text;
using System.Windows.Data;
using System.Windows.Media.Imaging;

namespace CompanyName.Converters
{
    [ValueConversion(typeof(System.Drawing.Image), typeof(System.Windows.Media.ImageSource))]

    /// <summary>
    /// One-way converter from System.Drawing.Image to System.Windows.Media.ImageSource
    /// </summary>
    public class ImageConverter : IValueConverter
    {
        public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
        {
            // empty images are empty...
            if (value == null) 
            { 
                return null; 
            }

            if (value.GetType() == typeof(System.Drawing.Image))
            {
                var image = (System.Drawing.Image)value;

            // Winforms Image we want to get the WPF Image from...
            var bitmap = new System.Windows.Media.Imaging.BitmapImage();
            bitmap.BeginInit();
            MemoryStream memoryStream = new MemoryStream();

            // Save to a memory stream...
            image.Save(memoryStream, ImageFormat.Bmp);

            // Rewind the stream...
            memoryStream.Seek(0, System.IO.SeekOrigin.Begin);
            bitmap.StreamSource = memoryStream;
            bitmap.EndInit();
            bitmap.Freeze();
            return bitmap;
            }

            else if (value.GetType() == typeof(System.Drawing.Bitmap))
            {
                var image = value as System.Drawing.Bitmap;

                System.Drawing.Bitmap bitmap = new System.Drawing.Bitmap(image);

                using (MemoryStream memory = new MemoryStream())
                {
                    bitmap.Save(memory, ImageFormat.Png);
                    memory.Position = 0;
                    BitmapImage bitmapImage = new BitmapImage();
                    bitmapImage.BeginInit();
                    bitmapImage.StreamSource = memory;
                    bitmapImage.CacheOption = BitmapCacheOption.OnLoad;
                    bitmapImage.EndInit();
                    bitmapImage.Freeze();
                    return bitmapImage;
                }
            }

            return null;
        }

        public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
        {
            return null;
        }
    }
}

Why in the world would you System.Drawing.Bitmap bitmap = new System.Drawing.Bitmap(image); ? isn't this just creating a copy of the bitmap in memory without any benefit? Is this just horrible code or is there a legitimate reason to copy the bitmap before converting it?

It is just horrible code. `System.Drawing.Image` is an abstract class, hence `value.GetType() == typeof(System.Drawing.Image)` will always be false. It should be `value is System.Drawing.Image` instead. The `else` block would then be redundant, as `Bitmap` is derived from `Image`. You would of course keep the `using` block for the MemoryStream from the `else` part. — Clemens, Sep 02 '15 at 04:53
@Clemens can the `Bitmap` created in the else branch be safely disposed at this point? or does the `BitmapImage` still need it? — Maslow, Sep 02 '15 at 13:27
@Clemens well, no. In the answer posted by Mark here, it points out that `.GetType()` would never return an abstract class. so the if block is entirely useless, the else is what actually happens. — Maslow, Sep 02 '15 at 15:21
That doesn't matter, as a Bitmap is also an Image. When you just check `if (value is System.Drawing.Image)` (as I wrote before), that will handle all Image-derived classes, *including* Bitmap in the same way. — Clemens, Sep 02 '15 at 16:41

score 1 · Answer 1 · answered Sep 02 '15 at 02:23

Copying the bitmap via the constructor without explicitly specifying the PixelFormat will result in the image being converted to 32bpp argb, which will in turn prevent problems arising if you try to bind to an image with a pixel format that the target doesn't support. In the case of WPF the Image control does render indexed images correctly but there might be other formats or other control types that will fail. The original author was probably just trying to cover all bases.

this is why we can't have nice coded things. thanks for the side effect knowledge, and good reason not to remove it. — Maslow, Sep 02 '15 at 13:24

Why would you create a new Bitmap to convert to BitmapImage?

1 Answers1