Vulkan: Fix ´vkCmdDrawIndexed´ crash on Adreno 5XX devices

Co-Authored-By: Clay John <[email protected]> Co-Authored-By: Darío <[email protected]>
godotengine · Jul 31, 2024 · 9f47b1e · 9f47b1e
1 parent 705b7a0
commit 9f47b1e
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 1 deletion.
diff --git a/drivers/vulkan/rendering_context_driver_vulkan.cpp b/drivers/vulkan/rendering_context_driver_vulkan.cpp
@@ -536,11 +536,36 @@ void RenderingContextDriverVulkan::_check_driver_workarounds(const VkPhysicalDev
 	//
 	// This bug was fixed in driver version 512.503.0, so we only enabled it on devices older than this.
 	//
+	//------------------------------------------------------------------------
+	//TODO: Godot should not use 'VK_MAKE_VERSION' but rather its own function
+	//https://github.com/godotengine/godot/pull/91514#issuecomment-2261019958
 	r_device.workarounds.avoid_compute_after_draw =
 			r_device.vendor == VENDOR_QUALCOMM &&
 			p_device_properties.deviceID >= 0x6000000 && // Adreno 6xx
 			p_device_properties.driverVersion < VK_MAKE_VERSION(512, 503, 0) &&
 			r_device.name.find("Turnip") < 0;
+
+	// Workaround for the Adreno 5XX family of devices.
+	// Various crashes at 'vkCmdDrawIndexed'.
+	// 'RENDER_GRAPH_REORDER' could not show any performance advantages on low devices.  
+	r_device.workarounds.avoid_render_graph_reorder =
+			r_device.vendor == VENDOR_QUALCOMM &&
+			p_device_properties.deviceID >= 0x5000000 && // Adreno 5xx
+			p_device_properties.deviceID <= 0x5999999;
+
+#ifdef ANDROID_ENABLED
+	print_line("============ Workarounds ============");
+	print_line("avoid_compute_after_draw: ", r_device.workarounds.avoid_compute_after_draw);
+	print_line("avoid_render_graph_reorder: ", r_device.workarounds.avoid_render_graph_reorder);
+	print_line("-------------------------------------");
+	print_line("name: ", r_device.name);
+	print_line("vendor: ", r_device.vendor);
+	print_line("deviceID: ", p_device_properties.deviceID);
+	uint32_t major = VK_VERSION_MAJOR(p_device_properties.driverVersion);
+	uint32_t minor = VK_VERSION_MINOR(p_device_properties.driverVersion);
+	uint32_t patch = VK_VERSION_PATCH(p_device_properties.driverVersion);
+	print_line("driverVersion: ", vformat("%d.%d.%d", major, minor, patch));
+#endif // ANDROID_ENABLED
 }
 
 bool RenderingContextDriverVulkan::_use_validation_layers() const {

diff --git a/servers/rendering/rendering_context_driver.h b/servers/rendering/rendering_context_driver.h
@@ -75,6 +75,7 @@ class RenderingContextDriver {
 
 	struct Workarounds {
 		bool avoid_compute_after_draw = false;
+		bool avoid_render_graph_reorder = false;
 	};
 
 	struct Device {

diff --git a/servers/rendering/rendering_device.cpp b/servers/rendering/rendering_device.cpp
@@ -5244,7 +5244,7 @@ void RenderingDevice::_end_frame() {
 
 	// The command buffer must be copied into a stack variable as the driver workarounds can change the command buffer in use.
 	RDD::CommandBufferID command_buffer = frames[frame].draw_command_buffer;
-	draw_graph.end(RENDER_GRAPH_REORDER, RENDER_GRAPH_FULL_BARRIERS, command_buffer, frames[frame].command_buffer_pool);
+	draw_graph.end(render_graph_reorder, RENDER_GRAPH_FULL_BARRIERS, command_buffer, frames[frame].command_buffer_pool);
 	driver->command_buffer_end(command_buffer);
 	driver->end_segment();
 }
@@ -6560,6 +6560,11 @@ RenderingDevice::RenderingDevice() {
 	if (singleton == nullptr) {
 		singleton = this;
 	}
+
+	render_graph_reorder = RENDER_GRAPH_REORDER;
+	if (get_device_workarounds().avoid_render_graph_reorder) {
+		render_graph_reorder = false;
+	}
 }
 
 /*****************/

diff --git a/servers/rendering/rendering_device.h b/servers/rendering/rendering_device.h
@@ -159,6 +159,7 @@ class RenderingDevice : public RenderingDeviceCommons {
 	uint32_t staging_buffer_block_size = 0;
 	uint64_t staging_buffer_max_size = 0;
 	bool staging_buffer_used = false;
+	bool render_graph_reorder = true; // will be overwritten in the constructor, see description for '#define RENDER_GRAPH_REORDER'
 
 	enum StagingRequiredAction {
 		STAGING_REQUIRED_ACTION_NONE,
@@ -830,6 +831,7 @@ class RenderingDevice : public RenderingDeviceCommons {
 public:
 	RenderingContextDriver *get_context_driver() const { return context; }
 
+	const RenderingContextDriver::Workarounds &get_device_workarounds() const { return device.workarounds; };
 	const RDD::Capabilities &get_device_capabilities() const { return driver->get_capabilities(); }
 
 	bool has_feature(const Features p_feature) const;