Skip to content

Conversation

@pekspro
Copy link
Contributor

@pekspro pekspro commented Jul 4, 2020

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This add several methods for saving images. Id addresses #1255.

There were no unit test for the existing extensions so I did not ceate anyone for this either. But I did make sure that all extensions for all file formats produced identical files with this code:

using SixLabors.ImageSharp;
using System;
using System.IO;
using System.Threading.Tasks;

namespace ConsoleAppImageSharpTester
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var img = Image.Load(@"testimage.png");

            Directory.CreateDirectory(@".\test");

            {
                img.SaveAsTga(@".\test\norm_name.tga");
                img.SaveAsTga(@".\test\norm_name_enc.tga", new SixLabors.ImageSharp.Formats.Tga.TgaEncoder());
                await img.SaveAsTgaAsync(@".\test\async_name.tga");
                await img.SaveAsTgaAsync(@".\test\async_name_enc.tga", new SixLabors.ImageSharp.Formats.Tga.TgaEncoder());
                using var f1 = new FileStream(@".\test\norm_stream.tga", FileMode.OpenOrCreate);
                img.SaveAsTga(f1);
                using var f2 = new FileStream(@".\test\norm_stream_enc.tga", FileMode.OpenOrCreate);
                img.SaveAsTga(f2, new SixLabors.ImageSharp.Formats.Tga.TgaEncoder());
                using var f3 = new FileStream(@".\test\async_stream.tga", FileMode.OpenOrCreate);
                await img.SaveAsTgaAsync(f3);
                using var f4 = new FileStream(@".\test\async_stream_enc.tga", FileMode.OpenOrCreate);
                await img.SaveAsTgaAsync(f4, new SixLabors.ImageSharp.Formats.Tga.TgaEncoder());
            }


            {
                img.SaveAsPng(@".\test\norm_name.png");
                img.SaveAsPng(@".\test\norm_name_enc.png", new SixLabors.ImageSharp.Formats.Png.PngEncoder());
                await img.SaveAsPngAsync(@".\test\async_name.png");
                await img.SaveAsPngAsync(@".\test\async_name_enc.png", new SixLabors.ImageSharp.Formats.Png.PngEncoder());
                using var f1 = new FileStream(@".\test\norm_stream.png", FileMode.OpenOrCreate);
                img.SaveAsPng(f1);
                using var f2 = new FileStream(@".\test\norm_stream_enc.png", FileMode.OpenOrCreate);
                img.SaveAsPng(f2, new SixLabors.ImageSharp.Formats.Png.PngEncoder());
                using var f3 = new FileStream(@".\test\async_stream.png", FileMode.OpenOrCreate);
                await img.SaveAsPngAsync(f3);
                using var f4 = new FileStream(@".\test\async_stream_enc.png", FileMode.OpenOrCreate);
                await img.SaveAsPngAsync(f4, new SixLabors.ImageSharp.Formats.Png.PngEncoder());
            }
        

            {
                img.SaveAsBmp(@".\test\norm_name.bmp");
                img.SaveAsBmp(@".\test\norm_name_enc.bmp", new SixLabors.ImageSharp.Formats.Bmp.BmpEncoder());
                await img.SaveAsBmpAsync(@".\test\async_name.bmp");
                await img.SaveAsBmpAsync(@".\test\async_name_enc.bmp", new SixLabors.ImageSharp.Formats.Bmp.BmpEncoder());
                using var f1 = new FileStream(@".\test\norm_stream.bmp", FileMode.OpenOrCreate);
                img.SaveAsBmp(f1);
                using var f2 = new FileStream(@".\test\norm_stream_enc.bmp", FileMode.OpenOrCreate);
                img.SaveAsBmp(f2, new SixLabors.ImageSharp.Formats.Bmp.BmpEncoder());
                using var f3 = new FileStream(@".\test\async_stream.bmp", FileMode.OpenOrCreate);
                await img.SaveAsBmpAsync(f3);
                using var f4 = new FileStream(@".\test\async_stream_enc.bmp", FileMode.OpenOrCreate);
                await img.SaveAsBmpAsync(f4, new SixLabors.ImageSharp.Formats.Bmp.BmpEncoder());
            }


            {
                img.SaveAsGif(@".\test\norm_name.gif");
                img.SaveAsGif(@".\test\norm_name_enc.gif", new SixLabors.ImageSharp.Formats.Gif.GifEncoder());
                await img.SaveAsGifAsync(@".\test\async_name.gif");
                await img.SaveAsGifAsync(@".\test\async_name_enc.gif", new SixLabors.ImageSharp.Formats.Gif.GifEncoder());
                using var f1 = new FileStream(@".\test\norm_stream.gif", FileMode.OpenOrCreate);
                img.SaveAsGif(f1);
                using var f2 = new FileStream(@".\test\norm_stream_enc.gif", FileMode.OpenOrCreate);
                img.SaveAsGif(f2, new SixLabors.ImageSharp.Formats.Gif.GifEncoder());
                using var f3 = new FileStream(@".\test\async_stream.gif", FileMode.OpenOrCreate);
                await img.SaveAsGifAsync(f3);
                using var f4 = new FileStream(@".\test\async_stream_enc.gif", FileMode.OpenOrCreate);
                await img.SaveAsGifAsync(f4, new SixLabors.ImageSharp.Formats.Gif.GifEncoder());
            }


            {
                img.SaveAsJpeg(@".\test\norm_name.jpg");
                img.SaveAsJpeg(@".\test\norm_name_enc.jpg", new SixLabors.ImageSharp.Formats.Jpeg.JpegEncoder());
                await img.SaveAsJpegAsync(@".\test\async_name.jpg");
                await img.SaveAsJpegAsync(@".\test\async_name_enc.jpg", new SixLabors.ImageSharp.Formats.Jpeg.JpegEncoder());
                using var f1 = new FileStream(@".\test\norm_stream.jpg", FileMode.OpenOrCreate);
                img.SaveAsJpeg(f1);
                using var f2 = new FileStream(@".\test\norm_stream_enc.jpg", FileMode.OpenOrCreate);
                img.SaveAsJpeg(f2, new SixLabors.ImageSharp.Formats.Jpeg.JpegEncoder());
                using var f3 = new FileStream(@".\test\async_stream.jpg", FileMode.OpenOrCreate);
                await img.SaveAsJpegAsync(f3);
                using var f4 = new FileStream(@".\test\async_stream_enc.jpg", FileMode.OpenOrCreate);
                await img.SaveAsJpegAsync(f4, new SixLabors.ImageSharp.Formats.Jpeg.JpegEncoder());
            }



        }
    }
}

