Skip to content

cube: Re-record command buffers each frame #1107

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 11, 2025
Merged

cube: Re-record command buffers each frame #1107

merged 3 commits into from
Apr 11, 2025

Conversation

charles-lunarg
Copy link
Contributor

Modifies cube & cubepp to re-record the command buffers each frame instead of pre-recording them at application startup and during resize.

This enables vast refactoring to use best practices for organizing frame and swapchain data into two resource groups: per-submission, and per- swapchain-image. A submission resource is any data needed to render (uniform buffer, descriptor set), any command buffers, the image acquire semaphore, and the fence used in the submission. There are currently two submission resource sets, and the application alternates between the two on subsequent frames. The swapchain-image resources are separate because they are dependent on the number of swapchain images and because the swapchain image index used each frame isn't deterministic.

In addition to the above architectural change, this commit refactors swapchain recreation to only recreate a minimum of necessary objects. This is the swapchain, the swapchain image views, the swapchain specific semaphores, the depth buffer, and the framebuffer. It is possible to avoid recreating frame buffers & semaphores using newer features ( dynamic_rendering & swapchain_maintenance1) but was decided against due to vkcube needing 1.0 compatability.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 415664.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 415679.

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 74c6fb3 to 8849d58 Compare April 7, 2025 20:06
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 415681.

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 8849d58 to 9ae529b Compare April 7, 2025 20:07
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 415683.

@charles-lunarg charles-lunarg linked an issue Apr 7, 2025 that may be closed by this pull request
vkcube with vvl reports VUID-vkQueueSubmit-pSignalSemaphores-00067 #1106
Closed
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1673 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1673 failed.

@water-chika
Copy link
Contributor

here are currently two submission resource sets, and the application alternates between the two on subsequent frames. The swapchain-image resources are separate because they are dependent on the number of swapchain images and because the swapchain image index used each frame isn't deterministic.

If there are only two submission resource sets, why we do not create a swapchain with two images?

@water-chika
Copy link
Contributor

re-record the command buffers each frame instead of pre-recording them at application startup and during resize.

If two resource groups (per-submission, and per- swapchain-image) could be associated, the command buffers could still be pre-recorded.

Such as, if there are 4 swapchain-images and 2 submission-resource set:

swapchain-image index submission-resource index
0 0
1 1
2 0
3 1

And create 4 command buffers to record these.

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 9ae529b to 918b9ce Compare April 8, 2025 21:16
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 416658.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 416674.

@charles-lunarg
Copy link
Contributor Author

My stance is that pre-recording command buffers is not indicative of a typical program architecture, so makes for a poor demonstration of the Vulkan API because of how rare pre-recording can be used outside of simple demo applications (like vkcube).

The other reason is that the order of swapchain images is 'effectively' (but not always) random, so to allow for continuous submissions you'd have to have a matrix of submission resource sets with each swapchain resource sets. And that makes understanding the synchronization flow significantly harder. The big idea is that we have a fence that protects the per-submission resources from being updated before the previous time the resource was used is done.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1676 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1676 failed.

@water-chika
Copy link
Contributor

How about performance influence?

@charles-lunarg
Copy link
Contributor Author

No perceptible performance influence. Recording command buffers is CHEAP all things considered - and for complex applications the ability to change what is and isn't drawn offers significant performance advantages.

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 918b9ce to 9e57d5a Compare April 10, 2025 21:11
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 418538.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 418553.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1679 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1679 passed.

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 9e57d5a to ae15d3a Compare April 11, 2025 18:36
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419276.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419291.

spencer-lunarg
spencer-lunarg requested changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@spencer-lunarg spencer-lunarg left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get a blank screen on both XCB and XLIB (didn't triage at all)

(But it is validation layer clean!)

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1681 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1681 passed.

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from ae15d3a to 0881bd0 Compare April 11, 2025 18:54
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419312.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1682 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419328.

@charles-lunarg
Copy link
Contributor Author

@spencer-lunarg good catch, I didn't run this on linux after making fixes on windows. It was a very simple inverted condition.

if (!demo->initialized || !demo->swapchain_ready) { // this is backwards, it should be  if (demo->initialized && demo->swapchain_ready) 
            demo_draw(demo);
            if (demo->is_minimized) {
                demo->curFrame++;
            }
            if (demo->frameCount != INT32_MAX && demo->curFrame == demo->frameCount) demo->quit = true;
        }

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 0881bd0 to 89a41e6 Compare April 11, 2025 18:56
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419329.

@spencer-lunarg
Copy link
Contributor

--wsi xlib still give a blank white screen

@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 89a41e6 to 6acc11f Compare April 11, 2025 19:07
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419364.

@charles-lunarg
Copy link
Contributor Author

@spencer-lunarg 🤦‍♂️ it was the same issue xcb had, thanks for checking. I made sure to check if vkcubepp has the same problem, it does not.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1685 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1685 passed.

@charles-lunarg
cube: Re-record command buffers each frame
83930c7 
Rerecord command buffers every frame, as command buffer recording isn't
slow, and outside of trivial samples (like vkcube) isn't a practical
architecture for organizing a Vulkan renderer. This makes vkcube and
vkcubepp reflect typical Vulkan renderer architecture and serves as a
better sample.

Create structs to organize objects into distinct synchronization scopes,
one for submissions and the other for swapchains. Resources associated
with a swapchain image such, as image views, framebuffers, and
presentation semaphores, must be duplicated as many times as there are
swapchain images. This number is dependent on the runtime, so can't be
pre-determined. The submission resources, which are the command buffers,
fences, descriptor sets, device memory, and uniform buffers, only needs
duplication for the pipelining. Specifically, this allows the GPU to
execute a command buffer while the CPU record the next frame's command
buffer. The FRAME_LAG constant dictates the pipeline depth, commonly
called double buffering, and is set to 2 since that is sufficient for a
vast majority of use cases.

Swapchain resizing now only re-creates the bare necessity of resources:
Swapchain, image views, framebuffers, presentation semaphores, and the
depth buffer. Presentation semaphores are recreated to prevent state
from an old swapchain from polluting a new swapchain. The depth buffer
is re-created since its size is dependent on the window size. All other
objects, such as pipelines and renderpasses, do not need recreation as
they either are not liable to change during resizing, or have mutable
state that can be flexibly updated, such as dynamic pipeline viewport
and scissors. The code for dynamic viewports and scissors was already
present but wasn't taken advantage of until now.
@charles-lunarg charles-lunarg force-pushed the rerecord_cube_command_buffers branch from 6acc11f to 83930c7 Compare April 11, 2025 20:32
@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419447.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build queued with queue ID 419462.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1687 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Tools build # 1687 passed.

spencer-lunarg
spencer-lunarg approved these changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@spencer-lunarg spencer-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now works on xcb/xlib/wayland for both c and cpp

@charles-lunarg charles-lunarg merged commit a8f2078 into KhronosGroup:main Apr 11, 2025
18 checks passed
@charles-lunarg charles-lunarg deleted the rerecord_cube_command_buffers branch April 11, 2025 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ziga-lunarg ziga-lunarg ziga-lunarg approved these changes

@spencer-lunarg spencer-lunarg spencer-lunarg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vkcube with vvl reports VUID-vkQueueSubmit-pSignalSemaphores-00067
5 participants
@charles-lunarg @ci-tester-lunarg @water-chika @spencer-lunarg @ziga-lunarg