Help needed: Problems with compute shaders

[thargy]

I've been trying to get the Particle Compute shader example working using ShaderGen, and I'm not pretty much stuck. If managed to find my way around a lot of idiosyncracies but I could do with some help if anyone has time?

I've managed to create the code shown in #62 (and I won't reproduce here as it's quite large), and, so long as I split the class files into their own files (see #62), the Compute, Vertex and Fragment shaders all compile without error.

When using GraphicsBackend.Direct3D11 the particles zoom around the screen much faster than before (possibly the cause of the next issues), but clearly the shaders are doing something.

When changing to GraphicsBackend.OpenGL, the shaders 'load' but I get the following error:

VeldridException: Unable to compile shader code for shader [] of type VertexShader: 0(25) : error C0000: syntax error, unexpected identifier, expecting reserved word or reserved word "in" or reserved word "out" or reserved word "uniform" at token "buffer"

When I get to the first GraphicsDevice.SwapBuffers(MainSwapchain); call in Draw. (see here)

From what I can gather this is probably due to the following section of the 330.glsl shader, being invalid syntax:

layout(std140) readonly buffer Particles
{
    Apocraphy_Shaders_ParticleInfo field_Particles[];
};

This is generated from the following field in the Vertex/Fragment shader file:
public StructuredBuffer<ParticleInfo> Particles;

When changing to `GraphicsBackend.Vulkan', I get a different error:

System.AccessViolationException
HResult=0x80004003
Message=Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Source=
StackTrace:

When I get to the first GraphicsDevice.SwapBuffers(MainSwapchain); call in Draw (see here).

This might be related to whatever's causing the particles to zoom around on Direct3D11, which is possibly due to preceeding two lines no longer pointing to the correct places in the output shaders:

            _cl.SetComputeResourceSet(0, _computeResourceSet);
            _cl.SetComputeResourceSet(1, _computeScreenSizeResourceSet);

(also here)

For completeness, below are the generated Vertex shaders:

OpenGL

#version 330 core

struct SamplerDummy { int _dummyValue; };

struct Apocraphy_Shaders_ScreenSize
{
    float Width;
    float Height;
    vec2 _padding;
};

struct Apocraphy_Shaders_FragmentInput
{
    vec4 Position;
    vec4 Color;
};

struct Apocraphy_Shaders_ParticleInfo
{
    vec2 Position;
    vec2 Velocity;
    vec4 Color;
};

layout(std140) readonly buffer Particles
{
    Apocraphy_Shaders_ParticleInfo field_Particles[];
};
layout(std140) uniform ScreenSize
{
    Apocraphy_Shaders_ScreenSize field_ScreenSize;
};

Apocraphy_Shaders_FragmentInput VS()
{
    Apocraphy_Shaders_ParticleInfo input_ = field_Particles[uint(gl_VertexID)];
    Apocraphy_Shaders_FragmentInput output_;
    output_.Position.x = 2 * (input_.Position.x / field_ScreenSize.Width - 0.5f);
    output_.Position.y = 2 * (input_.Position.y / field_ScreenSize.Height - 0.5f);
    output_.Position.z = 0.f;
    output_.Position.w = 1.f;
    output_.Color = input_.Color;
    return output_;
}


out vec4 fsin_0;

void main()
{
    Apocraphy_Shaders_FragmentInput output_ = VS();
    fsin_0 = output_.Color;
    gl_Position = output_.Position;
        gl_Position.z = gl_Position.z * 2.0 - gl_Position.w;
}

Vulkan

#version 450
#extension GL_ARB_separate_shader_objects : enable
#extension GL_ARB_shading_language_420pack : enable
struct Apocraphy_Shaders_ScreenSize
{
    float Width;
    float Height;
    vec2 _padding;
};

struct Apocraphy_Shaders_FragmentInput
{
    vec4 Position;
    vec4 Color;
};

struct Apocraphy_Shaders_ParticleInfo
{
    vec2 Position;
    vec2 Velocity;
    vec4 Color;
};

layout(std140, set = 0, binding = 0) readonly buffer Particles
{
    Apocraphy_Shaders_ParticleInfo field_Particles[];
};
layout(set = 0, binding = 1) uniform ScreenSize
{
    Apocraphy_Shaders_ScreenSize field_ScreenSize;
};

Apocraphy_Shaders_FragmentInput VS()
{
    Apocraphy_Shaders_ParticleInfo input_ = field_Particles[gl_VertexIndex];
    Apocraphy_Shaders_FragmentInput output_;
    output_.Position.x = 2 * (input_.Position.x / field_ScreenSize.Width - 0.5f);
    output_.Position.y = 2 * (input_.Position.y / field_ScreenSize.Height - 0.5f);
    output_.Position.z = 0.f;
    output_.Position.w = 1.f;
    output_.Color = input_.Color;
    return output_;
}


layout(location = 0) out vec4 fsin_0;

void main()
{
    Apocraphy_Shaders_FragmentInput output_ = VS();
    fsin_0 = output_.Color;
    gl_Position = output_.Position;
        gl_Position.y = -gl_Position.y; // Correct for Vulkan clip coordinates
}

Direct3D:

struct Apocraphy_Shaders_ScreenSize
{
    float Width;
    float Height;
    float2 _padding;
};

struct Apocraphy_Shaders_FragmentInput
{
    float4 Position : SV_Position;
    float4 Color : COLOR0;
};

struct Apocraphy_Shaders_ParticleInfo
{
    float2 Position : POSITION0;
    float2 Velocity : POSITION1;
    float4 Color : COLOR0;
};

StructuredBuffer<Apocraphy_Shaders_ParticleInfo> Particles: register(t0);
cbuffer ScreenSizeBuffer : register(b0)
{
    Apocraphy_Shaders_ScreenSize ScreenSize;
}

Apocraphy_Shaders_FragmentInput VS(uint _builtins_VertexID : SV_VertexID)
{
    Apocraphy_Shaders_ParticleInfo input = Particles[_builtins_VertexID];
    Apocraphy_Shaders_FragmentInput output;
    output.Position.x = 2 * (input.Position.x / ScreenSize.Width - 0.5f);
    output.Position.y = 2 * (input.Position.y / ScreenSize.Height - 0.5f);
    output.Position.z = 0.f;
    output.Position.w = 1.f;
    output.Color = input.Color;
    return output;
}

[mellinoe]

The OpenGL problem is caused by the #version header being wrong. Basically, we need to detect when StructuredBuffers are being used, and switch to #version 430 core. We already switch versions in some cases when different resource types are used -- we just missed this one. This is working in Compute shaders because those automatically default to 430 core.

_cl.SetComputeResourceSet(0, _computeResourceSet);
_cl.SetComputeResourceSet(1, _computeScreenSizeResourceSet);

This part of ShaderGen isn't terribly well-documented. If you don't do anything special, then all of the resources you define will go into set 0. To override that, you have to put a [ResourceSet] attribute on the field. So the public ScreenSize ScreenSize; field should have [ResourceSet(1)] above it. That should fix up the resource linkage.

Note that the D3D11 backend doesn't care about ResourceSet's per-se. Everything gets translated into a flat binding scheme there, so as long as the resources are in order, then things will work out. I'm not sure why the particles are moving too fast there -- that's potentially a different problem.

[thargy]

Thanks, I’ll try that out tomorrow, I managed to figure most stuff out myself from diving through the code, but I ran out of time and I just don’t have enough shader experience. If I get it working would you like me to fork Veldrid-Samples and create a Pull Request with the updated shader for ComputeParticles, as it would be nice to have a complete example with Compute Shaders? I understand the benefit of not all the shaders using ShaderGen in the samples.

I also converted the shaders to embedded resources as you did in the other ShaderGen projects.

[mellinoe]

I think I would prefer to keep things as-is over in Veldrid-Samples, but I appreciate the offer. I've had feedback that people would rather see the shader code explicitly in the repo rather than have another tool introduced to them. I would like if at least the "basic" examples are using regular shader code, even if it takes longer to write.

I'll go ahead and fix the #version issue in a minute.

[thargy]

So the public ScreenSize ScreenSize; field should have [ResourceSet(1)] above it.

Thanks, adding the explicit ResourceSet attributes to the shaders fixed both the zoomies and the Vulkan crash.

I think I would prefer to keep things as-is over in Veldrid-Samples

Understood!

I'll go ahead and fix the #version issue in a minute.

Great, thanks! What's the NuGet pre-release strategy, is it done automatically on pull request acceptance?

[mellinoe]

Great, thanks! What's the NuGet pre-release strategy, is it done automatically on pull request acceptance?

Yep, new versions are automatically pushed to myget when a commit happens. I think that I fixed the #versino issue in this commit.

[thargy]

Yep, new versions are automatically pushed to myget when a commit happens. I think that I fixed the #versino issue in this commit.

I couldn't see any update when using the NuGet browser, so I went and checked the myget feed directly here and noticed that the build names are not correctly formatted, and so don't sort as you would expect.

Although the latest by date is:

1.2.0-beta2-g464080fc94	47.06 KB	Mon, 14 May 2018 04:58:00 GMT

it appears a long way down the list, with

1.2.0-beta2-g464080fc94	47.06 KB	Mon, 14 May 2018 04:58:00 GMT

appearing first, and so being the 'latest' by NuGet naming convention. As such Visual Studio won't show any update being available (it takes the first item in the feed as the latest).

To fix you need to use the correct semantic version (see (here)[https://docs.microsoft.com/en-us/nuget/reference/package-versioning]). Part of the issue is that the builds are currently outputting 1.2.0-beta2, which technically preceed 1.2.0-g due to suffices being sorted alphabetically.

You can supply build metadata in Semantic version 2, but technically it should be preceeded with a '+' not a '-' to indicate it is build metadata. Also, you should avoid on 'official' releases as it isn't supported by older NuGet clients.

e.g. (in descending order)

1.2.0+gf000000000
1.2.0+ge000000000
1.2.0-beta+gd000000000
1.2.0-alpha+gc000000000
1.9.2+gb000000000

Hope that helps.

[mellinoe]

Yeah, browsing in the VS UI won't work very well -- personally I just always manually edit package references and have less issues that way.

I would like to improve the CI auto-versioning, but I haven't looked into the specifics of it. Right now, the versioning is just handled by this package, and I haven't tried to customize it very much. Perhaps there's a way that we can get it to spit out something like v1.2.0-master-125 ?

You can supply build metadata in Semantic version 2, but technically it should be preceeded with a '+' not a '-' to indicate it is build metadata. Also, you should avoid on 'official' releases as it isn't supported by older NuGet clients.

Sure -- the git tag is just there for the rolling builds from CI. I'm not sure if the GitVersioning library lets you change the delimiter there. I think I'm just using the default.

[thargy]

@mellinoe - I did a quick read through of the Nerdbank.GitVersioning Nuget and I think you may be using it wrong.

The 3rd, 'patch' value is supposed to be set automatically by Nerdbank to the Git Height, so that it gives the expected package ordering. However, yours is always hard coded '0'. This seems to be because this line in version.json:
"version": "1.2.0.0-beta2"

should read:
"version": "1.2-beta2"

according to the specified file format.

I recommend you correct to "1.3-beta" to start numbering correctly.

[thargy]

This appears to be corrected in 71806a5.