Skip to content

Remove the System.Drawing dependency #6363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 20, 2022

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Oct 9, 2022

Description

The change here is removing the dependency on System.Drawing and replacing it with a public class Imager for image handling. It allows users to use Imager directly for image handling. It is important to note that this is a breaking change.
The change contains update to the samples code too.

Public Interfaces

namespace Microsoft.ML.Data
{
    /// <summary>
    /// Provide interfaces for imaging operations.
    /// </summary>
    public class MLImage : IDisposable
    {
        /// <summary>
        /// Create a new MLImage instance from a stream.
        /// </summary>
        /// <param name="imageStream">The stream to create the image from.</param>
        /// <returns>MLImage object.</returns>
        public static MLImage CreateFromStream(Stream imageStream)
        {
        }

        /// <summary>
        /// Create a new MLImage instance from a stream.
        /// </summary>
        /// <param name="imagePath">The image file path to create the image from.</param>
        /// <returns>MLImage object.</returns>
        public static MLImage CreateFromFile(string imagePath)
        {
        }

        /// <summary>
        /// Creates MLImage object from the pixel data span.
        /// </summary>
        /// <param name="width">The width of the image in pixels.</param>
        /// <param name="height">The height of the image in pixels.</param>
        /// <param name="pixelFormat">The pixel format to create the image with.</param>
        /// <param name="imagePixelData">The pixels data to create the image from.</param>
        /// <returns>MLImage object.</returns>
        public static unsafe MLImage CreateFromPixels(int width, int height, MLPixelFormat pixelFormat, ReadOnlySpan<byte> imagePixelData)
        {
        }

        /// <summary>
        /// Gets the pixel format for this Image.
        /// </summary>
        public MLPixelFormat PixelFormat { get; }

        /// <summary>
        /// Gets the image pixel data.
        /// </summary>
        public ReadOnlySpan<byte> Pixels { get { } }

        /// <summary>
        /// Gets or sets the image tag.
        /// </summary>
        public string Tag { get; set; }

        /// <summary>
        /// Gets the image width in pixels.
        /// </summary>
        public int Width { get { } }

        /// <summary>
        /// Gets the image height in pixels.
        /// </summary>
        public int Height { get { } }

        /// <summary>
        /// Gets how many bits per pixel used by current image object.
        /// </summary>
        public int BitsPerPixel { get { } }

        /// <summary>
        /// Save the current image to a file.
        /// </summary>
        /// <param name="imagePath">The path of the file to save the image to.</param>
        /// <remarks>The saved image encoding will be detected from the file extension.</remarks>
        public void Save(string imagePath) 
        {
        }

        /// <summary>
        /// Disposes the image.
        /// </summary>
        public void Dispose()
        {
        }
    }

    /// <summary>
    /// The mode to decide how the image should be resized.
    /// </summary>
    public enum MLPixelFormat
    {
        /// <summary>
        /// Pads the resized image to fit the bounds of its container.
        /// </summary>
        Unknown,

        /// <summary>
        /// Specifies that the format is 32 bits per pixel; 8 bits each are used for the blue, green, red, and alpha components.
        /// </summary>
        Bgra32,

        /// <summary>
        /// Specifies that the format is 32 bits per pixel; 8 bits each are used for the red, green, blue, and alpha components.
        /// </summary>
        Rgba32
    }

Examples

Load Image

using Stream stream = new FileStream(@"tomato.bmp", FileMode.Open);
MLImage image = MLImage.CreateFromStream(stream);

Load from pixel data

byte[] imageData = new byte[width * height * 4]; // 4 for the red, green, blue, and alpha colors
for (int i = 0; i < imageData.Length; i += 4)
{
    // Fill the buffer with the Bgra32 format
    imageData[i] = blue;
    imageData[i + 1] = green;
    imageData[i + 2] = red;
    imageData[i + 3] = alpha;
}

MLImage image = MLImage .CreateFromPixels(width, height, MLPixelFormat.Bgra32, imageData);

Access the image pixel data

ReadOnlySpan<byte> pixelData = image.Pixels;

Save the image

image. Save(@"foo.png");

*Breaking Change
The breaking change is mostly about the used data with the Image analytic transformers. Previously, System.Drawing.Bitmap type was used. Now this is replaced with the Imager type. Here is an example:

-var image = Image.FromFile(imagePath) as Bitmap;
+var image = MLImage.CreateFromFile(imagePath);

and then use the created image object, something like:

var prediction = transformer.Predict(image);

It is a good idea to look at the sample code changes in this PR which give a better idea how users can migrate from using System.Drawing to Imager class.

@ghost ghost assigned tarekgh Oct 9, 2022
@tarekgh tarekgh requested a review from michaelgsharp October 9, 2022 21:03
@tarekgh
Copy link
Member Author

tarekgh commented Oct 9, 2022

CC @ericstj @luisquintanilla @NiklasGustafsson @JakeRadMSFT @LittleLittleCloud

@codecov
Copy link

codecov bot commented Oct 9, 2022

Codecov Report

Merging #6363 (152e065) into main (d2e2f4f) will decrease coverage by 0.08%.
The diff coverage is 83.33%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6363      +/-   ##
==========================================
- Coverage   68.59%   68.51%   -0.09%     
==========================================
  Files        1171     1166       -5     
  Lines      248036   246586    -1450     
  Branches    25736    25764      +28     
==========================================
- Hits       170150   168947    -1203     
+ Misses      71126    70924     -202     
+ Partials     6760     6715      -45     
Flag Coverage Δ
Debug 68.51% <83.33%> (-0.09%) ⬇️
production 62.92% <80.98%> (-0.05%) ⬇️
test 88.92% <85.03%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...c/Microsoft.ML.ImageAnalytics/ExtensionsCatalog.cs 21.42% <ø> (ø)
src/Microsoft.ML.TimeSeries/ExtensionsCatalog.cs 88.23% <ø> (ø)
...cenariosWithDirectInstantiation/TensorflowTests.cs 92.82% <ø> (+0.46%) ⬆️
src/Microsoft.ML.ImageAnalytics/ImageResizer.cs 72.05% <69.56%> (-3.12%) ⬇️
...Microsoft.ML.ImageAnalytics/ImagePixelExtractor.cs 72.22% <71.42%> (-0.40%) ⬇️
src/Microsoft.ML.ImageAnalytics/MLImage.cs 80.25% <80.25%> (ø)
test/Microsoft.ML.Tests/ImagesTests.cs 91.79% <84.66%> (-6.21%) ⬇️
src/Microsoft.ML.ImageAnalytics/ImageGrayscale.cs 72.64% <100.00%> (-3.59%) ⬇️
src/Microsoft.ML.ImageAnalytics/ImageLoader.cs 82.22% <100.00%> (+0.56%) ⬆️
src/Microsoft.ML.ImageAnalytics/ImageType.cs 76.47% <100.00%> (ø)
... and 50 more

@tarekgh
Copy link
Member Author

tarekgh commented Oct 10, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly comments on the API

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing the API suggestions, that part LGTM. Added some comments on docs / testing. @michaelgsharp can you be sure to review usage?

Copy link
Contributor

@dakersnar dakersnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not totally familiar with all of the internal algorithms used, but overall LGTM.

I second most of Eric's suggestions, especially adding unit tests to MLImage.

@mapo80
Copy link

mapo80 commented Oct 20, 2022

I hope to be able to use this version as soon as possible! Thanks

@michaelgsharp michaelgsharp merged commit 018293c into dotnet:main Oct 20, 2022
@tarekgh tarekgh deleted the RemoveSystemDrawingDependency branch October 20, 2022 21:40
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants