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 Span methods on LoadPixelData and SavePixelData #567

Merged
merged 26 commits into from
May 12, 2018
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
92ab411
Expose LoadPixelData overloads accepting Span<T>
iamcarbon May 9, 2018
cddab85
Cleanup ByteExtensions
iamcarbon May 9, 2018
54791bf
Remove trailing whitespace
iamcarbon May 9, 2018
c6c9ae8
Simplify png decoder
iamcarbon May 9, 2018
1653a93
Simplify GifDecoder
iamcarbon May 9, 2018
ea38b24
Remove ByteExtensions
iamcarbon May 9, 2018
bddc20a
Simplfiy ToRgbaHex
iamcarbon May 9, 2018
1c79b08
Merge branch 'master' into memory-rc1
iamcarbon May 9, 2018
5156890
👮
iamcarbon May 9, 2018
15494b4
Merge branch 'memory-rc1' of https://github.com/carbon/ImageSharp int…
iamcarbon May 9, 2018
00af2df
Rename Gaurd.NotNullOrEmpty to NotNullOrWhiteSpace to match behavior …
iamcarbon May 10, 2018
eb318a3
Don't use Linq to check if IEnumerable is empty.
iamcarbon May 10, 2018
fd766a5
React to Gaurd changes
iamcarbon May 10, 2018
017c7fb
Verify ColorBuilder throws on null and empty
iamcarbon May 10, 2018
fda2d1f
Optimize ColorBuilder
iamcarbon May 10, 2018
4f779ca
Improve ColorBuilder test coverage
iamcarbon May 10, 2018
46e0a85
Allow leading period in FindFormatByFileExtension
iamcarbon May 10, 2018
202e472
Use ReadOnlySpans in LoadPixelData
iamcarbon May 10, 2018
45e5d5b
Allow pixel data to be saved directly to a span
iamcarbon May 10, 2018
09a77e4
Merge branch 'master' into memory-rc1
antonfirsov May 10, 2018
a05b618
Update Span ->ReadOnlySpan
iamcarbon May 11, 2018
92f353b
Tidy up ImageExtension docs
iamcarbon May 11, 2018
1695b59
Update three more spans to ReadOnly
iamcarbon May 11, 2018
63dc698
Merge branch 'memory-rc1' of https://github.com/carbon/ImageSharp int…
iamcarbon May 11, 2018
93c44f4
React to LoadPixelData change
iamcarbon May 11, 2018
05aa089
Merge branch 'master' into memory-rc1
JimBobSquarePants May 12, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 0 additions & 55 deletions src/ImageSharp/Common/Extensions/ByteExtensions.cs

This file was deleted.

87 changes: 32 additions & 55 deletions src/ImageSharp/Common/Helpers/Guard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

