Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using SixLabors.ImageSharp.Advanced;
using SixLabors.ImageSharp.ColorProfiles.Companding;
using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Processing.Processors.Convolution.Parameters;
Expand Down Expand Up @@ -112,7 +113,7 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
}
else
{
ApplyInverseGammaExposureRowOperation operation = new(sourceRectangle, source.PixelBuffer, processingBuffer, this.Configuration, 1 / this.gamma);
ApplyInverseGammaExposureRowOperation operation = new(sourceRectangle, source.PixelBuffer, processingBuffer, this.Configuration, this.gamma);
ParallelRowIterator.IterateRows(
this.Configuration,
sourceRectangle,
Expand Down Expand Up @@ -305,15 +306,9 @@ public void Invoke(int y, Span<Vector4> span)
{
Span<TPixel> targetRowSpan = this.targetPixels.DangerousGetRowSpan(y)[this.bounds.X..];
PixelOperations<TPixel>.Instance.ToVector4(this.configuration, targetRowSpan[..span.Length], span, PixelConversionModifiers.Premultiply);
ref Vector4 baseRef = ref MemoryMarshal.GetReference(span);

for (int x = 0; x < this.bounds.Width; x++)
{
ref Vector4 v = ref Unsafe.Add(ref baseRef, (uint)x);
v.X = MathF.Pow(v.X, this.gamma);
v.Y = MathF.Pow(v.Y, this.gamma);
v.Z = MathF.Pow(v.Z, this.gamma);
}
// Input is premultiplied [0,1] so the LUT is safe here.
GammaCompanding.Expand(span[..this.bounds.Width], this.gamma);

PixelOperations<TPixel>.Instance.FromVector4Destructive(this.configuration, span, targetRowSpan);
}
Expand Down Expand Up @@ -367,44 +362,34 @@ public void Invoke(int y, Span<Vector4> span)
private readonly Buffer2D<TPixel> targetPixels;
private readonly Buffer2D<Vector4> sourceValues;
private readonly Configuration configuration;
private readonly float inverseGamma;
private readonly float gamma;

[MethodImpl(InliningOptions.ShortMethod)]
public ApplyInverseGammaExposureRowOperation(
Rectangle bounds,
Buffer2D<TPixel> targetPixels,
Buffer2D<Vector4> sourceValues,
Configuration configuration,
float inverseGamma)
float gamma)
{
this.bounds = bounds;
this.targetPixels = targetPixels;
this.sourceValues = sourceValues;
this.configuration = configuration;
this.inverseGamma = inverseGamma;
this.gamma = gamma;
}

/// <inheritdoc/>
[MethodImpl(InliningOptions.ShortMethod)]
public void Invoke(int y)
{
Vector4 low = Vector4.Zero;
Vector4 high = new(float.PositiveInfinity, float.PositiveInfinity, float.PositiveInfinity, float.PositiveInfinity);

Span<TPixel> targetPixelSpan = this.targetPixels.DangerousGetRowSpan(y)[this.bounds.X..];
Span<Vector4> sourceRowSpan = this.sourceValues.DangerousGetRowSpan(y)[this.bounds.X..];
ref Vector4 sourceRef = ref MemoryMarshal.GetReference(sourceRowSpan);
Span<Vector4> sourceRowSpan = this.sourceValues.DangerousGetRowSpan(y).Slice(this.bounds.X, this.bounds.Width);

for (int x = 0; x < this.bounds.Width; x++)
{
ref Vector4 v = ref Unsafe.Add(ref sourceRef, (uint)x);
Vector4 clamp = Numerics.Clamp(v, low, high);
v.X = MathF.Pow(clamp.X, this.inverseGamma);
v.Y = MathF.Pow(clamp.Y, this.inverseGamma);
v.Z = MathF.Pow(clamp.Z, this.inverseGamma);
}
Numerics.Clamp(MemoryMarshal.Cast<Vector4, float>(sourceRowSpan), 0, 1F);
GammaCompanding.Compress(sourceRowSpan, this.gamma);

PixelOperations<TPixel>.Instance.FromVector4Destructive(this.configuration, sourceRowSpan[..this.bounds.Width], targetPixelSpan, PixelConversionModifiers.Premultiply);
PixelOperations<TPixel>.Instance.FromVector4Destructive(this.configuration, sourceRowSpan, targetPixelSpan, PixelConversionModifiers.Premultiply);
}
}

