Skip to content
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

Expose non-nullable configuration to remove AOT limiting null check #2514

Merged
merged 8 commits into from
Oct 5, 2023
40 changes: 4 additions & 36 deletions src/ImageSharp/Advanced/AdvancedImageExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,25 @@ public static IImageEncoder DetectEncoder(this Image source, string filePath)
Guard.NotNull(filePath, nameof(filePath));

string ext = Path.GetExtension(filePath);
if (!source.GetConfiguration().ImageFormatsManager.TryFindFormatByFileExtension(ext, out IImageFormat? format))
if (!source.Configuration.ImageFormatsManager.TryFindFormatByFileExtension(ext, out IImageFormat? format))
{
StringBuilder sb = new();
sb = sb.AppendLine(CultureInfo.InvariantCulture, $"No encoder was found for extension '{ext}'. Registered encoders include:");
foreach (IImageFormat fmt in source.GetConfiguration().ImageFormats)
foreach (IImageFormat fmt in source.Configuration.ImageFormats)
{
sb = sb.AppendFormat(CultureInfo.InvariantCulture, " - {0} : {1}{2}", fmt.Name, string.Join(", ", fmt.FileExtensions), Environment.NewLine);
}

throw new UnknownImageFormatException(sb.ToString());
}

IImageEncoder? encoder = source.GetConfiguration().ImageFormatsManager.GetEncoder(format);
IImageEncoder? encoder = source.Configuration.ImageFormatsManager.GetEncoder(format);

if (encoder is null)
{
StringBuilder sb = new();
sb = sb.AppendLine(CultureInfo.InvariantCulture, $"No encoder was found for extension '{ext}' using image format '{format.Name}'. Registered encoders include:");
foreach (KeyValuePair<IImageFormat, IImageEncoder> enc in source.GetConfiguration().ImageFormatsManager.ImageEncoders)
foreach (KeyValuePair<IImageFormat, IImageEncoder> enc in source.Configuration.ImageFormatsManager.ImageEncoders)
{
sb = sb.AppendFormat(CultureInfo.InvariantCulture, " - {0} : {1}{2}", enc.Key, enc.Value.GetType().Name, Environment.NewLine);
}
Expand Down Expand Up @@ -76,30 +76,6 @@ public static void AcceptVisitor(this Image source, IImageVisitor visitor)
public static Task AcceptVisitorAsync(this Image source, IImageVisitorAsync visitor, CancellationToken cancellationToken = default)
=> source.AcceptAsync(visitor, cancellationToken);

/// <summary>
/// Gets the configuration for the image.
/// </summary>
/// <param name="source">The source image.</param>
/// <returns>Returns the configuration.</returns>
public static Configuration GetConfiguration(this Image source)
Copy link
Member

Choose a reason for hiding this comment

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

we should probably leave these in despite then no longer being needed for backwards compatability, anyone with a dependency on this public API will break otherwise, shoudl mark them Obsolete instead

Copy link
Member

@antonfirsov antonfirsov Oct 4, 2023

Choose a reason for hiding this comment

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

There are several breaking changes coming to 3.1 already without obsoletion eg. #2455 or #2485. I see little point using Obsolete selectively.

What is worrying me is that the net amount of breaking changes between 3.0.* -> 3.1.0 is much bigger than between 2.0.* -> 2.1.0. Maybe we should rename 3.1 to 4.0 to avoid confusion?

Copy link
Member Author

Choose a reason for hiding this comment

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

My plan with major versions has always been to support latest the .NET LTS version. There are breaking changes (unfortunately more than I'd like), but they typically only affect scenarios involving advanced behavior.

=> GetConfiguration((IConfigurationProvider)source);

/// <summary>
/// Gets the configuration for the image frame.
/// </summary>
/// <param name="source">The source image.</param>
/// <returns>Returns the configuration.</returns>
public static Configuration GetConfiguration(this ImageFrame source)
=> GetConfiguration((IConfigurationProvider)source);

/// <summary>
/// Gets the configuration.
/// </summary>
/// <param name="source">The source image</param>
/// <returns>Returns the bounds of the image</returns>
private static Configuration GetConfiguration(IConfigurationProvider source)
=> source?.Configuration ?? Configuration.Default;

/// <summary>
/// Gets the representation of the pixels as a <see cref="IMemoryGroup{T}"/> containing the backing pixel data of the image
/// stored in row major order, as a list of contiguous <see cref="Memory{T}"/> blocks in the source image's pixel format.
Expand Down Expand Up @@ -167,12 +143,4 @@ public static Memory<TPixel> DangerousGetPixelRowMemory<TPixel>(this Image<TPixe

return source.Frames.RootFrame.PixelBuffer.GetSafeRowMemory(rowIndex);
}

/// <summary>
/// Gets the <see cref="MemoryAllocator"/> assigned to 'source'.
/// </summary>
/// <param name="source">The source image.</param>
/// <returns>Returns the configuration.</returns>
internal static MemoryAllocator GetMemoryAllocator(this IConfigurationProvider source)
=> GetConfiguration(source).MemoryAllocator;
}
2 changes: 1 addition & 1 deletion src/ImageSharp/Advanced/IConfigurationProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace SixLabors.ImageSharp.Advanced;
/// <summary>
/// Defines the contract for objects that can provide access to configuration.
/// </summary>
internal interface IConfigurationProvider
public interface IConfigurationProvider
{
/// <summary>
/// Gets the configuration which allows altering default behaviour or extending the library.
Expand Down
2 changes: 1 addition & 1 deletion src/ImageSharp/Formats/Bmp/BmpEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public sealed class BmpEncoder : QuantizingImageEncoder
/// <inheritdoc/>
protected override void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken cancellationToken)
{
BmpEncoderCore encoder = new(this, image.GetMemoryAllocator());
BmpEncoderCore encoder = new(this, image.Configuration.MemoryAllocator);
encoder.Encode(image, stream, cancellationToken);
}
}
2 changes: 1 addition & 1 deletion src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
Guard.NotNull(image, nameof(image));
Guard.NotNull(stream, nameof(stream));

