Skip to content

Commit

Permalink
vk: improve staging debug tracking
Browse files Browse the repository at this point in the history
Move RT buffer commits to better locations, right after they have locked staging.
  • Loading branch information
w23 committed Dec 10, 2024
1 parent bf53d6d commit 8406ee1
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 21 deletions.
6 changes: 5 additions & 1 deletion ref/vk/TODO.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
## Next
- [ ] Fix glitch geometry
- [ ] Which specific models produce it? Use nsight btw
- [ ] Proper staging-vs-frame tracking, replace tag with something sensitive
- currently assert fails because there's 2 frame latency, not one.
- [ ] comment for future: full staging might want to wait for previous frame to finish
Expand All @@ -18,7 +20,6 @@
## 2024-12-10 E383
- [x] Add transfer stage to submit semaphore separating command buffer: fixes sync for rt
- [x] Issue staging commit for a bunch of RT buffers (likely not all of them)
- [ ] Go through all staged buffers and make sure that they are committed
- [x] move destination buffer tracking to outside of staging:
- [x] vk_geometry
- [x] vk_light: grid, metadata
Expand All @@ -27,6 +28,9 @@
- [x] staging should not be aware of cmdbuf either
- [x] `R_VkStagingCommit()`: -- removed
- [x] `R_VkStagingGetCommandBuffer()` -- removed
- [x] Go through all staged buffers and make sure that they are committed
- [x] Commit staging in right places for right buffers
- [x] Add mode staging debug tracking/logs

## 2024-05-24 E379
- [ ] refactor staging:
Expand Down
5 changes: 5 additions & 0 deletions ref/vk/vk_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ qboolean VK_BufferCreate(const char *debug_name, vk_buffer_t *buf, uint32_t size

buf->size = size;

INFO("Created buffer=%llx, name=\"%s\", size=%u", (unsigned long long)buf->buffer, debug_name, size);

return true;
}

Expand Down Expand Up @@ -196,6 +198,7 @@ vk_buffer_locked_t R_VkBufferLock(vk_buffer_t *buf, vk_buffer_lock_t lock) {
}

void R_VkBufferUnlock(vk_buffer_locked_t lock) {
DEBUG("buf=%llx staging pending++", (unsigned long long)lock.impl_.buf->buffer);
R_VkStagingUnlock(lock.impl_.handle);
}

Expand All @@ -222,6 +225,8 @@ void R_VkBufferStagingCommit(vk_buffer_t *buf, struct vk_combuf_s *combuf) {
//DEBUG("buffer=%p copy %d regions from staging buffer=%p", buf->buffer, stb->regions.count, stb->staging);
vkCmdCopyBuffer(cmdbuf, stb->staging, buf->buffer, stb->regions.count, stb->regions.items);

DEBUG("buf=%llx staging pending-=%u", (unsigned long long)buf->buffer, stb->regions.count);
R_VkStagingCopied(stb->regions.count);
stb->regions.count = 0;

//FIXME R_VkCombufScopeEnd(combuf, begin_index, VK_PIPELINE_STAGE_TRANSFER_BIT);
Expand Down
1 change: 1 addition & 0 deletions ref/vk/vk_cvar.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ void VK_LoadCvars( void )
vk_device_target_id = gEngine.Cvar_Get( "vk_device_target_id", "", FCVAR_GLCONFIG, "Selected video device id" );

vk_debug_log = gEngine.Cvar_Get("vk_debug_log_", "", FCVAR_GLCONFIG | FCVAR_READ_ONLY, "");
R_LogSetVerboseModules( vk_debug_log->string );

gEngine.Cmd_AddCommand("vk_debug_log", setDebugLog, "Set modules to enable debug logs for");
}
Expand Down
2 changes: 2 additions & 0 deletions ref/vk/vk_image.c
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ void R_VkImageUploadCommit( struct vk_combuf_s *combuf, VkPipelineStageFlagBits
};

R_VkStagingUnlock(up->staging.lock.handle);
R_VkStagingCopied(1);

// Mark image as uploaded
up->image->upload_slot = -1;
Expand Down Expand Up @@ -535,6 +536,7 @@ void R_VkImageUploadCancel( r_vk_image_t *img ) {
// Technically we won't need that staging region anymore at all, but it doesn't matter,
// it's just easier to mark it to be freed this way.
R_VkStagingUnlock(up->staging.lock.handle);
R_VkStagingCopied(1);

// Mark upload slot as unused, and image as not subjet to uploading
up->image = NULL;
Expand Down
12 changes: 8 additions & 4 deletions ref/vk/vk_rtx.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,6 @@ static void performTracing( vk_combuf_t *combuf, const perform_tracing_args_t* a
.light_bindings = args->light_bindings,
});

