Added Image<TPixel> constructor from single ImageFrame

[br3aker]

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 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

Added contructor from single frame used in some cases (and will be used in new version of jpeg decoder). Replaced multi-frame ctors with single-frame ctors where applicable.

[codecov]

Codecov Report

Merging #1680 (67cab31) into master (0099bb0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1680   +/-   ##
=======================================
  Coverage   84.32%   84.32%           
=======================================
  Files         816      816           
  Lines       35890    35899    +9     
  Branches     4182     4182           
=======================================
+ Hits        30263    30272    +9     
  Misses       4804     4804           
  Partials      823      823           

Flag	Coverage Δ
unittests	84.32% <100.00%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
.../Formats/Tiff/Writers/TiffBiColorWriter{TPixel}.cs	95.74% <100.00%> (ø)
src/ImageSharp/ImageFrameCollection{TPixel}.cs	91.79% <100.00%> (+0.38%)	⬆️
src/ImageSharp/Image{TPixel}.cs	94.11% <100.00%> (+0.21%)	⬆️
...cessors/Transforms/EntropyCropProcessor{TPixel}.cs	88.88% <100.00%> (ø)

---

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 dcd9890...67cab31. Read the comment docs.

[antonfirsov]

@br3aker I've seen your #1597 (comment), just didn't have chance to react yet. I don't see the point in detaching ImageFrame<T> and Image<T> this much, the existing ImageFrame[] constructor is intended to support image.Clone() functionality AFAIK.

Is there anything preventing you to create an Image<T> with Image.CreateUninitialized<T>() in SpectralToImageConverter<T> instead of creating an ImageFrame<T>?

[br3aker]

@antonfirsov I didn't remove already existing API for multi-frame images just added yet another internal contructor which already fixes several new[] { singleFrame }. While it's only a couple of bytes wasted it's still a waste which can be easily avoided without breaking semantics of the constructors.

You are right, ImageFrame[] is used for cloning but is not limited by it. For example, Tiff decoder initializes image with ctor dependent on provided ImageFrame collection. Jpeg can't have more than one frame and it's 'natural' to create jpeg image from single frame, it's semantically correct for this format. And it's internal anyway, end user won't be able to break anything.

I prefer creating ImageFrame<T> instead of final Image<T> for 3 reasons:

1. JpegDecoderCore produces Image<T>, including pixel buffer itself and related metadata. Practically JpegDecoderCore produces metadata and ScanDecoder + SpectralConverter produces pixel buffer. And only then everything is merged together to return Image<T>.
2. The only thing preventing Image<T> creation is ImageMetadata instance. While it can be easily passed to the converter ctor, it won't be used at all just passed to actual Image<T> ctor. And we don't even know if metadata is 'initialized', yes, it's a class and we only want the reference but it looks clunky to be honest. "Why would pixel converter use metadata on top of already injected decoding settings?"
3. As I said, semantics. This jpeg is a single frame image and you even have a Start of Frame marker.

Furthermore, as in converter we work with pixel buffer directly in hot path, it's usually better to save extra reference to frame itself which would look somewhat like this:

class SpectralConverter<T>
{
    // exists here only to return after entire conversion chain
    public Image<T> Image { get; private set; }
    
    // actually used field
    private ImageFrame<T> frame;

    // ...
}

[antonfirsov]

Sorry for disappearing, stuck in analysis paralysis with this a bit. The justification makes sense, but in long term we need to aim for decoder techniques, that can be refactored to work with public API-s. ImageFrame<T> has been designed to be created and managed with ImageFrameCollection<T>, it's constructor will be never public, same true for the proposed new Image<T> constructor.

Maybe we should define public Image<T> constructors which consume, and take ownership of a Buffer2D<T> instead? You can fill the Buffer2D<T> in SpectralToImageConverter<T> then.

@JimBobSquarePants thoughts?

[JimBobSquarePants]

I'll have to review how that works with our existing APIs. I'm loath to expose more public constructors unless we absolutely need them. All the wrapping ones we have already do make me uneasy.

[br3aker]

Sorry for no response, was fixing cooling on my pc.

@antonfirsov Buffer2D constructor is even more suitable for converters, I like that!

@JimBobSquarePants maybe we can expose internal Image<T> constructor which accepts existing ImageFrameCollection<T>? That way constructors for single and many frames would go away.

[br3aker]

As new Jpeg decoding pipeline have been merged, this is no longer needed, closing this for now.