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.

Sign up for GitHub

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

Jump to bottom

cleanup projects and environment variables #2812

Merged
merged 8 commits into from
Nov 14, 2024
Merged

cleanup projects and environment variables #2812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
418ce0e
cleanup projects and environment variables
kasperk81 Sep 22, 2024
9cc5ff2
add net9.0
kasperk81 Sep 23, 2024
56b28fc
net9.0
kasperk81 Sep 23, 2024
e6cf1e5
Revert .net9
kasperk81 Oct 1, 2024
5f19369
Merge branch 'main' into refactor
JimBobSquarePants Oct 30, 2024
93299e1
Merge branch 'main' into refactor
JimBobSquarePants Nov 14, 2024
87d6f00
Test against NET 9
JimBobSquarePants Nov 14, 2024
62744c6
FIx build
JimBobSquarePants Nov 14, 2024
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
27 changes: 26 additions & 1 deletion .github/workflows/build-and-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,31 @@ jobs:
isARM:
- ${{ contains(github.event.pull_request.labels.*.name, 'arch:arm32') || contains(github.event.pull_request.labels.*.name, 'arch:arm64') }}
options:
- os: ubuntu-latest
framework: net9.0
sdk: 9.0.x
sdk-preview: true
runtime: -x64
codecov: false
- os: macos-13 # macos-latest runs on arm64 runners where libgdiplus is unavailable
framework: net9.0
sdk: 9.0.x
sdk-preview: true
runtime: -x64
codecov: false
- os: windows-latest
framework: net9.0
sdk: 9.0.x
sdk-preview: true
runtime: -x64
codecov: false
- os: buildjet-4vcpu-ubuntu-2204-arm
framework: net9.0
sdk: 9.0.x
sdk-preview: true
runtime: -x64
codecov: false

- os: ubuntu-latest
framework: net8.0
sdk: 8.0.x
Expand Down Expand Up @@ -100,7 +125,7 @@ jobs:
uses: actions/setup-dotnet@v4
with:
dotnet-version: |
8.0.x
9.0.x

- name: DotNet Build
if: ${{ matrix.options.sdk-preview != true }}
Expand Down
3 changes: 3 additions & 0 deletions src/ImageSharp.ruleset
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="ImageSharp" ToolsVersion="17.0">
<Include Path="..\shared-infrastructure\sixlabors.ruleset" Action="Default" />
<Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp.NetAnalyzers" RuleNamespace="Microsoft.CodeAnalysis.CSharp.NetAnalyzers">
<Rule Id="CA2022" Action="Info" />
Copy link

Choose a reason for hiding this comment

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

For my learning, why was this analyzer downgraded from the default Warning to Info? The issues that this analyzer flags are typically bugs when reading from Streams.

Copy link
Member

Choose a reason for hiding this comment

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

The warnings were in areas of our code where it wasn’t really applicable. Our decoders are designed to handle degenerate images and in each warning instance we had pre-allocated an empty buffer to read to. If we don’t get a full read then those buffers remain partially unfillled and the decoding process will return when we look for the next chunk.

Copy link

Choose a reason for hiding this comment

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

Hmm, I'm wondering if we have a missed use case / bug in our analyzer then. Do you happen to have a link to example code that it flagged? Maybe it just couldn't understand the pattern.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think the analyzer will ever be possible to follow the code well enough here.

Take this for example. In ReadFrame in the GIF decoder we attempt to read a local color palette from a stream. The analyzer will warn us that we might not get the full read though.

ImageSharp/src/ImageSharp/Formats/Gif/GifDecoderCore.cs

Line 432 in 2d7cf48

stream.Read(this.currentLocalColorTable.GetSpan()[..length]);

In this instance we simply do not care as the following method ReadFrameColors reads and checks the next byte. If it's invalid then we do nothing.

ImageSharp/src/ImageSharp/Formats/Gif/GifDecoderCore.cs

Lines 533 to 534 in 2d7cf48

int minCodeSize = stream.ReadByte();
if (LzwDecoder.IsValidMinCodeSize(minCodeSize))

We also do a sense check and break when performing the next iteration to find additional frames so the decoder will quit after decoding as much information as possible.

