Merge pull request #85532 from RandomShaper/kill_redund_clear

Apply some low-hanging fruit optimizations to Vulkan RD

2 files changed, 80 insertions, 51 deletions

diff --git a/drivers/vulkan/rendering_device_vulkan.cpp b/drivers/vulkan/rendering_device_vulkan.cpp
index 23d5be869f..04aeac2bfc 100644
--- a/drivers/vulkan/rendering_device_vulkan.cpp
+++ b/drivers/vulkan/rendering_device_vulkan.cpp
@@ -6844,24 +6844,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());
@@ -7011,6 +7011,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;
 
@@ -7025,21 +7026,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;
+			}
 		}
 	}
 
@@ -7066,7 +7076,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;
@@ -7082,6 +7092,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);
 	}
 
@@ -7120,6 +7131,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;
 
@@ -7135,13 +7147,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;
+			}
 		}
 	}
 
@@ -7167,7 +7195,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;
@@ -7187,6 +7215,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);
 	}
 
diff --git a/drivers/vulkan/rendering_device_vulkan.h b/drivers/vulkan/rendering_device_vulkan.h
index 4480cbd718..e8ad0e4f45 100644
--- a/drivers/vulkan/rendering_device_vulkan.h
+++ b/drivers/vulkan/rendering_device_vulkan.h
@@ -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);
@@ -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 ****/
@@ -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;
@@ -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.
@@ -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.
@@ -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 ****/
@@ -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 ****/
@@ -713,8 +713,8 @@ 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 {
@@ -722,7 +722,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
 		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
@@ -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 ****/
@@ -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;
@@ -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 ****/
@@ -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);