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.

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

Apply some low-hanging fruit optimizations to Vulkan RD #85532

Merged
merged 2 commits into from
Dec 8, 2023
Merged

Apply some low-hanging fruit optimizations to Vulkan RD #85532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4964d9a
Remove redundant explicit clears in the Vulkan RD
RandomShaper Nov 30, 2023
72bf8df
Remove superfluous locking in RID owners in Vulkan RD
RandomShaper Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 66 additions & 37 deletions drivers/vulkan/rendering_device_vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6841,24 +6841,24 @@ Error RenderingDeviceVulkan::_draw_list_setup_framebuffer(Framebuffer *p_framebu
return OK;
}

Error RenderingDeviceVulkan::_draw_list_render_pass_begin(Framebuffer *framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_colors, float p_clear_depth, uint32_t p_clear_stencil, Point2i viewport_offset, Point2i viewport_size, VkFramebuffer vkframebuffer, VkRenderPass render_pass, VkCommandBuffer command_buffer, VkSubpassContents subpass_contents, const Vector<RID> &p_storage_textures) {
Error RenderingDeviceVulkan::_draw_list_render_pass_begin(Framebuffer *framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_colors, float p_clear_depth, uint32_t p_clear_stencil, Point2i viewport_offset, Point2i viewport_size, VkFramebuffer vkframebuffer, VkRenderPass render_pass, VkCommandBuffer command_buffer, VkSubpassContents subpass_contents, const Vector<RID> &p_storage_textures, bool p_constrained_to_region) {
VkRenderPassBeginInfo render_pass_begin;
render_pass_begin.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO;
render_pass_begin.pNext = nullptr;
render_pass_begin.renderPass = render_pass;
render_pass_begin.framebuffer = vkframebuffer;
/*
* Given how API works, it makes sense to always fully operate on the whole framebuffer.
* This allows better continue operations for operations like shadowmapping.
render_pass_begin.renderArea.extent.width = viewport_size.width;
render_pass_begin.renderArea.extent.height = viewport_size.height;
render_pass_begin.renderArea.offset.x = viewport_offset.x;
render_pass_begin.renderArea.offset.y = viewport_offset.y;
*/
render_pass_begin.renderArea.extent.width = framebuffer->size.width;
render_pass_begin.renderArea.extent.height = framebuffer->size.height;
render_pass_begin.renderArea.offset.x = 0;
render_pass_begin.renderArea.offset.y = 0;

if (p_constrained_to_region) {
render_pass_begin.renderArea.extent.width = viewport_size.width;
render_pass_begin.renderArea.extent.height = viewport_size.height;
render_pass_begin.renderArea.offset.x = viewport_offset.x;
render_pass_begin.renderArea.offset.y = viewport_offset.y;
} else {
render_pass_begin.renderArea.extent.width = framebuffer->size.width;
render_pass_begin.renderArea.extent.height = framebuffer->size.height;
render_pass_begin.renderArea.offset.x = 0;
render_pass_begin.renderArea.offset.y = 0;
}

Vector<VkClearValue> clear_values;
clear_values.resize(framebuffer->texture_ids.size());
Expand Down Expand Up @@ -7008,6 +7008,7 @@ RenderingDevice::DrawListID RenderingDeviceVulkan::draw_list_begin(RID p_framebu

Point2i viewport_offset;
Point2i viewport_size = framebuffer->size;
bool constrained_to_region = false;
bool needs_clear_color = false;
bool needs_clear_depth = false;

Expand All @@ -7022,21 +7023,30 @@ RenderingDevice::DrawListID RenderingDeviceVulkan::draw_list_begin(RID p_framebu

viewport_offset = regioni.position;
viewport_size = regioni.size;
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION_CONTINUE) {
needs_clear_color = true;
p_initial_color_action = INITIAL_ACTION_CONTINUE;
}
if (p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION_CONTINUE) {
needs_clear_depth = true;
p_initial_depth_action = INITIAL_ACTION_CONTINUE;
}
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_color = true;
p_initial_color_action = INITIAL_ACTION_KEEP;
}
if (p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_depth = true;
p_initial_depth_action = INITIAL_ACTION_KEEP;

// If clearing regions both in color and depth, we can switch to a fast path where we let Vulkan to the clears
// and we constrain the render area to the region.
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION && p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION) {
constrained_to_region = true;
p_initial_color_action = INITIAL_ACTION_CLEAR;
p_initial_depth_action = INITIAL_ACTION_CLEAR;
} else {
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION_CONTINUE) {
needs_clear_color = true;
p_initial_color_action = INITIAL_ACTION_CONTINUE;
}
if (p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION_CONTINUE) {
needs_clear_depth = true;
p_initial_depth_action = INITIAL_ACTION_CONTINUE;
}
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_color = true;
p_initial_color_action = INITIAL_ACTION_KEEP;
}
if (p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_depth = true;
p_initial_depth_action = INITIAL_ACTION_KEEP;
}
}
}

Expand All @@ -7063,7 +7073,7 @@ RenderingDevice::DrawListID RenderingDeviceVulkan::draw_list_begin(RID p_framebu
ERR_FAIL_COND_V(err != OK, INVALID_ID);

VkCommandBuffer command_buffer = frames[frame].draw_command_buffer;
err = _draw_list_render_pass_begin(framebuffer, p_initial_color_action, p_final_color_action, p_initial_depth_action, p_final_depth_action, p_clear_color_values, p_clear_depth, p_clear_stencil, viewport_offset, viewport_size, vkframebuffer, render_pass, command_buffer, VK_SUBPASS_CONTENTS_INLINE, p_storage_textures);
err = _draw_list_render_pass_begin(framebuffer, p_initial_color_action, p_final_color_action, p_initial_depth_action, p_final_depth_action, p_clear_color_values, p_clear_depth, p_clear_stencil, viewport_offset, viewport_size, vkframebuffer, render_pass, command_buffer, VK_SUBPASS_CONTENTS_INLINE, p_storage_textures, constrained_to_region);

if (err != OK) {
return INVALID_ID;
Expand All @@ -7079,6 +7089,7 @@ RenderingDevice::DrawListID RenderingDeviceVulkan::draw_list_begin(RID p_framebu
draw_list_current_subpass = 0;

if (needs_clear_color || needs_clear_depth) {
DEV_ASSERT(!constrained_to_region);
_draw_list_insert_clear_region(draw_list, framebuffer, viewport_offset, viewport_size, needs_clear_color, p_clear_color_values, needs_clear_depth, p_clear_depth, p_clear_stencil);
}

Expand Down Expand Up @@ -7117,6 +7128,7 @@ Error RenderingDeviceVulkan::draw_list_begin_split(RID p_framebuffer, uint32_t p
Point2i viewport_offset;
Point2i viewport_size = framebuffer->size;

bool constrained_to_region = false;
bool needs_clear_color = false;
bool needs_clear_depth = false;

Expand All @@ -7132,13 +7144,29 @@ Error RenderingDeviceVulkan::draw_list_begin_split(RID p_framebuffer, uint32_t p
viewport_offset = regioni.position;
viewport_size = regioni.size;

if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_color = true;
p_initial_color_action = INITIAL_ACTION_KEEP;
}
if (p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_depth = true;
p_initial_depth_action = INITIAL_ACTION_KEEP;
// If clearing regions both in color and depth, we can switch to a fast path where we let Vulkan to the clears
// and we constrain the render area to the region.
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION && p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION) {
constrained_to_region = true;
p_initial_color_action = INITIAL_ACTION_CLEAR;
p_initial_depth_action = INITIAL_ACTION_CLEAR;
} else {
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION_CONTINUE) {
needs_clear_color = true;
p_initial_color_action = INITIAL_ACTION_CONTINUE;
}
if (p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION_CONTINUE) {
needs_clear_depth = true;
p_initial_depth_action = INITIAL_ACTION_CONTINUE;
}
if (p_initial_color_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_color = true;
p_initial_color_action = INITIAL_ACTION_KEEP;
}
if (p_initial_depth_action == INITIAL_ACTION_CLEAR_REGION) {
needs_clear_depth = true;
p_initial_depth_action = INITIAL_ACTION_KEEP;
}
}
}

Expand All @@ -7164,7 +7192,7 @@ Error RenderingDeviceVulkan::draw_list_begin_split(RID p_framebuffer, uint32_t p
ERR_FAIL_COND_V(err != OK, ERR_CANT_CREATE);

VkCommandBuffer frame_command_buffer = frames[frame].draw_command_buffer;
err = _draw_list_render_pass_begin(framebuffer, p_initial_color_action, p_final_color_action, p_initial_depth_action, p_final_depth_action, p_clear_color_values, p_clear_depth, p_clear_stencil, viewport_offset, viewport_size, vkframebuffer, render_pass, frame_command_buffer, VK_SUBPASS_CONTENTS_SECONDARY_COMMAND_BUFFERS, p_storage_textures);
err = _draw_list_render_pass_begin(framebuffer, p_initial_color_action, p_final_color_action, p_initial_depth_action, p_final_depth_action, p_clear_color_values, p_clear_depth, p_clear_stencil, viewport_offset, viewport_size, vkframebuffer, render_pass, frame_command_buffer, VK_SUBPASS_CONTENTS_SECONDARY_COMMAND_BUFFERS, p_storage_textures, constrained_to_region);

if (err != OK) {
return ERR_CANT_CREATE;
Expand All @@ -7184,6 +7212,7 @@ Error RenderingDeviceVulkan::draw_list_begin_split(RID p_framebuffer, uint32_t p
}

if (needs_clear_color || needs_clear_depth) {
DEV_ASSERT(!constrained_to_region);
_draw_list_insert_clear_region(&draw_list[0], framebuffer, viewport_offset, viewport_size, needs_clear_color, p_clear_color_values, needs_clear_depth, p_clear_depth, p_clear_stencil);
}

Expand Down
28 changes: 14 additions & 14 deletions drivers/vulkan/rendering_device_vulkan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
RID owner;
};

RID_Owner<Texture, true> texture_owner;
RID_Owner<Texture> texture_owner;
uint32_t texture_upload_region_size_px = 0;

Vector<uint8_t> _texture_get_data_from_image(Texture *tex, VkImage p_image, VmaAllocation p_allocation, uint32_t p_layer, bool p_2d = false);
Expand Down Expand Up @@ -409,7 +409,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
uint32_t view_count;
};

RID_Owner<Framebuffer, true> framebuffer_owner;
RID_Owner<Framebuffer> framebuffer_owner;

/***********************/
/**** VERTEX BUFFER ****/
Expand All @@ -424,7 +424,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
// This mapping is done here internally, and it's not
// exposed.

RID_Owner<Buffer, true> vertex_buffer_owner;
RID_Owner<Buffer> vertex_buffer_owner;

struct VertexDescriptionKey {
Vector<VertexAttribute> vertex_formats;
Expand Down Expand Up @@ -506,7 +506,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
Vector<VkDeviceSize> offsets;
};

RID_Owner<VertexArray, true> vertex_array_owner;
RID_Owner<VertexArray> vertex_array_owner;

struct IndexBuffer : public Buffer {
uint32_t max_index = 0; // Used for validation.
Expand All @@ -515,7 +515,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
bool supports_restart_indices = false;
};

RID_Owner<IndexBuffer, true> index_buffer_owner;
RID_Owner<IndexBuffer> index_buffer_owner;

struct IndexArray {
uint32_t max_index = 0; // Remember the maximum index here too, for validation.
Expand All @@ -526,7 +526,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
bool supports_restart_indices = false;
};

RID_Owner<IndexArray, true> index_array_owner;
RID_Owner<IndexArray> index_array_owner;

/****************/
/**** SHADER ****/
Expand Down Expand Up @@ -649,7 +649,7 @@ class RenderingDeviceVulkan : public RenderingDevice {

String _shader_uniform_debug(RID p_shader, int p_set = -1);

RID_Owner<Shader, true> shader_owner;
RID_Owner<Shader> shader_owner;

/******************/
/**** UNIFORMS ****/
Expand Down Expand Up @@ -713,16 +713,16 @@ class RenderingDeviceVulkan : public RenderingDevice {
DescriptorPool *_descriptor_pool_allocate(const DescriptorPoolKey &p_key);
void _descriptor_pool_free(const DescriptorPoolKey &p_key, DescriptorPool *p_pool);

RID_Owner<Buffer, true> uniform_buffer_owner;
RID_Owner<Buffer, true> storage_buffer_owner;
RID_Owner<Buffer> uniform_buffer_owner;
RID_Owner<Buffer> storage_buffer_owner;

// Texture buffer needs a view.
struct TextureBuffer {
Buffer buffer;
VkBufferView view = VK_NULL_HANDLE;
};

RID_Owner<TextureBuffer, true> texture_buffer_owner;
RID_Owner<TextureBuffer> texture_buffer_owner;

// This structure contains the descriptor set. They _need_ to be allocated
// for a shader (and will be erased when this shader is erased), but should
Expand Down Expand Up @@ -752,7 +752,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
void *invalidated_callback_userdata = nullptr;
};

RID_Owner<UniformSet, true> uniform_set_owner;
RID_Owner<UniformSet> uniform_set_owner;

/*******************/
/**** PIPELINES ****/
Expand Down Expand Up @@ -791,7 +791,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
uint32_t push_constant_stages_mask = 0;
};

RID_Owner<RenderPipeline, true> render_pipeline_owner;
RID_Owner<RenderPipeline> render_pipeline_owner;

struct PipelineCacheHeader {
uint32_t magic;
Expand Down Expand Up @@ -830,7 +830,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
uint32_t local_group_size[3] = { 0, 0, 0 };
};

RID_Owner<ComputePipeline, true> compute_pipeline_owner;
RID_Owner<ComputePipeline> compute_pipeline_owner;

/*******************/
/**** DRAW LIST ****/
Expand Down Expand Up @@ -931,7 +931,7 @@ class RenderingDeviceVulkan : public RenderingDevice {

void _draw_list_insert_clear_region(DrawList *p_draw_list, Framebuffer *p_framebuffer, Point2i p_viewport_offset, Point2i p_viewport_size, bool p_clear_color, const Vector<Color> &p_clear_colors, bool p_clear_depth, float p_depth, uint32_t p_stencil);
Error _draw_list_setup_framebuffer(Framebuffer *p_framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, VkFramebuffer *r_framebuffer, VkRenderPass *r_render_pass, uint32_t *r_subpass_count);
Error _draw_list_render_pass_begin(Framebuffer *framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_colors, float p_clear_depth, uint32_t p_clear_stencil, Point2i viewport_offset, Point2i viewport_size, VkFramebuffer vkframebuffer, VkRenderPass render_pass, VkCommandBuffer command_buffer, VkSubpassContents subpass_contents, const Vector<RID> &p_storage_textures);
Error _draw_list_render_pass_begin(Framebuffer *framebuffer, InitialAction p_initial_color_action, FinalAction p_final_color_action, InitialAction p_initial_depth_action, FinalAction p_final_depth_action, const Vector<Color> &p_clear_colors, float p_clear_depth, uint32_t p_clear_stencil, Point2i viewport_offset, Point2i viewport_size, VkFramebuffer vkframebuffer, VkRenderPass render_pass, VkCommandBuffer command_buffer, VkSubpassContents subpass_contents, const Vector<RID> &p_storage_textures, bool p_constrained_to_region);
_FORCE_INLINE_ DrawList *_get_draw_list_ptr(DrawListID p_id);
Buffer *_get_buffer_from_owner(RID p_buffer, VkPipelineStageFlags &dst_stage_mask, VkAccessFlags &dst_access, BitField<BarrierMask> p_post_barrier);
Error _draw_list_allocate(const Rect2i &p_viewport, uint32_t p_splits, uint32_t p_subpass);
Expand Down
Loading