namespace SixLabors.ImageSharp
{
Expand All @@ -15,78 +14,62 @@ namespace SixLabors.ImageSharp
internal static class Guard
{
/// <summary>
/// Verifies, that the method parameter with specified object value is not null
/// and throws an exception if it is found to be so.
/// Ensures that the value is not null.
/// </summary>
/// <param name="target">The target object, which cannot be null.</param>
/// <param name="value">The target object, which cannot be null.</param>
/// <param name="parameterName">The name of the parameter that is to be checked.</param>
/// <param name="message">The error message, if any to add to the exception.</param>
/// <exception cref="ArgumentNullException"><paramref name="target"/> is null</exception>
public static void NotNull(object target, string parameterName, string message = "")
/// <exception cref="ArgumentNullException"><paramref name="value"/> is null</exception>
public static void NotNull(object value, string parameterName)
{
if (target == null)
if (value == null)
{
if (!string.IsNullOrWhiteSpace(message))
{
throw new ArgumentNullException(parameterName, message);
}

throw new ArgumentNullException(parameterName);
}
}

/// <summary>
/// Verifies, that the string method parameter with specified object value and message
/// is not null, not empty and does not contain only blanks and throws an exception
/// if the object is null.
/// Ensures that the target value is not null, empty, or whitespace.
/// </summary>
/// <param name="target">The target string, which should be checked against being null or empty.</param>
/// <param name="value">The target string, which should be checked against being null or empty.</param>
/// <param name="parameterName">Name of the parameter.</param>
/// <param name="message">The error message, if any to add to the exception.</param>
/// <exception cref="ArgumentNullException"><paramref name="target"/> is null.</exception>
/// <exception cref="ArgumentException"><paramref name="target"/> is empty or contains only blanks.</exception>
public static void NotNullOrEmpty(string target, string parameterName, string message = "")
/// <exception cref="ArgumentNullException"><paramref name="value"/> is null.</exception>
/// <exception cref="ArgumentException"><paramref name="value"/> is empty or contains only blanks.</exception>
public static void NotNullOrWhiteSpace(string value, string parameterName)
{
NotNull(target, parameterName, message);

if (string.IsNullOrWhiteSpace(target))
if (value == null)
{
if (!string.IsNullOrWhiteSpace(message))
{
throw new ArgumentException(message, parameterName);
}
throw new ArgumentNullException(parameterName);
}

throw new ArgumentException("Value cannot be null or empty and cannot contain only blanks.", parameterName);
if (string.IsNullOrWhiteSpace(value))
{
throw new ArgumentException("Must not be empty or whitespace.", parameterName);
}
}

/// <summary>
/// Verifies, that the enumeration is not null and not empty.
/// Ensures that the enumeration is not null or empty.
/// </summary>
/// <typeparam name="T">The type of objects in the <paramref name="target"/></typeparam>
/// <param name="target">The target enumeration, which should be checked against being null or empty.</param>
/// <typeparam name="T">The type of objects in the <paramref name="value"/></typeparam>
/// <param name="value">The target enumeration, which should be checked against being null or empty.</param>
/// <param name="parameterName">Name of the parameter.</param>
/// <param name="message">The error message, if any to add to the exception.</param>
/// <exception cref="ArgumentNullException"><paramref name="target"/> is null.</exception>
/// <exception cref="ArgumentException"><paramref name="target"/> is empty.</exception>
public static void NotNullOrEmpty<T>(IEnumerable<T> target, string parameterName, string message = "")
/// <exception cref="ArgumentNullException"><paramref name="value"/> is null.</exception>
/// <exception cref="ArgumentException"><paramref name="value"/> is empty.</exception>
public static void NotNullOrEmpty<T>(ICollection<T> value, string parameterName)
{
NotNull(target, parameterName, message);

if (!target.Any())
if (value == null)
{
if (!string.IsNullOrWhiteSpace(message))
{
throw new ArgumentException(message, parameterName);
}
throw new ArgumentNullException(parameterName);
}

throw new ArgumentException("Value cannot be empty.", parameterName);
if (value.Count == 0)
{
throw new ArgumentException("Must not be empty.", parameterName);
}
}

/// <summary>
/// Verifies that the specified value is less than a maximum value
/// and throws an exception if it is not.
/// Ensures that the specified value is less than a maximum value.
/// </summary>
/// <param name="value">The target value, which should be validated.</param>
/// <param name="max">The maximum value.</param>
Expand Down Expand Up @@ -191,15 +174,9 @@ public static void MustBeBetweenOrEqualTo<TValue>(TValue value, TValue min, TVal
/// Verifies, that the method parameter with specified target value is true
/// and throws an exception if it is found to be so.
/// </summary>
/// <param name="target">
/// The target value, which cannot be false.
/// </param>
/// <param name="parameterName">
/// The name of the parameter that is to be checked.
/// </param>
/// <param name="message">
/// The error message, if any to add to the exception.
/// </param>
/// <param name="target">The target value, which cannot be false.</param>
/// <param name="parameterName">The name of the parameter that is to be checked.</param>
/// <param name="message">The error message, if any to add to the exception.</param>
/// <exception cref="ArgumentException">
/// <paramref name="target"/> is false
/// </exception>
Expand Down
24 changes: 8 additions & 16 deletions src/ImageSharp/Formats/Gif/GifDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ internal sealed class GifDecoderCore
/// </summary>
private IManagedByteBuffer globalColorTable;

/// <summary>
/// The global color table length
/// </summary>
private int globalColorTableLength;

/// <summary>
/// The area to restore.
/// </summary>
Expand Down Expand Up @@ -333,8 +328,8 @@ private void ReadFrame<TPixel>(ref Image<TPixel> image, ref ImageFrame<TPixel> p
indices = this.configuration.MemoryManager.AllocateManagedByteBuffer(imageDescriptor.Width * imageDescriptor.Height, true);

this.ReadFrameIndices(imageDescriptor, indices.Span);
IManagedByteBuffer colorTable = localColorTable ?? this.globalColorTable;
this.ReadFrameColors(ref image, ref previousFrame, indices.Span, colorTable.Span, imageDescriptor);
ReadOnlySpan<Rgb24> colorTable = MemoryMarshal.Cast<byte, Rgb24>((localColorTable ?? this.globalColorTable).Span);
this.ReadFrameColors(ref image, ref previousFrame, indices.Span, colorTable, imageDescriptor);

// Skip any remaining blocks
this.Skip(0);
Expand Down Expand Up @@ -370,7 +365,7 @@ private void ReadFrameIndices(in GifImageDescriptor imageDescriptor, Span<byte>
/// <param name="indices">The indexed pixels.</param>
/// <param name="colorTable">The color table containing the available colors.</param>
/// <param name="descriptor">The <see cref="GifImageDescriptor"/></param>
private void ReadFrameColors<TPixel>(ref Image<TPixel> image, ref ImageFrame<TPixel> previousFrame, Span<byte> indices, Span<byte> colorTable, in GifImageDescriptor descriptor)
private void ReadFrameColors<TPixel>(ref Image<TPixel> image, ref ImageFrame<TPixel> previousFrame, Span<byte> indices, ReadOnlySpan<Rgb24> colorTable, in GifImageDescriptor descriptor)
where TPixel : struct, IPixel<TPixel>
{
ref byte indicesRef = ref MemoryMarshal.GetReference(indices);
Expand Down Expand Up @@ -458,11 +453,8 @@ private void ReadFrameColors<TPixel>(ref Image<TPixel> image, ref ImageFrame<TPi
if (this.graphicsControlExtension.TransparencyFlag == false ||
this.graphicsControlExtension.TransparencyIndex != index)
{
int indexOffset = index * 3;

ref TPixel pixel = ref Unsafe.Add(ref rowRef, x);
rgba.Rgb = colorTable.GetRgb24(indexOffset);

rgba.Rgb = colorTable[index];
pixel.PackFromRgba32(rgba);
}

Expand Down Expand Up @@ -534,12 +526,12 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(Stream stream)

if (this.logicalScreenDescriptor.GlobalColorTableFlag)
{
this.globalColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize * 3;
int globalColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize * 3;

this.globalColorTable = this.MemoryManager.AllocateManagedByteBuffer(this.globalColorTableLength, true);
this.globalColorTable = this.MemoryManager.AllocateManagedByteBuffer(globalColorTableLength, true);

// Read the global color table from the stream
stream.Read(this.globalColorTable.Array, 0, this.globalColorTableLength);
// Read the global color table data from the stream
stream.Read(this.globalColorTable.Array, 0, globalColorTableLength);
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/ImageSharp/Formats/ImageFormatManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ public void AddImageFormat(IImageFormat format)
/// <returns>The <see cref="IImageFormat"/> if found otherwise null</returns>
public IImageFormat FindFormatByFileExtension(string extension)
{
Guard.NotNullOrWhiteSpace(extension, nameof(extension));

if (extension[0] == '.')
Copy link
Member

Choose a reason for hiding this comment

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

Good idea!

{
extension = extension.Substring(1);
}

return this.imageFormats.FirstOrDefault(x => x.FileExtensions.Contains(extension, StringComparer.OrdinalIgnoreCase));
}

Expand Down
16 changes: 6 additions & 10 deletions src/ImageSharp/Formats/Png/PngDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ private void ProcessScanlineFromPalette<TPixel>(ReadOnlySpan<byte> defilteredSca
where TPixel : struct, IPixel<TPixel>
{
ReadOnlySpan<byte> newScanline = ToArrayByBitsLength(defilteredScanline, this.bytesPerScanline, this.header.BitDepth);
byte[] pal = this.palette;
ReadOnlySpan<Rgb24> pal = MemoryMarshal.Cast<byte, Rgb24>(this.palette);
Copy link
Member

Choose a reason for hiding this comment

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

won't this fail if this.palette isn't the correct length? just raising this as a subtle change of behavior that might introduce exceptions that previously (potentially incorrectly) allowed.

note: not asking for a change just pointing it out incase someone else thinks its important.

Copy link
Contributor Author

@iamcarbon iamcarbon May 10, 2018

Choose a reason for hiding this comment

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

This should only throw if the we try indexing into the palette outside it's bounds.

var color = default(TPixel);

var rgba = default(Rgba32);
Expand All @@ -865,10 +865,9 @@ private void ProcessScanlineFromPalette<TPixel>(ReadOnlySpan<byte> defilteredSca
for (int x = 0; x < this.header.Width; x++)
{
int index = newScanline[x];
int pixelOffset = index * 3;

rgba.A = this.paletteAlpha.Length > index ? this.paletteAlpha[index] : (byte)255;
rgba.Rgb = pal.GetRgb24(pixelOffset);
rgba.Rgb = pal[index];

color.PackFromRgba32(rgba);
row[x] = color;
Expand All @@ -881,9 +880,8 @@ private void ProcessScanlineFromPalette<TPixel>(ReadOnlySpan<byte> defilteredSca
for (int x = 0; x < this.header.Width; x++)
{
int index = newScanline[x];
int pixelOffset = index * 3;

rgba.Rgb = pal.GetRgb24(pixelOffset);
rgba.Rgb = pal[index];

color.PackFromRgba32(rgba);
row[x] = color;
Expand Down Expand Up @@ -946,6 +944,7 @@ private void ProcessInterlacedDefilteredScanline<TPixel>(ReadOnlySpan<byte> defi

ReadOnlySpan<byte> newScanline = ToArrayByBitsLength(scanlineBuffer, this.bytesPerScanline, this.header.BitDepth);
var rgba = default(Rgba32);
Span<Rgb24> pal = MemoryMarshal.Cast<byte, Rgb24>(this.palette);

if (this.paletteAlpha != null && this.paletteAlpha.Length > 0)
{
Expand All @@ -954,10 +953,9 @@ private void ProcessInterlacedDefilteredScanline<TPixel>(ReadOnlySpan<byte> defi
for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o++)
{
int index = newScanline[o];
int offset = index * 3;

rgba.A = this.paletteAlpha.Length > index ? this.paletteAlpha[index] : (byte)255;
rgba.Rgb = this.palette.GetRgb24(offset);
rgba.Rgb = pal[index];

color.PackFromRgba32(rgba);
rowSpan[x] = color;
Expand All @@ -970,10 +968,8 @@ private void ProcessInterlacedDefilteredScanline<TPixel>(ReadOnlySpan<byte> defi
for (int x = pixelOffset, o = 0; x < this.header.Width; x += increment, o++)
{
int index = newScanline[o];
int offset = index * 3;

rgba.Rgb = this.palette.GetRgb24(offset);

rgba.Rgb = pal[index];
color.PackFromRgba32(rgba);
rowSpan[x] = color;
}
Expand Down
Loading