@CLAassistant
Copy link

CLAassistant commented Jul 4, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jul 4, 2020

Codecov Report

Merging #1256 into master will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1256      +/-   ##
==========================================
+ Coverage   82.61%   82.68%   +0.07%     
==========================================
  Files         697      697              
  Lines       30529    30589      +60     
  Branches     3445     3460      +15     
==========================================
+ Hits        25221    25294      +73     
+ Misses       4603     4593      -10     
+ Partials      705      702       -3     
Flag Coverage Δ
#unittests 82.68% <100.00%> (+0.07%) ⬆️
Impacted Files Coverage Δ
src/ImageSharp/Formats/Bmp/ImageExtensions.cs 100.00% <100.00%> (ø)
src/ImageSharp/Formats/Gif/ImageExtensions.cs 100.00% <100.00%> (ø)
src/ImageSharp/Formats/Jpeg/ImageExtensions.cs 100.00% <100.00%> (ø)
src/ImageSharp/Formats/Png/ImageExtensions.cs 100.00% <100.00%> (ø)
src/ImageSharp/Formats/Tga/ImageExtensions.cs 100.00% <100.00%> (ø)
src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs 96.85% <0.00%> (+1.88%) ⬆️
src/ImageSharp/Formats/Tga/TgaEncoderCore.cs 99.35% <0.00%> (+1.94%) ⬆️
src/ImageSharp/Formats/Gif/GifFormat.cs 100.00% <0.00%> (+11.11%) ⬆️
src/ImageSharp/Formats/Bmp/BmpFormat.cs 100.00% <0.00%> (+12.50%) ⬆️
src/ImageSharp/Formats/Jpeg/JpegFormat.cs 100.00% <0.00%> (+12.50%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0594f7...8cd3948. Read the comment docs.

Copy link
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

Good job, looks good after a quick review! Thanks!

It's not your fault that there is no existing precedent to extensively test this type of boilerplate, but I'm a bit hesitant if it's ok to add this huge amount of uncovered methods.

@pekspro if I provide some guidance are you OK with adding the tests as well?

@pekspro
Copy link
Contributor Author

pekspro commented Jul 6, 2020

Sure @antonfirsov, I would like to see some unit test too on this :-) Just give me some guidance and I fix it :-)

