Skip to content

Commit

Permalink
Manually free uniform sets.
Browse files Browse the repository at this point in the history
See #735
  • Loading branch information
Zylann committed Mar 7, 2025
1 parent f2bdc4c commit 662086f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 0 deletions.
16 changes: 16 additions & 0 deletions engine/detail_rendering/render_detail_texture_gpu_task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,8 @@ void RenderDetailTextureGPUTask::prepare(GPUTaskContext &ctx) {

// Make compute list

_uniform_sets_to_free.reserve(5 + modifiers.size());

const int compute_list_id = rd.compute_list_begin();

// Gather hits
Expand All @@ -310,6 +312,7 @@ void RenderDetailTextureGPUTask::prepare(GPUTaskContext &ctx) {
gather_hits_uniforms[5] = hit_positions_uniform;

const RID gather_hits_uniform_set_rid = uniform_set_create(rd, gather_hits_uniforms, gather_hits_shader_rid, 0);
_uniform_sets_to_free.push_back(gather_hits_uniform_set_rid);

rd.compute_list_bind_compute_pipeline(compute_list_id, _gather_hits_pipeline_rid);
rd.compute_list_bind_uniform_set(compute_list_id, gather_hits_uniform_set_rid, 0);
Expand Down Expand Up @@ -350,6 +353,7 @@ void RenderDetailTextureGPUTask::prepare(GPUTaskContext &ctx) {
}

const RID detail_generator_uniform_set = uniform_set_create(rd, detail_generator_uniforms, shader_rid, 0);
_uniform_sets_to_free.push_back(detail_generator_uniform_set);

rd.compute_list_bind_compute_pipeline(compute_list_id, _detail_generator_pipeline_rid);
rd.compute_list_bind_uniform_set(compute_list_id, detail_generator_uniform_set, 0);
Expand Down Expand Up @@ -402,6 +406,7 @@ void RenderDetailTextureGPUTask::prepare(GPUTaskContext &ctx) {

const RID detail_modifier_uniform_set =
uniform_set_create(rd, detail_modifier_uniforms, modifier_shader_rid, 0);
_uniform_sets_to_free.push_back(detail_modifier_uniform_set);

const RID pipeline_rid = _detail_modifier_pipelines[modifier_index];
rd.compute_list_bind_compute_pipeline(compute_list_id, pipeline_rid);
Expand Down Expand Up @@ -440,6 +445,7 @@ void RenderDetailTextureGPUTask::prepare(GPUTaskContext &ctx) {

const RID detail_normalmap_uniform_set_rid =
uniform_set_create(rd, detail_normalmap_uniforms, detail_normalmap_shader_rid, 0);
_uniform_sets_to_free.push_back(detail_normalmap_uniform_set_rid);

rd.compute_list_bind_compute_pipeline(compute_list_id, _detail_normalmap_pipeline_rid);
rd.compute_list_bind_uniform_set(compute_list_id, detail_normalmap_uniform_set_rid, 0);
Expand Down Expand Up @@ -470,6 +476,7 @@ void RenderDetailTextureGPUTask::prepare(GPUTaskContext &ctx) {
dilation_uniforms[1] = image1_uniform;
dilation_uniforms[2] = dilation_params_uniform;
const RID dilation_uniform_set_rid = uniform_set_create(rd, dilation_uniforms, dilation_shader_rid, 0);
_uniform_sets_to_free.push_back(dilation_uniform_set_rid);

rd.compute_list_bind_compute_pipeline(compute_list_id, _normalmap_dilation_pipeline_rid);
rd.compute_list_bind_uniform_set(compute_list_id, dilation_uniform_set_rid, 0);
Expand Down Expand Up @@ -506,6 +513,7 @@ void RenderDetailTextureGPUTask::prepare(GPUTaskContext &ctx) {
dilation_uniforms[2] = dilation_params_uniform;
// TODO Do I really have to create a new uniform set every time I modify just one of the passed values?
const RID dilation_uniform_set_rid = uniform_set_create(rd, dilation_uniforms, dilation_shader_rid, 0);
_uniform_sets_to_free.push_back(dilation_uniform_set_rid);

rd.compute_list_bind_uniform_set(compute_list_id, dilation_uniform_set_rid, 0);

Expand Down Expand Up @@ -550,6 +558,14 @@ PackedByteArray RenderDetailTextureGPUTask::collect_texture_and_cleanup(
free_rendering_device_rid(rd, rid);
}

// This is not documented: from dev source, uniform sets are "automatically freed by their dependencies". So
// apparently we don't *have* to free them. However, I got a bug report where RID limit got exceeded after
// several minutes, unless those RIDs get freed after use... something odd is going on, but absence of
// documentation on the automatic behavior doesn't help.
for (RID rid : _uniform_sets_to_free) {
free_rendering_device_rid(rd, rid);
}

storage_buffer_pool.recycle(_mesh_vertices_sb);
storage_buffer_pool.recycle(_mesh_indices_sb);
storage_buffer_pool.recycle(_cell_triangles_sb);
Expand Down
1 change: 1 addition & 0 deletions engine/detail_rendering/render_detail_texture_gpu_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class RenderDetailTextureGPUTask : public IGPUTask {
RID _detail_normalmap_pipeline_rid;
RID _normalmap_dilation_pipeline_rid;
StdVector<RID> _detail_modifier_pipelines;
StdVector<RID> _uniform_sets_to_free;

GPUStorageBuffer _mesh_vertices_sb;
GPUStorageBuffer _mesh_indices_sb;
Expand Down
8 changes: 8 additions & 0 deletions generators/generate_block_gpu_task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ void GenerateBlockGPUTask::prepare(GPUTaskContext &ctx) {

// Generate

_uniform_sets_to_free.reserve(_boxes_data.size() * (1 + modifiers.size()));

for (unsigned int box_index = 0; box_index < _boxes_data.size(); ++box_index) {
BoxData &bd = _boxes_data[box_index];

Expand All @@ -153,6 +155,7 @@ void GenerateBlockGPUTask::prepare(GPUTaskContext &ctx) {
// Mutex (instead of BinaryMutex)
const RID generator_uniform_set =
zylann::godot::uniform_set_create(rd, generator_uniforms, generator_shader_rid, 0);
_uniform_sets_to_free.push_back(generator_uniform_set);

{
ZN_PROFILE_SCOPE_NAMED("compute_list_bind_compute_pipeline");
Expand Down Expand Up @@ -212,6 +215,7 @@ void GenerateBlockGPUTask::prepare(GPUTaskContext &ctx) {

const RID modifier_uniform_set =
zylann::godot::uniform_set_create(rd, modifier_uniforms, modifier_shader_rid, 0);
_uniform_sets_to_free.push_back(modifier_uniform_set);

const RID pipeline_rid = _modifier_pipelines[modifier_index];
rd.compute_list_bind_compute_pipeline(compute_list_id, pipeline_rid);
Expand Down Expand Up @@ -469,6 +473,10 @@ void GenerateBlockGPUTask::collect(GPUTaskContext &ctx) {
zylann::godot::free_rendering_device_rid(rd, rid);
}

for (const RID rid : _uniform_sets_to_free) {
zylann::godot::free_rendering_device_rid(rd, rid);
}

// We leave conversion to the CPU task, because we have only one thread for GPU work and it only exists for waiting
// blocking functions, not doing work
consumer_task->set_gpu_results(std::move(results));
Expand Down
1 change: 1 addition & 0 deletions generators/generate_block_gpu_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class GenerateBlockGPUTask : public IGPUTask {
StdVector<BoxData> _boxes_data;
RID _generator_pipeline_rid;
StdVector<RID> _modifier_pipelines;
StdVector<RID> _uniform_sets_to_free;
};

} // namespace zylann::voxel
Expand Down

0 comments on commit 662086f

Please sign in to comment.