// FIXME what's the right place for these
R_VkBufferStagingCommit(&g_ray_model_state.kusochki_buffer, combuf);
R_VkBufferStagingCommit(&g_ray_model_state.model_headers_buffer, combuf);

// Upload kusochki updates
{
const VkBufferMemoryBarrier bmb[] = { {
Expand Down Expand Up @@ -265,6 +261,7 @@ static void performTracing( vk_combuf_t *combuf, const perform_tracing_args_t* a
{
rt_resource_t *const tlas = R_VkResourceGetByIndex(ExternalResource_tlas);
tlas->resource = RT_VkAccelPrepareTlas(combuf);
R_VkBufferStagingCommit(&g_ray_model_state.model_headers_buffer, combuf);
}

prepareUniformBuffer(args->render_args, args->frame_index, args->frame_counter, args->fov_angle_y, args->frame_width, args->frame_height);
Expand Down Expand Up @@ -548,6 +545,13 @@ void VK_RayFrameEnd(const vk_ray_frame_render_args_t* args)
// Feed tlas with dynamic data
RT_DynamicModelProcessFrame();

// FIXME what's the right place for this?
// This needs to happen every frame where we might've locked staging for kusochki
// - After dynamic stuff (might upload kusochki)
// - Before performTracing(), even if it is not called
// See ~3:00:00-3:40:00 of stream E383 about push-vs-pull models and their boundaries.
R_VkBufferStagingCommit(&g_ray_model_state.kusochki_buffer, args->combuf);

ASSERT(args->dst.width <= g_rtx.max_frame_width);
ASSERT(args->dst.height <= g_rtx.max_frame_height);

Expand Down
43 changes: 27 additions & 16 deletions ref/vk/vk_staging.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ static struct {
r_flipping_buffer_t buffer_alloc;

uint32_t locked_count;
uint32_t pending_count;

uint32_t current_generation;

struct {
Expand Down Expand Up @@ -67,22 +69,6 @@ static uint32_t allocateInRing(uint32_t size, uint32_t alignment) {
return R_FlippingBuffer_Alloc(&g_staging.buffer_alloc, size, alignment );
}

void R_VkStagingGenerationRelease(uint32_t gen) {
DEBUG("Release: gen=%u current_gen=%u ring offsets=[%u, %u, %u]", gen, g_staging.current_generation,
g_staging.buffer_alloc.frame_offsets[0],
g_staging.buffer_alloc.frame_offsets[1],
g_staging.buffer_alloc.ring.head
);
R_FlippingBuffer_Flip(&g_staging.buffer_alloc);
}

uint32_t R_VkStagingGenerationCommit(void) {
DEBUG("Commit: locked_count=%d gen=%u", g_staging.locked_count, g_staging.current_generation);
ASSERT(g_staging.locked_count == 0);
g_staging.stats.total_size = g_staging.stats.images_size + g_staging.stats.buffers_size;
return g_staging.current_generation++;
}

r_vkstaging_region_t R_VkStagingLock(uint32_t size) {
const uint32_t alignment = 4;
const uint32_t offset = R_FlippingBuffer_Alloc(&g_staging.buffer_alloc, size, alignment);
Expand All @@ -100,8 +86,33 @@ r_vkstaging_region_t R_VkStagingLock(uint32_t size) {
}

void R_VkStagingUnlock(r_vkstaging_handle_t handle) {
DEBUG("Unlock: locked_count=%u pending_count=%u gen=%u", g_staging.locked_count, g_staging.pending_count, g_staging.current_generation);
ASSERT(g_staging.current_generation == handle.generation);
ASSERT(g_staging.locked_count > 0);
g_staging.locked_count--;
g_staging.pending_count++;
}

void R_VkStagingCopied(uint32_t count) {
ASSERT(g_staging.pending_count >= count);
g_staging.pending_count -= count;
}

void R_VkStagingGenerationRelease(uint32_t gen) {
DEBUG("Release: gen=%u current_gen=%u ring offsets=[%u, %u, %u]", gen, g_staging.current_generation,
g_staging.buffer_alloc.frame_offsets[0],
g_staging.buffer_alloc.frame_offsets[1],
g_staging.buffer_alloc.ring.head
);
R_FlippingBuffer_Flip(&g_staging.buffer_alloc);
}

uint32_t R_VkStagingGenerationCommit(void) {
DEBUG("Commit: locked_count=%u pending_count=%u gen=%u", g_staging.locked_count, g_staging.pending_count, g_staging.current_generation);

ASSERT(g_staging.locked_count == 0);
ASSERT(g_staging.pending_count == 0);

g_staging.stats.total_size = g_staging.stats.images_size + g_staging.stats.buffers_size;
return g_staging.current_generation++;
}

0 comments on commit 8406ee1

Please sign in to comment.