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

Triple buffering for metal - sokol_gfx.h #888

Open
danielchasehooper opened this issue Sep 15, 2023 · 1 comment
Open

Triple buffering for metal - sokol_gfx.h #888

danielchasehooper opened this issue Sep 15, 2023 · 1 comment

Comments

@danielchasehooper
Copy link
Contributor

Apple recommends triple buffering dynamic content in metal. Should SG_NUM_INFLIGHT_FRAMES be set to 3 when the metal backend is chosen, or should sokol allow #define SG_NUM_INFLIGHT_FRAMES 3 to to be declared before the header is imported?

@floooh
Copy link
Owner

floooh commented Sep 17, 2023

Currently the only way to change SG_NUM_INFLIGHT_FRAMES is to patch the sokol header, I did it this way because that value unfortunately leaks into the public API (for instance here:

sokol/sokol_gfx.h

Line 2291 in d4ac122

const void* mtl_buffers[SG_NUM_INFLIGHT_FRAMES];
)... although that's not exactly a core feature.

This public API 'leakage' is also why it must be a comptime constant and can't be provided in sg_desc at startup.

If you decide to make this a preprocessor define which can be provided from the outside, you'll need to make sure that all places which include the header (not just the implementation) see the same define (so it should probably be provided on the compiler command line by the build system instead of relying on a #define in code before the header is included.

Also make sure to check that this change doesn't increase frame latency.

Apart from that, as far as I can see the sokol_gfx.h 'frame management' code is identical with that Apple example code (except that kMaxInflightBuffers is 2 instead of 3), but whether this longer 'pipeline' also increases latency depends on the CAMetalLayer swap strategy: if it works like Vulkan's FIFO strategy it would increase latency, if it works like Mailbox strategy, it would not, because if two frames are waiting for presentation it would pick the most recent one and discard the other.

Currently I don't want to make this change in the 'official' header though, because I don't like this 'NUM_INFLIGHT_FRAMES' constant in the first place. A "proper" fix would be to have a MAX_INFLIGHT_FRAMES constant that cannot be changed and is set to something 'big enough for all use cases' like 4 and use that in the public API structs, while the actual num_inflight_frames would be provided in sg_desc and must be <= MAX_INFLIGHT_FRAMES.

(PS: if you're 'workload' comfortably fits into the 8.333ms frame budget (for 120Hz), triple buffering shouldn't be needed, because then the CPU side always has enough wiggle room to provide the next frame in time before the next presentation needs to happen, and TBH I don't quite understand from that article why 3 buffers would be needed (instead of just 2) to prevent the GPU and CPU from trampling on each others feet (the GPU will read one buffer, while the CPU writes the next, and since the CPU is usually ready before the next vsync, the buffer that was just written by the CPU will already be ready at the start of the next frame for the GPU to read).

...but anyway, if you see any advantages with NUM_INFLIGHT_FRAMES = 3 let me know and we can figure something out :)

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

No branches or pull requests

2 participants