ImageSharp/src/ImageSharp/Formats/Gif/GifDecoderCore.cs

Lines 155 to 156 in 2d7cf48

nextFlag = stream.ReadByte();
if (nextFlag == -1)

So, our code is safe, we could perhaps break earlier but we would end up complicating the code in order to do so. Info will work in our scenarios as it will prompt us to check the code but not inhibit our ability to work.

Copy link

Choose a reason for hiding this comment

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

Taking a quick look, I believe the reason why the code is safe is because BufferedReadStream guarantees it will fill out the entire passed in buffer on each call to .Read.

ImageSharp/src/ImageSharp/IO/BufferedReadStream.cs

Lines 176 to 195 in 2d7cf48

public override int Read(Span<byte> buffer)
{
this.cancellationToken.ThrowIfCancellationRequested();
// Too big for our buffer. Read directly from the stream.
int count = buffer.Length;
if (count > this.BufferSize)
{
return this.ReadToBufferDirectSlow(buffer);
}
// Too big for remaining buffer but less than entire buffer length
// Copy to buffer then read from there.
if ((uint)this.readBufferIndex > (uint)(this.BufferSize - count))
{
return this.ReadToBufferViaCopySlow(buffer);
}
return this.ReadToBufferViaCopyFast(buffer);
}

ReadToBufferDirectSlow and ReadToBufferViaCopySlow read from the underlying stream in a loop until it fills the buffer or hits the end of the stream.

ImageSharp/src/ImageSharp/IO/BufferedReadStream.cs

Lines 347 to 357 in 2d7cf48

// Read doesn't always guarantee the full returned length so read a byte
// at a time until we get either our count or hit the end of the stream.
int count = buffer.Length;
int n = 0;
int i;
do
{
i = baseStream.Read(buffer[n..count]);
n += i;
}
while (n < count && i > 0);

ImageSharp/src/ImageSharp/IO/BufferedReadStream.cs

Lines 320 to 326 in 2d7cf48

private int ReadToBufferViaCopySlow(Span<byte> buffer)
{
// Refill our buffer then copy.
this.FillReadBuffer();
return this.ReadToBufferViaCopyFast(buffer);
}

ImageSharp/src/ImageSharp/IO/BufferedReadStream.cs

Lines 270 to 288 in 2d7cf48

private void FillReadBuffer()
{
this.cancellationToken.ThrowIfCancellationRequested();
Stream baseStream = this.BaseStream;
if (this.readerPosition != baseStream.Position)
{
baseStream.Seek(this.readerPosition, SeekOrigin.Begin);
}
// Read doesn't always guarantee the full returned length so read a byte
// at a time until we get either our count or hit the end of the stream.
int n = 0;
int i;
do
{
i = baseStream.Read(this.readBuffer, n, this.BufferSize - n);
n += i;
}
while (n < this.BufferSize && i > 0);

</Rules>
</RuleSet>
3 changes: 1 addition & 2 deletions src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
using SixLabors.ImageSharp.Metadata.Profiles.Icc;
using SixLabors.ImageSharp.Metadata.Profiles.Iptc;
using SixLabors.ImageSharp.Metadata.Profiles.Xmp;
using SixLabors.ImageSharp.PixelFormats;

namespace SixLabors.ImageSharp.Formats.Jpeg;

Expand Down Expand Up @@ -1473,7 +1472,7 @@ private void ProcessStartOfScanMarker(BufferedReadStream stream, int remaining)

this.Frame.ComponentOrder[i / 2] = (byte)componentIndex;

IJpegComponent component = this.Frame.Components[componentIndex];
JpegComponent component = this.Frame.Components[componentIndex];

// 1 byte: Huffman table selectors.
// 4 bits - dc
Expand Down
20 changes: 10 additions & 10 deletions src/ImageSharp/Formats/Webp/Lossy/Vp8Encoding.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,12 +667,12 @@ public static void EncPredChroma8(Span<byte> dst, Span<byte> left, Span<byte> to

// V block.
dst = dst[8..];
if (top != default)
if (!top.IsEmpty)
{
top = top[8..];
}

if (left != default)
if (!left.IsEmpty)
{
left = left[16..];
}
Expand Down Expand Up @@ -701,7 +701,7 @@ public static void EncPredLuma4(Span<byte> dst, Span<byte> top, int topOffset, S

private static void VerticalPred(Span<byte> dst, Span<byte> top, int size)
{
if (top != default)
if (!top.IsEmpty)
{
for (int j = 0; j < size; j++)
{
Expand All @@ -716,7 +716,7 @@ private static void VerticalPred(Span<byte> dst, Span<byte> top, int size)

public static void HorizontalPred(Span<byte> dst, Span<byte> left, int size)
{
if (left != default)
if (!left.IsEmpty)
{
left = left[1..]; // in the reference implementation, left starts at - 1.
for (int j = 0; j < size; j++)
Expand All @@ -732,9 +732,9 @@ public static void HorizontalPred(Span<byte> dst, Span<byte> left, int size)

public static void TrueMotion(Span<byte> dst, Span<byte> left, Span<byte> top, int size)
{
if (left != default)
if (!left.IsEmpty)
{
if (top != default)
if (!top.IsEmpty)
{
Span<byte> clip = Clip1.AsSpan(255 - left[0]); // left [0] instead of left[-1], original left starts at -1
for (int y = 0; y < size; y++)
Expand All @@ -759,7 +759,7 @@ public static void TrueMotion(Span<byte> dst, Span<byte> left, Span<byte> top, i
// is equivalent to VE prediction where you just copy the top samples.
// Note that if top samples are not available, the default value is
// then 129, and not 127 as in the VerticalPred case.
if (top != default)
if (!top.IsEmpty)
{
VerticalPred(dst, top, size);
}
Expand All @@ -774,14 +774,14 @@ private static void DcMode(Span<byte> dst, Span<byte> left, Span<byte> top, int
{
int dc = 0;
int j;
if (top != default)
if (!top.IsEmpty)
{
for (j = 0; j < size; j++)
{
dc += top[j];
}

if (left != default)
if (!left.IsEmpty)
{
// top and left present.
left = left[1..]; // in the reference implementation, left starts at -1.
Expand All @@ -798,7 +798,7 @@ private static void DcMode(Span<byte> dst, Span<byte> left, Span<byte> top, int

dc = (dc + round) >> shift;
}
else if (left != default)
else if (!left.IsEmpty)
{
// left but no top.
left = left[1..]; // in the reference implementation, left starts at -1.
Expand Down
16 changes: 8 additions & 8 deletions src/ImageSharp/Formats/Webp/Lossy/YuvConversion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private static void UpSampleScalar(Span<byte> topY, Span<byte> bottomY, Span<byt
uint uv0 = ((3 * tluv) + luv + 0x00020002u) >> 2;
YuvToBgr(topY[0], (int)(uv0 & 0xff), (int)(uv0 >> 16), topDst);

if (bottomY != default)
if (!bottomY.IsEmpty)
{
uv0 = ((3 * luv) + tluv + 0x00020002u) >> 2;
YuvToBgr(bottomY[0], (int)uv0 & 0xff, (int)(uv0 >> 16), bottomDst);
Expand All @@ -69,7 +69,7 @@ private static void UpSampleScalar(Span<byte> topY, Span<byte> bottomY, Span<byt
YuvToBgr(topY[xMul2 - 1], (int)(uv0 & 0xff), (int)(uv0 >> 16), topDst[((xMul2 - 1) * xStep)..]);
YuvToBgr(topY[xMul2 - 0], (int)(uv1 & 0xff), (int)(uv1 >> 16), topDst[((xMul2 - 0) * xStep)..]);

if (bottomY != default)
if (!bottomY.IsEmpty)
{
uv0 = (diag03 + luv) >> 1;
uv1 = (diag12 + uv) >> 1;
Expand All @@ -85,7 +85,7 @@ private static void UpSampleScalar(Span<byte> topY, Span<byte> bottomY, Span<byt
{
uv0 = ((3 * tluv) + luv + 0x00020002u) >> 2;
YuvToBgr(topY[len - 1], (int)(uv0 & 0xff), (int)(uv0 >> 16), topDst[((len - 1) * xStep)..]);
if (bottomY != default)
if (!bottomY.IsEmpty)
{
uv0 = ((3 * luv) + tluv + 0x00020002u) >> 2;
YuvToBgr(bottomY[len - 1], (int)(uv0 & 0xff), (int)(uv0 >> 16), bottomDst[((len - 1) * xStep)..]);
Expand Down Expand Up @@ -120,7 +120,7 @@ private static void UpSampleSse41(Span<byte> topY, Span<byte> bottomY, Span<byte
int u0t = (topU[0] + uDiag) >> 1;
int v0t = (topV[0] + vDiag) >> 1;
YuvToBgr(topY[0], u0t, v0t, topDst);
if (bottomY != default)
if (!bottomY.IsEmpty)
{
int u0b = (curU[0] + uDiag) >> 1;
int v0b = (curV[0] + vDiag) >> 1;
Expand All @@ -134,7 +134,7 @@ private static void UpSampleSse41(Span<byte> topY, Span<byte> bottomY, Span<byte
ref byte topVRef = ref MemoryMarshal.GetReference(topV);
ref byte curURef = ref MemoryMarshal.GetReference(curU);
ref byte curVRef = ref MemoryMarshal.GetReference(curV);
if (bottomY != default)
if (!bottomY.IsEmpty)
{
for (pos = 1, uvPos = 0; pos + 32 + 1 <= len; pos += 32, uvPos += 16)
{
Expand All @@ -160,12 +160,12 @@ private static void UpSampleSse41(Span<byte> topY, Span<byte> bottomY, Span<byte
Span<byte> tmpTopDst = ru[(4 * 32)..];
Span<byte> tmpBottomDst = tmpTopDst[(4 * 32)..];
Span<byte> tmpTop = tmpBottomDst[(4 * 32)..];
Span<byte> tmpBottom = (bottomY == default) ? null : tmpTop[32..];
Span<byte> tmpBottom = bottomY.IsEmpty ? null : tmpTop[32..];
UpSampleLastBlock(topU[uvPos..], curU[uvPos..], leftOver, ru);
UpSampleLastBlock(topV[uvPos..], curV[uvPos..], leftOver, rv);

topY[pos..len].CopyTo(tmpTop);
if (bottomY != default)
if (!bottomY.IsEmpty)
{
bottomY[pos..len].CopyTo(tmpBottom);
ConvertYuvToBgrWithBottomYSse41(tmpTop, tmpBottom, tmpTopDst, tmpBottomDst, ru, rv, 0, xStep);
Expand All @@ -176,7 +176,7 @@ private static void UpSampleSse41(Span<byte> topY, Span<byte> bottomY, Span<byte
}

tmpTopDst[..((len - pos) * xStep)].CopyTo(topDst[(pos * xStep)..]);
if (bottomY != default)
if (!bottomY.IsEmpty)
{
tmpBottomDst[..((len - pos) * xStep)].CopyTo(bottomDst[(pos * xStep)..]);
}
Expand Down
5 changes: 2 additions & 3 deletions src/ImageSharp/ImageSharp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<PackageTags>Image Resize Crop Gif Jpg Jpeg Bitmap Pbm Png Tga Tiff WebP NetCore</PackageTags>
<Description>A new, fully featured, fully managed, cross-platform, 2D graphics API for .NET</Description>
<Configurations>Debug;Release</Configurations>
<IsTrimmable>true</IsTrimmable>
</PropertyGroup>

<!-- This enables the nullable analysis and treats all nullable warnings as error-->
Expand All @@ -29,14 +30,12 @@
<Choose>
<When Condition="$(SIXLABORS_TESTING_PREVIEW) == true">
<PropertyGroup>
<TargetFrameworks>net8.0</TargetFrameworks>
<IsTrimmable>true</IsTrimmable>
<TargetFrameworks>net8.0;net9.0</TargetFrameworks>
</PropertyGroup>
</When>
<Otherwise>
<PropertyGroup>
<TargetFrameworks>net8.0</TargetFrameworks>
<IsTrimmable>true</IsTrimmable>
</PropertyGroup>
</Otherwise>
</Choose>
Expand Down
4 changes: 2 additions & 2 deletions src/ImageSharp/Image{TPixel}.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ internal Image(Configuration configuration, ImageMetadata metadata, IEnumerable<
/// <summary>
/// Gets the root frame.
/// </summary>
private IPixelSource<TPixel> PixelSourceUnsafe => this.frames.RootFrameUnsafe;
private ImageFrame<TPixel> PixelSourceUnsafe => this.frames.RootFrameUnsafe;

/// <summary>
/// Gets or sets the pixel at the specified position.
Expand Down Expand Up @@ -324,7 +324,7 @@ public bool DangerousTryGetSinglePixelMemory(out Memory<TPixel> memory)
}

/// <summary>
/// Clones the current image
/// Clones the current image.
/// </summary>
/// <returns>Returns a new image with all the same metadata as the original.</returns>
public Image<TPixel> Clone() => this.Clone(this.Configuration);
Expand Down
2 changes: 1 addition & 1 deletion src/ImageSharp/Metadata/Profiles/Exif/ExifWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ private static bool HasValue(IExifValue exifValue)
return true;
}

private static uint GetLength(IList<IExifValue> values)
private static uint GetLength(List<IExifValue> values)
{
if (values.Count == 0)
{
Expand Down
2 changes: 1 addition & 1 deletion src/ImageSharp/Metadata/Profiles/ICC/DataReader/IccDataReader.TagDataEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public IccChromaticityTagDataEntry ReadChromaticityTagDataEntry()
ushort channelCount = this.ReadUInt16();
var colorant = (IccColorantEncoding)this.ReadUInt16();

if (Enum.IsDefined(typeof(IccColorantEncoding), colorant) && colorant != IccColorantEncoding.Unknown)
if (Enum.IsDefined(colorant) && colorant != IccColorantEncoding.Unknown)
{
// The type is known and so are the values (they are constant)
// channelCount should always be 3 but it doesn't really matter if it's not
Expand Down
6 changes: 3 additions & 3 deletions src/ImageSharp/Metadata/Profiles/ICC/IccProfile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ public bool CheckIsValid()
}

return arrayValid &&
Enum.IsDefined(typeof(IccColorSpaceType), this.Header.DataColorSpace) &&
Enum.IsDefined(typeof(IccColorSpaceType), this.Header.ProfileConnectionSpace) &&
Enum.IsDefined(typeof(IccRenderingIntent), this.Header.RenderingIntent) &&
Enum.IsDefined(this.Header.DataColorSpace) &&
Enum.IsDefined(this.Header.ProfileConnectionSpace) &&
Enum.IsDefined(this.Header.RenderingIntent) &&
this.Header.Size is >= minSize and < maxSize;
}

Expand Down
6 changes: 3 additions & 3 deletions src/ImageSharp/Processing/Processors/Convolution/Parameters/BokehBlurKernelDataProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ internal static class BokehBlurKernelDataProvider
/// <summary>
/// Gets the kernel scales to adjust the component values in each kernel
/// </summary>
private static IReadOnlyList<float> KernelScales { get; } = new[] { 1.4f, 1.2f, 1.2f, 1.2f, 1.2f, 1.2f };
private static float[] KernelScales { get; } = new[] { 1.4f, 1.2f, 1.2f, 1.2f, 1.2f, 1.2f };

/// <summary>
/// Gets the available bokeh blur kernel parameters
/// </summary>
private static IReadOnlyList<Vector4[]> KernelComponents { get; } = new[]
private static Vector4[][] KernelComponents { get; } = new[]
{
// 1 component
new[] { new Vector4(0.862325f, 1.624835f, 0.767583f, 1.862321f) },
Expand Down Expand Up @@ -112,7 +112,7 @@ public static BokehBlurKernelData GetBokehBlurKernelData(
private static (Vector4[] Parameters, float Scale) GetParameters(int componentsCount)
{
// Prepare the kernel components
int index = Math.Max(0, Math.Min(componentsCount - 1, KernelComponents.Count));
int index = Math.Max(0, Math.Min(componentsCount - 1, KernelComponents.Length));

return (KernelComponents[index], KernelScales[index]);
}
Expand Down
Loading
Loading