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

Index buffer offset not being set with dynamic index buffer and indirect call #3251

Open
oxheron opened this issue Feb 12, 2024 · 25 comments
Open

Comments

@oxheron
Copy link

oxheron commented Feb 12, 2024

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Build https://github.com/oxheron/bgfx
Run example 01-cubes
Use renderdoc or similar and look at the order of vertices emitted between the first call
and the second. You will see that it is different. If you look in the source code however you will see the vertex buffer changes between each call, and the index buffer is supposed to change to accommodate this. However, it doesn't.
Yes this is a really scuffed way to validate this, but I didn't have much time to finish this. I will try and make a better example if needed.

Expected behavior
I expected the vertices order to be the same between each call as the index buffer would change order to go along with the vertex buffer changing order.

Screenshots
bgfx_1

bgfx_2

@oxheron
Copy link
Author

oxheron commented Feb 12, 2024

Sorry if the screenshot quality is bad

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

It doesn't look like there is going to be an easy fix for this in opengl, as the functions to offset buffers don't work on instance buffers, and glMultiDrawElementsIndirect doesn't have an offset either. However Im pretty sure a pure bgfx solution would work in Vulkan and D3D

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

And this may not be specific to dynamic buffers either, just any index buffers that are subbuffers & need to be offsetted.

@bkaradzic
Copy link
Owner

Can you fix your example to actually work as any other example with shaders binaries being in runtime instead in source directory. I just ran example you provided and it just crashes.

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

Sorry. Thought that was a build thing not working for me. Ill fix it

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

I cant build the hlsl or metal shaders so ill just write a script in the cubes dir to do that if you're using windows. Also, so far this issue has only occurred on OpenGL and Vulkan.

@bkaradzic
Copy link
Owner

GL shaders + SPIRV are enough, I'm looking at GL bug, SPIRV just for VK reference.

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

Should be good now.

@bkaradzic
Copy link
Owner

It doesn't work...

If you just name your shaders vs_, fs_, cs_ it would build with provided makefile, and it would just output where it needs to be.

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

ok. cant see why but, ill fix that now.

@bkaradzic
Copy link
Owner

I'm also on Linux, so .bat doesn't work there.

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

yeah, that was just for windows. cause i didn't see your message about D3D till id already finished it.

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

hopefully it works now

@bkaradzic
Copy link
Owner

Failed to load shaders/glsl/vs_bug.bin

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

Did you build the shaders?

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

Cause I just put the names so they should build with the system.

@bkaradzic
Copy link
Owner

diff --git a/examples/01-cubes/fs_bug.sc b/examples/01-cubes/fs_bug.sc
index 4b544fe..226f516 100644
--- a/examples/01-cubes/fs_bug.sc
+++ b/examples/01-cubes/fs_bug.sc
@@ -1,7 +1,5 @@
-vec4 vec4_splat(float input)
-{
-       return vec4(input, input, input, input);
-}
+
+#include <bgfx_shader.sh>
 
 void main()
 {
diff --git a/examples/01-cubes/vs_bug.sc b/examples/01-cubes/vs_bug.sc
index 36c3aa7..2748563 100644
--- a/examples/01-cubes/vs_bug.sc
+++ b/examples/01-cubes/vs_bug.sc
@@ -1,6 +1,8 @@
 $input a_position
 
+#include <bgfx_shader.sh>
+
 void main()
 {
-       gl_Position = vec4(a_position, 1);
+       gl_Position = vec4(a_position, 1.0);
 }

@bkaradzic
Copy link
Owner

Ok, so now when I run GL I should see red triangle. And when I run VK I should see something else?

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

No, its a really hard bug to show

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

But it only shows up in a rendering debugger. If you read my initial comment youll see what I mean. Also, after reading that if you have a better idea on how to confirm a bug like that then Ill implement it.

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

Just visually i mean

@bkaradzic
Copy link
Owner

Ok, I'll try to understand what's going on... It's usually better to have repro to be obvious when something is wrong.

@oxheron
Copy link
Author

oxheron commented Feb 14, 2024

yeah no, sorry. I 100% agree. I will say, I think my thought is that when you use dynamic index buffers bgfx packs all of the data into one index buffer, and an offset is used to make sure the right buffer is selected. But when you do a indirect call there is no way for this offset to be set for index buffers (because all index buffer offsets are part of the indirect call, which is created on the gpu). I can't say why it seems to get the vertex buffer right as I would assume this issue happens on the vertex buffers as well (but it doesn't).

@oxheron
Copy link
Author

oxheron commented Feb 19, 2024

I implemented a potential temporary fix here, on the master branch https://github.com/oxheron/bgfx, if you want to take a look at it. It just provides an interface for users to get the index buffer offset, so it puts it on the users to add that into their indirect buffer calls. This should probably be tested more, but it worked for me on Vulkan so far (and seems that it could be a just bgfx side call anyways).

@oxheron
Copy link
Author

oxheron commented Feb 19, 2024

I don't know if this would be a good long term fix or not however.

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