Skip to content
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

Q: gfx: Quad rendering on Metal but not OpenGL #1117

Open
xslendix opened this issue Oct 4, 2024 · 8 comments
Open

Q: gfx: Quad rendering on Metal but not OpenGL #1117

xslendix opened this issue Oct 4, 2024 · 8 comments

Comments

@xslendix
Copy link

xslendix commented Oct 4, 2024

Hello, I have the following shaders:

#version 450

precision mediump float;

layout (location = 0) in flat vec4 inColor;

layout (location = 0) out vec4 fragColor;

void main() 
{
	fragColor = inColor;
}

and

#version 450

layout (location = 0) in vec3 aPos;
layout (location = 1) in vec4 aColor;

layout (std140, binding=0) uniform matrices
{
	mat4 projection;
	mat4 view;
	mat4 model;
};

layout (location = 0) out flat vec4 outColor;

void main()
{
	gl_Position = projection * view * model * vec4(aPos, 1.0);
	outColor = aColor;
}

For MSL, I use glslcc to re-compile. The way I get a shader descriptor:

get_shader_desc :: proc(
	shader: string,
	backend: sg.Backend,
	attrs_core: [16]sg.Shader_Attr_Desc,
	attrs_hlsl: [16]sg.Shader_Attr_Desc,
	attrs_metal: [16]sg.Shader_Attr_Desc,
) -> (desc: sg.Shader_Desc) {
	desc.label = fmt.caprintf("{}_shader", shader)
	desc.vs.entry = "main"
	desc.fs.entry = "main"
	vs_src: string
	defer delete(vs_src)
	fs_src: string
	defer delete(fs_src)
	#partial switch backend {
	case .GLCORE:
		fs_src = fmt.aprintf("{}/{}.{}", SHADER_DIR, shader, "frag.glsl")
		vs_src = fmt.aprintf("{}/{}.{}", SHADER_DIR, shader, "vert.glsl")
		desc.attrs = attrs_core
	case .D3D11:
		fs_src = fmt.aprintf("{}/{}.{}", SHADER_DIR, shader, "frag.hlsl")
		vs_src = fmt.aprintf("{}/{}.{}", SHADER_DIR, shader, "vert.hlsl")
		desc.vs.d3d11_target = "vs_5_0"
		desc.fs.d3d11_target = "ps_5_0"
		desc.attrs = attrs_hlsl
	case .METAL_MACOS:
		fs_src = fmt.aprintf("{}/{}.{}", SHADER_DIR, shader, "frag.msl")
		vs_src = fmt.aprintf("{}/{}.{}", SHADER_DIR, shader, "vert.msl")
		desc.vs.entry = "main0"
		desc.fs.entry = "main0"
		desc.attrs = attrs_metal
	case:
		fmt.panicf("Unsupported backend: {}", backend)
	}
	vs_src_c, ok := read_whole_shader(vs_src)
	if !ok do fmt.panicf("Failed to read vertex shader code")
	fs_src_c, ok2 := read_whole_shader(fs_src)
	if !ok2 do fmt.panicf("Failed to read fragment shader code")
	desc.vs.source = vs_src_c
	desc.fs.source = fs_src_c

	return
}

This is how I make my pipeline and buffers:

	g_gs.bind.vertex_buffers[0] = sg.make_buffer(
		{type = .VERTEXBUFFER, data = {ptr = &vertices, size = size_of(vertices)}},
	)

	indices := [?]u16{0, 1, 2, 0, 2, 3}
	g_gs.bind.index_buffer = sg.make_buffer(
		{type = .INDEXBUFFER, data = {ptr = &indices, size = size_of(indices)}},
	)

	g_gs.pipeline = sg.make_pipeline(
		{
			shader = sg.make_shader(get_shader_desc("basic", sg.query_backend(), {}, {}, {})),
			cull_mode = .FRONT,
			index_type = .UINT16,
			layout = {attrs = {0 = {format = .FLOAT3}, 1 = {format = .FLOAT4}}},
		},
	)

For some reason, on Metal, this works perfectly fine, for OpenGL, I just passthrough the original GLSL code. Why does it render on one but not the other?

macos

opengl_linux

Those two are the same program running, with only shaders deferring. What causes this discrepancy?

@xslendix xslendix changed the title gfx: Quad rendering on Metal but not OpenGL Q: gfx: Quad rendering on Metal but not OpenGL Oct 4, 2024
@floooh
Copy link
Owner

floooh commented Oct 5, 2024

Can you try building/running with --debug too see if the sokol-gfx validation layer has anything to complain about?

One thing that looks wrong is that the shader-desc struct is missing uniform descriptions like this:

https://github.com/floooh/sokol-odin/blob/a459fd5b2b1b50382ce8845d896fe5194b8fc898/examples/mrt/shader.odin#L1304-L1308

..specifically for this uniform block:

layout (std140, binding=0) uniform matrices
{
	mat4 projection;
	mat4 view;
	mat4 model;
};

For the non-GL APIs you need to provide at least the size of the uniform block (e.g. in this case 3 * 64 bytes), and for the GL backend you'll need to describe the interior of the uniform block (e.g. that it consists of 3 mat4's).

With the sokol-gfx validation layer enabled, you should at least get errors in the sg_apply_uniforms() call, also in the Metal backend.

Also, since you are recompiling the shader anyway, also check out the sokol-shdc tool, this is specifically tailored to sokol-gfx: https://github.com/floooh/sokol-tools/blob/master/docs/sokol-shdc.md

PS: also those defer calls in the get_shader_desc function look suspicious, any data referenced by the ShaderDesc struct needs to survive until the make_shader call returns.

@floooh
Copy link
Owner

floooh commented Oct 5, 2024

...strange, same error happens with all Odin versions downloaded from the Github release page (so far I tested back to the dev-2024-06

Scratch that, I was just being dumb... I forgot to rebuild the C libraries so that they match the updated Odin bindings...

@xslendix
Copy link
Author

xslendix commented Oct 6, 2024

I have changed the pipeline creation to this:

	g_gs.pipeline = sg.make_pipeline(
		{
			shader = sg.make_shader(
				get_shader_desc(
					"basic",
					sg.query_backend(),
					{},
					{},
					{},
					{
						0 = {
							size = size_of(Matrices),
							layout = .STD140,
							uniforms = {
								0 = {name = "projection", type = .MAT4, array_count = 1},
								1 = {name = "view", type = .MAT4, array_count = 1},
								2 = {name = "model", type = .MAT4, array_count = 1},
							},
						},
					},
				),
			),
			cull_mode = .FRONT,
			index_type = .UINT16,
			layout = {attrs = {0 = {format = .FLOAT3}, 1 = {format = .FLOAT4}}},
		},
	)

Where it passes that Shader_Uniform_Block_Desc to the vertex shader (desc.vs.uniform_blocks = gl_vs_uniform_blocks). This got rid of all the errors while running with -debug, but still, the quad is not rendering, even with culling disabled.

@floooh
Copy link
Owner

floooh commented Oct 6, 2024

Don't you provide an empty attrs_core array to the get_shader_desc function when for the GL case?

...wait no, that's actually fine because the attr position is defined by layout(location)...

Hmm... did you try running the code in RenderDoc to check what actually arrives down in OpenGL?

@floooh
Copy link
Owner

floooh commented Oct 6, 2024

PPS: how do you initialize the OpenGL context, e.g. do you actually create a version 4.5 context? Since you pass through the GL shader code as is, I would expect that the #version 450 can cause trouble if the GL version isn't at least version 4.5, I would also expect shader compilation errors coming out of GL in that case though which should be visible as sokol-gfx error messages.

Is your project public somewhere so that I could have a look, that would simplify the 'remote debugging' a lot.

...also the various defer in the get_shader_desc() function still look fishy to me, but that would either fail on both backends or none.

@xslendix
Copy link
Author

xslendix commented Oct 6, 2024

@floooh I have sent you an email with the link to the full project source code. The defers are fine they just store paths to the shaders.

@floooh
Copy link
Owner

floooh commented Oct 6, 2024

Btw, one thing I just noticed while working on the sokol-shdc tool, since your uniforms are inside a uniform block, they need to be referred as follows in the shader desc:

uniforms = {
    0 = {name = "matrices.projection", type = .MAT4, array_count = 1},
    1 = {name = "matrices.view", type = .MAT4, array_count = 1},
    2 = {name = "matrices.model", type = .MAT4, array_count = 1},
},

You can also keep the array_count uninitialized, since the default value is 1.

Little details like this are also taken care of by the sokol-shdc tool. This would also not produce an error message, since uniform names which couldn't be looked up are ignored (

sokol/sokol_gfx.h

Lines 9398 to 9400 in 38e4c9a

if (u->gl_loc == -1) {
continue;
}
), this is expected behaviour since GLSL compilers tend to drop uniforms which are not referenced by the shader code.

@xslendix
Copy link
Author

xslendix commented Oct 9, 2024

Tried with that but still no shot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants