Fix resize pad color application for non Nearest Neighbor samplers #2052
Conversation
| @@ -101,7 +101,7 @@ protected virtual void Dispose(bool disposing) | |||
| /// Returns a <see cref="ResizeKernel"/> for an index value between 0 and DestinationSize - 1. | |||
| /// </summary> | |||
| [MethodImpl(InliningOptions.ShortMethod)] | |||
| internal ref ResizeKernel GetKernel(int destIdx) => ref this.kernels[destIdx]; | |||
| internal ref ResizeKernel GetKernel(nint destIdx) => ref this.kernels[destIdx]; | |||
There was a problem hiding this comment.
For the plain array access nint doesn't bring any advantage.
Here the JIT misses a optimization (a bug is tracked, but can't find it at the moment).
If you want best machine code here, look at Sharplab method GetKernelC.
It's more C# code and I doubt it's needed here. I'd just go with int (or nint as you have it now) and wait for the JIT fix.
There was a problem hiding this comment.
No MemoryMarshal.GetArrayDataReference for us yet unfortunately..... One day!
| ref Vector4 fpBase = ref transposedFirstPassBufferSpan[top]; | ||
|
|
||
| for (int x = 0; x < this.destWidth; x++) | ||
| for (nint x = left; x < right; x++) |
There was a problem hiding this comment.
Here it's good to have nint, as in the Unsafe.Adds below this removes the sign extending move movsxd which is beneficial.
There was a problem hiding this comment.
Would it be better to have the loop as
| for (nint x = left; x < right; x++) | |
| for (nint x = 0; x < (right - left); x++) |
so that the subtractions below x - left can be eliminated (shifted to the loop variable), so that it's just x.
I assume current JIT won't (completely) CSE that expression.
There was a problem hiding this comment.
Good spot. Not sure how I missed that.
| @@ -184,13 +186,13 @@ private void CalculateFirstPassValues(RowInterval calculationInterval) | |||
| // Span<Vector4> firstPassSpan = transposedFirstPassBufferSpan.Slice(y - this.currentWindow.Min); | |||
| ref Vector4 firstPassBaseRef = ref transposedFirstPassBufferSpan[y - this.currentWindow.Min]; | |||
|
|
|||
| for (int x = this.targetWorkingRect.Left; x < this.targetWorkingRect.Right; x++) | |||
| for (nint x = left; x < right; x++) | |||
There was a problem hiding this comment.
Same here:
| for (nint x = left; x < right; x++) | |
| for (nint x = 0; x < (right - left); x++) |
?
And adjust below to just x instead of x - left.
There was a problem hiding this comment.
Ah, I have to add an extra incremental here. I need both x & x - left.
Prerequisites
Description
The new
ResizeOptions.PadColorproperty fill operation was being overwritten by theResizeWorker. This changes fixes the worker to only allocate buffers matching the target rectangle of interest writing them to the correct target span.