Expose Buffer2D and ParallelHelper internals#1035
Conversation
antonfirsov
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1035 +/- ##
==========================================
- Coverage 89.87% 89.76% -0.11%
==========================================
Files 1103 1104 +1
Lines 48896 48922 +26
Branches 3442 3444 +2
==========================================
- Hits 43944 43915 -29
- Misses 4249 4253 +4
- Partials 703 754 +51
Continue to review full report at Codecov.
|
JimBobSquarePants
left a comment
JimBobSquarePants
left a comment
There was a problem hiding this comment.
All looks good to me!
| { | ||
| // Shall be compatible with ParallelOptions.MaxDegreeOfParallelism: | ||
| // https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.paralleloptions.maxdegreeofparallelism | ||
| if (maxDegreeOfParallelism == 0 || maxDegreeOfParallelism < -1) |
| /// instantiating a temporary buffer for each <paramref name="body"/> invocation. | ||
| /// </summary> | ||
| public static void IterateRowsWithTempBuffer<T>( | ||
| internal static void IterateRowsWithTempBuffer<T>( |
There was a problem hiding this comment.
I'm assuming this is internal because Drawing does not use it?
There was a problem hiding this comment.
Yes. I'm not fully happy with this API yet, so I'd keep it internal for now.
| /// </remarks> | ||
| /// <typeparam name="T">The value type.</typeparam> | ||
| internal sealed class Buffer2D<T> : IDisposable | ||
| // TODO: Consider moving this type to the SixLabors.Memory namespace (SixLabors.Core). |
There was a problem hiding this comment.
That's probably a useful consideration.
There was a problem hiding this comment.
Not 100% sure yet, depends on future investigations.
src/ImageSharp/Memory/Buffer2D{T}.cs
Outdated
| /// <summary> | ||
| /// Gets the height. | ||
| /// </summary> | ||
| /// </summary>Bu |
There was a problem hiding this comment.
Surprised this didn't break compilation!
| public void FillDestinationPixels(RowInterval rowInterval, Buffer2D<TPixel> destination) | ||
| { | ||
| Span<Vector4> tempColSpan = this.tempColumnBuffer.GetSpan(); | ||
| Span<Vector4> transposedFirstPassBufferSpan = this.transposedFirstPassBuffer.GetSpan(); |
There was a problem hiding this comment.
Good optimization spot.
|
@JimBobSquarePants wow that was quick! Does it mean that you are happy with my namespace choice |
|
Yeah, happy with that, certainly advanced and Utils is a common suffix. |
…ernals Expose Buffer2D and ParallelHelper internals
2 participants
Prerequisites
Description
Sister PR of #1028. Fix more points of #967.
API-s being exposed:
Buffer2D<T>andBuffer2DExtensions: minimum API surface needed by DrawingParallelHelper: andParallelExecutionSettings:IterateRowsWithTempBufferSixLabors.ImageSharp.Advanced.ParallelUtilsAdditional:
Fixed error cases for
Configuration.MaxDegreeOfParallelismandParallelExecutionSettings.MaxDegreeOfParallelism.