CommunityToolkit / dotnet

.NET Community Toolkit is a collection of helpers and APIs that work for all .NET developers and are agnostic of any specific UI platform. The toolkit is maintained and published by Microsoft, and part of the .NET Foundation.
https://docs.microsoft.com/dotnet/communitytoolkit/?WT.mc_id=dotnet-0000-bramin
Other
2.95k stars 291 forks source link

Error When Creating Memory2D from MemoryManager with Offset #742

Closed ron-kuhn closed 1 month ago

ron-kuhn commented 1 year ago

Describe the bug

When creating Memory2D from a MemoryManager the internal offset should be in bytes but it is just this index offset (see line 381). The created Memory2D object will be pointing to an invalid region if using a generic other than byte or sbyte. See the code (xunit test below to reproduce.

Regression

Worked in 8.1.0

Steps to reproduce

csharp
    public class Memory2DTester<T> : MemoryManager<T> where T : unmanaged, INumber<T>, IMinMaxValue<T>
    {
        public const int Width = 10;
        public const int Height = 10;

        private readonly T[] _data = Enumerable.Range(0, Width*Height).Select(T.CreateChecked).ToArray();

        protected override void Dispose(bool disposing)
        { }

        // get bottom right 9x9
        public Memory2D<T> GetMemory2DFromMM() =>
            new(this, Width + 1, Height - 1, Width - 1, 1);

        public Memory2D<T> GetMemory2DFromArray() =>
            new(_data, Width + 1, Height - 1, Width - 1, 1);

        public override Span<T> GetSpan() => new Span<T>(_data);

        public override MemoryHandle Pin(int elementIndex = 0)
        {
            throw new NotImplementedException();
        }

        public override void Unpin()
        {
            throw new NotImplementedException();
        }
    }

    [Fact]
    void Test()
    {
        var byteTester = new Memory2DTester<byte>();
        var byteSpan2DFromMM = byteTester.GetMemory2DFromMM().Span;
        var byteSpan2DFromArray = byteTester.GetMemory2DFromArray().Span;
        Assert.Equal(11, byteSpan2DFromMM[0, 0]);
        Assert.Equal(11, byteSpan2DFromArray[0, 0]);

        var shortTester = new Memory2DTester<short>();
        var shortSpan2DFromMM = shortTester.GetMemory2DFromMM().Span;
        var shortSpan2DFromArray = shortTester.GetMemory2DFromArray().Span;
        Assert.Equal(11, shortSpan2DFromArray[0, 0]);
        Assert.Equal(11, shortSpan2DFromMM[0, 0]);
    }


### Expected behavior

Should pass the unit test but fails on last line of unit test (shorSpan2DFromMM).

### Screenshots

![image](https://github.com/CommunityToolkit/dotnet/assets/67481892/61c5d8d2-df63-4cb8-bd59-6cfdd737f29c)

### IDE and version

VS 2022

### IDE version

_No response_

### Nuget packages

- [ ] CommunityToolkit.Common
- [ ] CommunityToolkit.Diagnostics
- [X] CommunityToolkit.HighPerformance
- [ ] CommunityToolkit.Mvvm (aka MVVM Toolkit)

### Nuget package version(s)

8.2.1

### Additional context

_No response_

### Help us help you

Yes, I'd like to be assigned to work on this item
ron-kuhn commented 1 year ago

Actually worked in version 8.2.0

ron-kuhn commented 10 months ago

@Sergio0694, I noticed a new version of CommunityToolkit.HighPerformance package (8.2.2). Is this fixed in that version?

Sergio0694 commented 10 months ago

It isn't, as this issue is still open. Haven't had time to triage this just yet 🙂

ron-kuhn commented 10 months ago

Just verified that the unittest still fails for 8.2.2. Should be easy fix described in the description.