As I see it, it would be nice if the extension methods worked with an interface instead of Image. But it is quit trick right now because of lines like this:

   encoder ?? source.GetConfiguration().ImageFormatsManager.FindEncoder(BmpFormat.Instance));

GetConfiguration does not return an interface, and ImageFormatsManager is also not an interface.

@pekspro
Copy link
Contributor Author

pekspro commented Jul 14, 2020

@antonfirsov , what if I created unit test like this?

    public class BmpImageExtensions
    {
        [Fact]
        public async Task SaveAsBmpAsync_Path()
        {
            string dir = TestEnvironment.CreateOutputDirectory(nameof(BmpImageExtensions));
            string file = Path.Combine(dir, "SaveAsBmpAsync_Path.bmp");

            using (var image = new Image<Rgba32>(10, 10))
            {
                await image.SaveAsBmpAsync(file);
            }

            using (Image.Load(file, out IImageFormat mime))
            {
                Assert.Equal("image/bmp", mime.DefaultMimeType);
            }
        }

        [Fact]
        public async Task SaveAsBmpAsync_Path_Encoder()
        {
            string dir = TestEnvironment.CreateOutputDirectory(nameof(BmpImageExtensions));
            string file = Path.Combine(dir, "SaveAsBmpAsync_Path_Encooder.bmp");

            using (var image = new Image<Rgba32>(10, 10))
            {
                await image.SaveAsBmpAsync(file, new BmpEncoder());
            }

            using (Image.Load(file, out IImageFormat mime))
            {
                Assert.Equal("image/bmp", mime.DefaultMimeType);
            }
        }

        [Fact]
        public async Task SaveAsBmpAsync_Stream()
        {
            using var memoryStream = new MemoryStream();

            using (var image = new Image<Rgba32>(10, 10))
            {
                await image.SaveAsBmpAsync(memoryStream);
            }

            memoryStream.Position = 0;

            using (Image.Load(memoryStream, out IImageFormat mime))
            {
                Assert.Equal("image/bmp", mime.DefaultMimeType);
            }
        }

        [Fact]
        public async Task SaveAsBmpAsync_Stream_Encoder()
        {
            using var memoryStream = new MemoryStream();

            using (var image = new Image<Rgba32>(10, 10))
            {
                await image.SaveAsBmpAsync(memoryStream, new BmpEncoder());
            }

            memoryStream.Position = 0;

            using (Image.Load(memoryStream, out IImageFormat mime))
            {
                Assert.Equal("image/bmp", mime.DefaultMimeType);
            }
        }
    }

@antonfirsov
Copy link
Member

@pekspro perfect, thanks! Go ahead!

@JimBobSquarePants
Copy link
Member

Perfect @pekspro Thanks so much for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants