Skip to content

Commit 842e2ff

Browse files
author
msftbot[bot]
authored
Fixed ReadOnlySpan2D<T>.Slice parameters order (#3951)
## Fixup for #3353 <!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. --> <!-- Add a brief overview here of the feature/bug & fix. --> ## PR Type What kind of change does this PR introduce? <!-- Please uncomment one or more that apply to this PR. --> - Bugfix <!-- - Feature --> <!-- - Code style update (formatting) --> <!-- - Refactoring (no functional changes, no api changes) --> <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## Overview <!-- Please describe the current behavior that you are modifying, or link to a relevant issue. --> `ReadOnlySpan2D<T>.Slice` has the incorrect ordering for the `height` and `width` parameters, which is inverted with respect to those in `Span2D<T>.Slice`. This is an oversight from a refactor in the original PR, and as a result it also causes the `ReadOnlySpan2D<T>.this[Range, Range]` indexer to fail for `ReadOnlySpan2D<T>`. This PR fixes both issues and adds a new series of tests for the two indexers as well. We probably didn't notice this before since those indexers are not available on UWP 🤔 > **NOTE:** this PR technically introduces a breaking change (swapped parameters in the `.Slice` method, but as that's an issue and not intended, and also because they're in the right order in `Span2D<T>`, it's more of a fix than an actual breaking change. ## PR Checklist Please check if your PR fulfills the following requirements: - [X] Tested code with current [supported SDKs](../readme.md#supported) - [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->~~ - [ ] ~~Sample in sample app has been added / updated (for bug fixes / features)~~ - [ ] ~~Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~ - [X] New major technical changes in the toolkit have or will be added to the [Wiki](https://github.com/windows-toolkit/WindowsCommunityToolkit/wiki) e.g. build changes, source generators, testing infrastructure, sample creation changes, etc... - [X] Tests for the changes have been added (for bug fixes / features) (if applicable) - [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [ ] Contains **NO** breaking changes
2 parents c31e049 + 6e40cf8 commit 842e2ff

File tree

3 files changed

+177
-11
lines changed

3 files changed

+177
-11
lines changed

‎Microsoft.Toolkit.HighPerformance/Memory/ReadOnlySpan2D{T}.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -803,15 +803,15 @@ public ref T DangerousGetReferenceAt(int i, int j)
803803
/// </summary>
804804
/// <param name="row">The target row to map within the current instance.</param>
805805
/// <param name="column">The target column to map within the current instance.</param>
806-
/// <param name="width">The width to map within the current instance.</param>
807806
/// <param name="height">The height to map within the current instance.</param>
807+
/// <param name="width">The width to map within the current instance.</param>
808808
/// <exception cref="ArgumentException">
809809
/// Thrown when either <paramref name="height"/>, <paramref name="width"/> or <paramref name="height"/>
810810
/// are negative or not within the bounds that are valid for the current instance.
811811
/// </exception>
812812
/// <returns>A new <see cref="ReadOnlySpan2D{T}"/> instance representing a slice of the current one.</returns>
813813
[Pure]
814-
public ReadOnlySpan2D<T> Slice(int row, int column, int width, int height)
814+
public ReadOnlySpan2D<T> Slice(int row, int column, int height, int width)
815815
{
816816
if ((uint)row >= Height)
817817
{
@@ -823,14 +823,14 @@ public ReadOnlySpan2D<T> Slice(int row, int column, int width, int height)
823823
ThrowHelper.ThrowArgumentOutOfRangeExceptionForColumn();
824824
}
825825

826-
if ((uint)width > (this.width - column))
826+
if ((uint)height > (Height - row))
827827
{
828-
ThrowHelper.ThrowArgumentOutOfRangeExceptionForWidth();
828+
ThrowHelper.ThrowArgumentOutOfRangeExceptionForHeight();
829829
}
830830

831-
if ((uint)height > (Height - row))
831+
if ((uint)width > (this.width - column))
832832
{
833-
ThrowHelper.ThrowArgumentOutOfRangeExceptionForHeight();
833+
ThrowHelper.ThrowArgumentOutOfRangeExceptionForWidth();
834834
}
835835

836836
nint shift = ((nint)(uint)this.stride * (nint)(uint)row) + (nint)(uint)column;

‎UnitTests/UnitTests.HighPerformance.Shared/Memory/Test_ReadOnlySpan2D{T}.cs

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,89 @@ ref Unsafe.AsRef<int>(null),
400400
Assert.IsTrue(Unsafe.AreSame(ref r0, ref array[0, 0]));
401401
}
402402

403+
#if NETCOREAPP3_1_OR_GREATER
404+
[TestCategory("Span2DT")]
405+
[TestMethod]
406+
public unsafe void Test_ReadOnlySpan2DT_Index_Indexer_1()
407+
{
408+
int[,] array = new int[4, 4];
409+
410+
ReadOnlySpan2D<int> span2d = new ReadOnlySpan2D<int>(array);
411+
412+
ref int arrayRef = ref array[1, 3];
413+
ref readonly int span2dRef = ref span2d[1, ^1];
414+
415+
Assert.IsTrue(Unsafe.AreSame(ref arrayRef, ref Unsafe.AsRef(in span2dRef)));
416+
}
417+
418+
[TestCategory("Span2DT")]
419+
[TestMethod]
420+
public unsafe void Test_ReadOnlySpan2DT_Index_Indexer_2()
421+
{
422+
int[,] array = new int[4, 4];
423+
424+
ReadOnlySpan2D<int> span2d = new ReadOnlySpan2D<int>(array);
425+
426+
ref int arrayRef = ref array[2, 1];
427+
ref readonly int span2dRef = ref span2d[^2, ^3];
428+
429+
Assert.IsTrue(Unsafe.AreSame(ref arrayRef, ref Unsafe.AsRef(in span2dRef)));
430+
}
431+
432+
[TestCategory("Span2DT")]
433+
[TestMethod]
434+
[ExpectedException(typeof(IndexOutOfRangeException))]
435+
public unsafe void Test_ReadOnlySpan2DT_Index_Indexer_Fail()
436+
{
437+
int[,] array = new int[4, 4];
438+
439+
ReadOnlySpan2D<int> span2d = new ReadOnlySpan2D<int>(array);
440+
441+
ref readonly int span2dRef = ref span2d[^6, 2];
442+
}
443+
444+
[TestCategory("Span2DT")]
445+
[TestMethod]
446+
public unsafe void Test_ReadOnlySpan2DT_Range_Indexer_1()
447+
{
448+
int[,] array = new int[4, 4];
449+
450+
ReadOnlySpan2D<int> span2d = new ReadOnlySpan2D<int>(array);
451+
ReadOnlySpan2D<int> slice = span2d[1.., 1..];
452+
453+
Assert.AreEqual(slice.Length, 9);
454+
Assert.IsTrue(Unsafe.AreSame(ref array[1, 1], ref Unsafe.AsRef(in slice[0, 0])));
455+
Assert.IsTrue(Unsafe.AreSame(ref array[3, 3], ref Unsafe.AsRef(in slice[2, 2])));
456+
}
457+
458+
[TestCategory("Span2DT")]
459+
[TestMethod]
460+
public unsafe void Test_ReadOnlySpan2DT_Range_Indexer_2()
461+
{
462+
int[,] array = new int[4, 4];
463+
464+
ReadOnlySpan2D<int> span2d = new ReadOnlySpan2D<int>(array);
465+
ReadOnlySpan2D<int> slice = span2d[0..^2, 1..^1];
466+
467+
Assert.AreEqual(slice.Length, 4);
468+
Assert.IsTrue(Unsafe.AreSame(ref array[0, 1], ref Unsafe.AsRef(in slice[0, 0])));
469+
Assert.IsTrue(Unsafe.AreSame(ref array[1, 2], ref Unsafe.AsRef(in slice[1, 1])));
470+
}
471+
472+
[TestCategory("Span2DT")]
473+
[TestMethod]
474+
[ExpectedException(typeof(ArgumentOutOfRangeException))]
475+
public unsafe void Test_ReadOnlySpan2DT_Range_Indexer_Fail()
476+
{
477+
int[,] array = new int[4, 4];
478+
479+
ReadOnlySpan2D<int> span2d = new ReadOnlySpan2D<int>(array);
480+
_ = span2d[0..6, 2..^1];
481+
482+
Assert.Fail();
483+
}
484+
#endif
485+
403486
[TestCategory("ReadOnlySpan2DT")]
404487
[TestMethod]
405488
public void Test_ReadOnlySpan2DT_Slice_1()
@@ -412,7 +495,7 @@ public void Test_ReadOnlySpan2DT_Slice_1()
412495

413496
ReadOnlySpan2D<int> span2d = new ReadOnlySpan2D<int>(array);
414497

415-
ReadOnlySpan2D<int> slice1 = span2d.Slice(1, 1, 2, 1);
498+
ReadOnlySpan2D<int> slice1 = span2d.Slice(1, 1, 1, 2);
416499

417500
Assert.AreEqual(slice1.Length, 2);
418501
Assert.AreEqual(slice1.Height, 1);
@@ -431,11 +514,11 @@ public void Test_ReadOnlySpan2DT_Slice_1()
431514

432515
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(-1, 1, 1, 1));
433516
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, -1, 1, 1));
434-
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, 1, -1, 1));
435517
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, 1, 1, -1));
518+
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, 1, -1, 1));
436519
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(10, 1, 1, 1));
437-
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, 12, 12, 1));
438-
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, 1, 1, 55));
520+
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, 12, 1, 12));
521+
Assert.ThrowsException<ArgumentOutOfRangeException>(() => new ReadOnlySpan2D<int>(array).Slice(1, 1, 55, 1));
439522
}
440523