Configuration configuration = image.GetConfiguration();
Configuration configuration = image.Configuration;
ImageMetadata metadata = image.Metadata;
BmpMetadata bmpMetadata = metadata.GetBmpMetadata();
this.bitsPerPixel ??= bmpMetadata.BitsPerPixel;
Expand Down
2 changes: 1 addition & 1 deletion src/ImageSharp/Formats/Gif/GifEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public sealed class GifEncoder : QuantizingImageEncoder
/// <inheritdoc/>
protected override void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken cancellationToken)
{
GifEncoderCore encoder = new(image.GetConfiguration(), this);
GifEncoderCore encoder = new(image.Configuration, this);
encoder.Encode(image, stream, cancellationToken);
}
}
2 changes: 1 addition & 1 deletion src/ImageSharp/Formats/Gif/GifEncoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private void EncodeAdditionalFrames<TPixel>(
// This frame is reused to store de-duplicated pixel buffers.
// This is more expensive memory-wise than de-duplicating indexed buffer but allows us to deduplicate
// frames using both local and global palettes.
using ImageFrame<TPixel> encodingFrame = new(previousFrame.GetConfiguration(), previousFrame.Size());
using ImageFrame<TPixel> encodingFrame = new(previousFrame.Configuration, previousFrame.Size());

for (int i = 1; i < image.Frames.Count; i++)
{
Expand Down
4 changes: 2 additions & 2 deletions src/ImageSharp/Formats/ImageEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protected abstract void Encode<TPixel>(Image<TPixel> image, Stream stream, Cance
private void EncodeWithSeekableStream<TPixel>(Image<TPixel> image, Stream stream, CancellationToken cancellationToken)
where TPixel : unmanaged, IPixel<TPixel>
{
Configuration configuration = image.GetConfiguration();
Configuration configuration = image.Configuration;
if (stream.CanSeek)
{
this.Encode(image, stream, cancellationToken);
Expand All @@ -59,7 +59,7 @@ private void EncodeWithSeekableStream<TPixel>(Image<TPixel> image, Stream stream
private async Task EncodeWithSeekableStreamAsync<TPixel>(Image<TPixel> image, Stream stream, CancellationToken cancellationToken)
where TPixel : unmanaged, IPixel<TPixel>
{
Configuration configuration = image.GetConfiguration();
Configuration configuration = image.Configuration;
if (stream.CanSeek)
{
await DoEncodeAsync(stream).ConfigureAwait(false);
Expand Down
Loading
Loading