bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
14.59k stars 1.92k forks source link

Ability to bind vertex buffer using offset #3262

Open falia18 opened 3 months ago

falia18 commented 3 months ago

I'm @Justine1801 on Discord if you want to talk about the PR. I really care about this getting into the main repository if possible, because it makes it much easier for remastering games.

Summary of changes

In bgfx_p and renderers, replace startVertex and num by offset and size.
In bgfx.h, add functions to bind a vertex buffer using offset instead of startVertex. Also allows to set instance buffer stride when binding.

Why the changes

My company worked on the remaster of a AAA game (not announced yet) with OpenGL-like API for graphics and ran into limitations due to the way buffers were handled in the game.

I am confident the changes work, they were extensively tested with DX11 & DX12, GNMP, a bit with Vulkan (before we switched to NVN) Examples run without issue for OpenGl but we couldn't test it in game due to other incompatibilities. Metal was not tested but the changes in renderer_mm are minimal so I don't think it will break anything.

Use cases

The changes couldn't be made for TransientVertexBuffer due to the struct being in the API (I can't change startVertex to offset because it would break existing projects). In my opinion it doesn't matter as much, because a transient buffer is meant to be used immediately anyway, and there is less chance you don't have the layout on hand to bypass the limitations.

Fixes

Changing Dynamic vertex buffer stride between creation time and bind time now works. It worked for static vertex buffer already, except in DX12 see my previous PR

Left to do

I added the changes to IDL but I don't know how to generate doxygen and bindings with it. Which is also why I haven't made the changes in the documentation yet.

About stride 0

TLDR: OpenGL only supports it from version 4.3. Works with minimal changes in cross platform code for all other backends.

These are the few additionnal changes needed to make stride 0 work:

The issue is OpenGL:
bgfx's code uses glVertexAttribPointer for vertex attributes, and when this function is given a stride of 0, the vertex attributes are understood to be tightly packed in the array (https://registry.khronos.org/OpenGL-Refpages/gl4/html/glVertexAttribPointer.xhtml)
Which means it won't work because it will autocompute the stride.

There is a way to make it work, and it's to replace that function by:
glBindVertexBuffer and glVertexAttribFormat
which work more like D3D functions.

But it's only available from OpenGL 4.3. It could be implemented with a define checking for minimal version, but from what I've seen while skimming through the gl renderer it's not something you do.

falia18 commented 3 months ago

Use cases example Launch 01-cubes with any renderer other than d3d12 (d3d12 needs this fix to work).
It contains example of buffers where startVertex isn't sufficient to properly bind them. I also implemented the working solution with this PR for comparison. Define BIND_WITH_OFFSET and checkout this PR to make it run. This is the expected result: image

Dynamic vertex buffer stride issue
Lauch 35-dynamic, it gives this:
image instead of this:
image