Expand Down Expand Up @@ -433,17 +418,16 @@ public ApplyInverseGamma3ExposureRowOperation(

/// <inheritdoc/>
[MethodImpl(InliningOptions.ShortMethod)]
public unsafe void Invoke(int y)
public void Invoke(int y)
{
Span<Vector4> sourceRowSpan = this.sourceValues.DangerousGetRowSpan(y).Slice(this.bounds.X, this.bounds.Width);
ref Vector4 sourceRef = ref MemoryMarshal.GetReference(sourceRowSpan);

Numerics.Clamp(MemoryMarshal.Cast<Vector4, float>(sourceRowSpan), 0, float.PositiveInfinity);
Numerics.Clamp(MemoryMarshal.Cast<Vector4, float>(sourceRowSpan), 0, 1F);
Numerics.CubeRootOnXYZ(sourceRowSpan);

Span<TPixel> targetPixelSpan = this.targetPixels.DangerousGetRowSpan(y)[this.bounds.X..];

PixelOperations<TPixel>.Instance.FromVector4Destructive(this.configuration, sourceRowSpan[..this.bounds.Width], targetPixelSpan, PixelConversionModifiers.Premultiply);
PixelOperations<TPixel>.Instance.FromVector4Destructive(this.configuration, sourceRowSpan, targetPixelSpan, PixelConversionModifiers.Premultiply);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

using System.Buffers;
using System.Numerics;
using SixLabors.ImageSharp.Advanced;
using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.PixelFormats;

Expand Down Expand Up @@ -79,10 +79,16 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
this.Configuration,
this.PreserveAlpha);

ParallelRowIterator.IterateRows<Convolution2DRowOperation<TPixel>, Vector4>(
this.Configuration,
interest,
in operation);
// Convolution is memory-bandwidth-bound with low arithmetic intensity.
// Parallelization degrades performance due to cache line contention from
// overlapping source row reads. See #3111.
using IMemoryOwner<Vector4> buffer = allocator.Allocate<Vector4>(operation.GetRequiredBufferLength(interest));
Span<Vector4> span = buffer.Memory.Span;

for (int y = interest.Top; y < interest.Bottom; y++)
{
operation.Invoke(y, span);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any reason for Convolution2DRowOperation isolation? IMO removing it/moving it to a private method would simplify code.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

None other than I want to make minimal changes.

}
}

Buffer2D<TPixel>.SwapOrCopyContent(source.PixelBuffer, targetPixels);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

using System.Buffers;
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -106,6 +107,12 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)

mapXY.BuildSamplingOffsetMap(this.KernelX.Length, this.KernelX.Length, interest, this.BorderWrapModeX, this.BorderWrapModeY);

MemoryAllocator allocator = this.Configuration.MemoryAllocator;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
MemoryAllocator allocator = this.Configuration.MemoryAllocator;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to enable warnings that help detecting unused locals or dead code?

Copy link
Copy Markdown
Member Author

@JimBobSquarePants JimBobSquarePants Apr 16, 2026

Choose a reason for hiding this comment

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

It was weird I missed that. I'll see what I can enable. Fixed anyway. I didn't use your suggestion as there was an addition rogue using to remove.

// Convolution is memory-bandwidth-bound with low arithmetic intensity.
// Parallelization degrades performance due to cache line contention from
// overlapping source row reads. See #3111.

// Horizontal convolution
HorizontalConvolutionRowOperation horizontalOperation = new(
interest,
Expand All @@ -116,10 +123,15 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
this.Configuration,
this.PreserveAlpha);

ParallelRowIterator.IterateRows<HorizontalConvolutionRowOperation, Vector4>(
this.Configuration,
interest,
in horizontalOperation);
using (IMemoryOwner<Vector4> hBuffer = allocator.Allocate<Vector4>(horizontalOperation.GetRequiredBufferLength(interest)))
{
Span<Vector4> hSpan = hBuffer.Memory.Span;

for (int y = interest.Top; y < interest.Bottom; y++)
{
horizontalOperation.Invoke(y, hSpan);
}
}

// Vertical convolution
VerticalConvolutionRowOperation verticalOperation = new(
Expand All @@ -131,10 +143,15 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
this.Configuration,
this.PreserveAlpha);

ParallelRowIterator.IterateRows<VerticalConvolutionRowOperation, Vector4>(
this.Configuration,
interest,
in verticalOperation);
using (IMemoryOwner<Vector4> vBuffer = allocator.Allocate<Vector4>(verticalOperation.GetRequiredBufferLength(interest)))
Comment thread
antonfirsov marked this conversation as resolved.
Outdated
{
Span<Vector4> vSpan = vBuffer.Memory.Span;

for (int y = interest.Top; y < interest.Bottom; y++)
{
verticalOperation.Invoke(y, vSpan);
}
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

using System.Buffers;
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -96,10 +97,17 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
map.BuildSamplingOffsetMap(this.KernelXY.Rows, this.KernelXY.Columns, interest, this.BorderWrapModeX, this.BorderWrapModeY);

RowOperation operation = new(interest, targetPixels, source.PixelBuffer, map, this.KernelXY, this.Configuration, this.PreserveAlpha);
ParallelRowIterator.IterateRows<RowOperation, Vector4>(
this.Configuration,
interest,
in operation);

// Convolution is memory-bandwidth-bound with low arithmetic intensity.
// Parallelization degrades performance due to cache line contention from
// overlapping source row reads. See #3111.
Comment thread
antonfirsov marked this conversation as resolved.
Outdated
using IMemoryOwner<Vector4> buffer = allocator.Allocate<Vector4>(operation.GetRequiredBufferLength(interest));
Span<Vector4> span = buffer.Memory.Span;

for (int y = interest.Top; y < interest.Bottom; y++)
{
operation.Invoke(y, span);
}
}

Buffer2D<TPixel>.SwapOrCopyContent(source.PixelBuffer, targetPixels);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,15 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
processor.Apply(pass);
}

// Convolution is memory-bandwidth-bound with low arithmetic intensity.
// Parallelization degrades performance due to cache line contention from
// overlapping source row reads. See #3111.
RowOperation operation = new(source.PixelBuffer, pass.PixelBuffer, interest);
ParallelRowIterator.IterateRows(
this.Configuration,
interest,
in operation);

for (int y = interest.Top; y < interest.Bottom; y++)
{
operation.Invoke(y);
}
Comment thread
antonfirsov marked this conversation as resolved.
Outdated
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,14 @@ protected override void OnFrameApply(ImageFrame<TPixel> source, ImageFrame<TPixe

Rectangle bounds = this.cropRectangle;

// Copying is cheap, we should process more pixels per task:
ParallelExecutionSettings parallelSettings =
ParallelExecutionSettings.FromConfiguration(this.Configuration).MultiplyMinimumPixelsPerTask(4);

// Copying is too cheap to benefit from parallelization;
// the overhead exceeds the work per task. See #3111.
RowOperation operation = new(bounds, source.PixelBuffer, destination.PixelBuffer);

ParallelRowIterator.IterateRows(
bounds,
in parallelSettings,
in operation);
for (int y = bounds.Top; y < bounds.Bottom; y++)
{
operation.Invoke(y);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The operation type is clearly unneeded here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I just wanted to do the minimal amount of work required to fix the issues for now.

}
}

/// <summary>
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading