ComputeSharp icon indicating copy to clipboard operation
ComputeSharp copied to clipboard

Incorrect computation? (ReadWriteBuffer with bool in struct)

Open bjornhellander opened this issue 2 years ago • 1 comments

First off: This is a seriously cool project!

I started tinkering with one of the samples and ended up with this not-working-as-I-expected piece of code. Having zero experience in shaders etc, this could be all due to me not understanding what I am doing, but it seems rather simple. So I just wanted to ask if this code looks wrong in any way? The kernel is simplified to the point of being silly, but the code still fails as described in the debug.assert comment below: B is not always set and P is sometimes garbled. Changing B to an int and adjusting the code accordingly makes it work the way I expected.

Using 2.0.0-alpha-12.

using System.Diagnostics;

namespace ComputeSharp.Sample
{
    public partial class Program
    {
        public static void Main()
        {
            var array = Enumerable.Range(1, 100).Select(i => new Stuff { P = new Int2(i, i) }).ToArray();

            using var gpuBuffer = GraphicsDevice.Default.AllocateReadWriteBuffer(array);
            GraphicsDevice.Default.For(100, new TestKernel(gpuBuffer));
            gpuBuffer.CopyTo(array);

            var ok = array.All(x => x.B == true && x.P.X == x.P.Y);
            Debug.Assert(ok); // Fails! B is false in some elements and P is messed up in others. 
        }

        [AutoConstructor]
        internal readonly partial struct TestKernel : IComputeShader
        {
            public readonly ReadWriteBuffer<Stuff> buffer;

            public void Execute()
            {
                buffer[ThreadIds.X].B = true;
            }
        }

        public struct Stuff
        {
            public Int2 P;
            public bool B;
        }
    }
}

bjornhellander avatar Jan 15 '22 13:01 bjornhellander

Hey! Thank you for the kind words, glad to hear you like this project! 😄

That minimal repro was great, thank you for taking the time to create that! I can repro the issue on my end as well. Not entirely sure why this is happening, I'll investigate when I have some time. In the meantime, you can replace that bool field with an int field, and just manually use 0 and 1 to represent false and true respectively. Not ideal, but it works 😅

Sergio0694 avatar Jan 15 '22 16:01 Sergio0694

@bjornhellander Thank you for opening this issue! I finally had some time to investigate this, and it actually revealed several other issues that I could also fix. The fix is included in #365, and it will be available in the next preview (as well as nightlies) 🙂

Sergio0694 avatar Aug 27 '22 21:08 Sergio0694