441524
[TestCategory("ReadOnlySpan2DT")]
@@ -458,7 +541,7 @@ public void Test_ReadOnlySpan2DT_Slice_2()
458541
Assert.AreEqual(slice1[0, 0], 1);
459542
Assert.AreEqual(slice1[1, 1], 5);
460543

461-
ReadOnlySpan2D<int> slice2 = slice1.Slice(1, 0, 2, 1);
544+
ReadOnlySpan2D<int> slice2 = slice1.Slice(1, 0, 1, 2);
462545

463546
Assert.AreEqual(slice2.Length, 2);
464547
Assert.AreEqual(slice2.Height, 1);

‎UnitTests/UnitTests.HighPerformance.Shared/Memory/Test_Span2D{T}.cs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,89 @@ ref Unsafe.AsRef<int>(null),
564564
Assert.IsTrue(Unsafe.AreSame(ref r0, ref array[0, 0]));
565565
}
566566

567+
#if NETCOREAPP3_1_OR_GREATER
568+
[TestCategory("Span2DT")]
569+
[TestMethod]
570+
public unsafe void Test_Span2DT_Index_Indexer_1()
571+
{
572+
int[,] array = new int[4, 4];
573+
574+
Span2D<int> span2d = new Span2D<int>(array);
575+
576+
ref int arrayRef = ref array[1, 3];
577+
ref int span2dRef = ref span2d[1, ^1];
578+
579+
Assert.IsTrue(Unsafe.AreSame(ref arrayRef, ref span2dRef));
580+
}
581+
582+
[TestCategory("Span2DT")]
583+
[TestMethod]
584+
public unsafe void Test_Span2DT_Index_Indexer_2()
585+
{
586+
int[,] array = new int[4, 4];
587+
588+
Span2D<int> span2d = new Span2D<int>(array);
589+
590+
ref int arrayRef = ref array[2, 1];
591+
ref int span2dRef = ref span2d[^2, ^3];
592+
593+
Assert.IsTrue(Unsafe.AreSame(ref arrayRef, ref span2dRef));
594+
}
595+
596+
[TestCategory("Span2DT")]
597+
[TestMethod]
598+
[ExpectedException(typeof(IndexOutOfRangeException))]
599+
public unsafe void Test_Span2DT_Index_Indexer_Fail()
600+
{
601+
int[,] array = new int[4, 4];
602+
603+
Span2D<int> span2d = new Span2D<int>(array);
604+
605+
ref int span2dRef = ref span2d[^6, 2];
606+
}
607+
608+
[TestCategory("Span2DT")]
609+
[TestMethod]
610+
public unsafe void Test_Span2DT_Range_Indexer_1()
611+
{
612+
int[,] array = new int[4, 4];
613+
614+
Span2D<int> span2d = new Span2D<int>(array);
615+
Span2D<int> slice = span2d[1.., 1..];
616+
617+
Assert.AreEqual(slice.Length, 9);
618+
Assert.IsTrue(Unsafe.AreSame(ref array[1, 1], ref slice[0, 0]));
619+
Assert.IsTrue(Unsafe.AreSame(ref array[3, 3], ref slice[2, 2]));
620+
}
621+
622+
[TestCategory("Span2DT")]
623+
[TestMethod]
624+
public unsafe void Test_Span2DT_Range_Indexer_2()
625+
{
626+
int[,] array = new int[4, 4];
627+
628+
Span2D<int> span2d = new Span2D<int>(array);
629+
Span2D<int> slice = span2d[0..^2, 1..^1];
630+
631+
Assert.AreEqual(slice.Length, 4);
632+
Assert.IsTrue(Unsafe.AreSame(ref array[0, 1], ref slice[0, 0]));
633+
Assert.IsTrue(Unsafe.AreSame(ref array[1, 2], ref slice[1, 1]));
634+
}
635+
636+
[TestCategory("Span2DT")]
637+
[TestMethod]
638+
[ExpectedException(typeof(ArgumentOutOfRangeException))]
639+
public unsafe void Test_Span2DT_Range_Indexer_Fail()
640+
{
641+
int[,] array = new int[4, 4];
642+
643+
Span2D<int> span2d = new Span2D<int>(array);
644+
_ = span2d[0..6, 2..^1];
645+
646+
Assert.Fail();
647+
}
648+
#endif
649+
567650
[TestCategory("Span2DT")]
568651
[TestMethod]
569652
public void Test_Span2DT_Slice_1()

0 commit comments

Comments
 (0)