-
-
Notifications
You must be signed in to change notification settings - Fork 889
Improve accessability of Span<T> methods. #1172
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1172 +/- ##
=======================================
Coverage 82.39% 82.40%
=======================================
Files 684 684
Lines 29516 29519 +3
Branches 3327 3328 +1
=======================================
+ Hits 24319 24324 +5
+ Misses 4508 4507 -1
+ Partials 689 688 -1
Continue to review full report at Codecov.
|
| if (mg.Count > 1) | ||
| { | ||
| span = default; | ||
| return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antonfirsov Is there a test somewhere I can tap into to test the false condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you can limit MemoryAllocator's BufferCapacity in many ways to ensure the backing memory is fragmented:
- like this (works with
TestMemoryAllocatoronly). - or like this.
antonfirsov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good after adding tests and exception docs.
| if (mg.Count > 1) | ||
| { | ||
| span = default; | ||
| return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you can limit MemoryAllocator's BufferCapacity in many ways to ensure the backing memory is fragmented:
- like this (works with
TestMemoryAllocatoronly). - or like this.
| /// at row <paramref name="rowIndex"/> beginning from the first pixel on that row. | ||
| /// </summary> | ||
| /// <param name="rowIndex">The row.</param> | ||
| /// <returns>The <see cref="Span{TPixel}"/></returns> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs are missing <exception> for ArgumentOutOfRangeException.
| /// </summary> | ||
| /// <param name="rowIndex">The row.</param> | ||
| /// <returns>The <see cref="Span{TPixel}"/></returns> | ||
| public Span<TPixel> GetPixelRowSpan(int rowIndex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous note.
Prerequisites
Description
Fixes #1164