diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 2b70f308a0..3982362453 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -42738,6 +42738,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vertex_descriptor_vk.h ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vma.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vma.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_vk.cc + ../../../flutter/LICENSE @@ -45674,6 +45676,8 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/vertex_descriptor_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/vma.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/vma.h +FILE: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.cc +FILE: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_vk.cc diff --git a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc index 3fb650d36a..2059ff11b1 100644 --- a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc +++ b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc @@ -598,6 +598,7 @@ void DlDispatcherBase::drawDiffRoundRect(const DlRoundRect& outer, PathBuilder builder; builder.AddRoundRect(outer); builder.AddRoundRect(inner); + builder.SetBounds(outer.GetBounds().Union(inner.GetBounds())); GetCanvas().DrawPath(builder.TakePath(FillType::kOdd), paint_); } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn b/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn index a750494b49..89cb5e4900 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn @@ -122,6 +122,8 @@ impeller_component("vulkan") { "vk.h", "vma.cc", "vma.h", + "workarounds_vk.cc", + "workarounds_vk.h", "yuv_conversion_library_vk.cc", "yuv_conversion_library_vk.h", "yuv_conversion_vk.cc", diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc index bce833a15c..3fb53bcce4 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -10,6 +10,7 @@ #include "impeller/base/validation.h" #include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "impeller/renderer/backend/vulkan/workarounds_vk.h" namespace impeller { @@ -482,7 +483,7 @@ bool CapabilitiesVK::HasExtension(const std::string& ext) const { } bool CapabilitiesVK::SupportsPrimitiveRestart() const { - return true; + return has_primitive_restart_; } void CapabilitiesVK::SetOffscreenFormat(PixelFormat pixel_format) const { @@ -759,4 +760,8 @@ ISize CapabilitiesVK::GetMaximumRenderPassAttachmentSize() const { return max_render_pass_attachment_size_; } +void CapabilitiesVK::ApplyWorkarounds(const WorkaroundsVK& workarounds) { + has_primitive_restart_ = !workarounds.slow_primitive_restart_performance; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h index 2f1811f54a..3a7824e22c 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -15,6 +15,7 @@ #include "impeller/base/backend_cast.h" #include "impeller/core/texture_descriptor.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "impeller/renderer/backend/vulkan/workarounds_vk.h" #include "impeller/renderer/capabilities.h" namespace impeller { @@ -281,6 +282,10 @@ class CapabilitiesVK final : public Capabilities, CompressionType compression_type, const FRCFormatDescriptor& desc) const; + //---------------------------------------------------------------------------- + /// @brief Update capabilities for the given set of workarounds. + void ApplyWorkarounds(const WorkaroundsVK& workarounds); + private: bool validations_enabled_ = false; std::map> exts_; @@ -298,6 +303,7 @@ class CapabilitiesVK final : public Capabilities, bool supports_texture_fixed_rate_compression_ = false; ISize max_render_pass_attachment_size_ = ISize{0, 0}; bool has_triangle_fans_ = true; + bool has_primitive_restart_ = true; bool is_valid_ = false; // The embedder.h API is responsible for providing the instance and device diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc index c72f944f48..027a18726c 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc @@ -12,6 +12,7 @@ #include "impeller/renderer/backend/vulkan/command_queue_vk.h" #include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" #include "impeller/renderer/backend/vulkan/render_pass_builder_vk.h" +#include "impeller/renderer/backend/vulkan/workarounds_vk.h" #include "impeller/renderer/render_target.h" #ifdef FML_OS_ANDROID @@ -457,10 +458,16 @@ void ContextVK::Setup(Settings settings) { //---------------------------------------------------------------------------- /// All done! /// + + // Apply workarounds for broken drivers. + auto driver_info = + std::make_unique(device_holder->physical_device); + WorkaroundsVK workarounds = GetWorkarounds(*driver_info); + caps->ApplyWorkarounds(workarounds); + device_holder_ = std::move(device_holder); idle_waiter_vk_ = std::make_shared(device_holder_); - driver_info_ = - std::make_unique(device_holder_->physical_device); + driver_info_ = std::move(driver_info); debug_report_ = std::move(debug_report); allocator_ = std::move(allocator); shader_library_ = std::move(shader_library); @@ -477,7 +484,7 @@ void ContextVK::Setup(Settings settings) { device_name_ = std::string(physical_device_properties.deviceName); command_queue_vk_ = std::make_shared(weak_from_this()); should_disable_surface_control_ = settings.disable_surface_control; - should_batch_cmd_buffers_ = driver_info_->CanBatchSubmitCommandBuffers(); + should_batch_cmd_buffers_ = !workarounds.batch_submit_command_buffer_timeout; is_valid_ = true; // Create the GPU Tracer later because it depends on state from diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc index a53c456560..45b4732642 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc @@ -317,12 +317,6 @@ void DriverInfoVK::DumpToLog() const { FML_LOG(IMPORTANT) << stream.str(); } -bool DriverInfoVK::CanBatchSubmitCommandBuffers() const { - return vendor_ == VendorVK::kARM || - (adreno_gpu_.has_value() && - adreno_gpu_.value() >= AdrenoGPU::kAdreno702); -} - bool DriverInfoVK::IsEmulator() const { #if FML_OS_ANDROID // Google SwiftShader on Android. @@ -358,4 +352,12 @@ bool DriverInfoVK::IsKnownBadDriver() const { return false; } +std::optional DriverInfoVK::GetMaliGPUInfo() const { + return mali_gpu_; +} + +std::optional DriverInfoVK::GetAdrenoGPUInfo() const { + return adreno_gpu_; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h index 5836118ff4..495b8f9a41 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h @@ -239,17 +239,16 @@ class DriverInfoVK { bool IsKnownBadDriver() const; //---------------------------------------------------------------------------- - /// @brief Determines if the driver can batch submit command buffers - /// without triggering erronious deadlock errors. + /// @brief Returns Mali GPU info if this is a Mali GPU, otherwise + /// std::nullopt. /// - /// Early 600 series Adreno drivers would deadlock if a command - /// buffer submission had too much work attached to it, this - /// requires the renderer to split up command buffers that could - /// be logically combined. + std::optional GetMaliGPUInfo() const; + + //---------------------------------------------------------------------------- + /// @brief Returns Adreno GPU info if this is a Adreno GPU, otherwise + /// std::nullopt. /// - /// @return True if device can batch submit command buffers. - /// - bool CanBatchSubmitCommandBuffers() const; + std::optional GetAdrenoGPUInfo() const; private: bool is_valid_ = false; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk_unittests.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk_unittests.cc index c1b40ed98e..b3daa2a953 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk_unittests.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk_unittests.cc @@ -82,7 +82,8 @@ bool CanBatchSubmitTest(std::string_view driver_name, bool qc = true) { prop->deviceType = VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU; }) .Build(); - return context->GetDriverInfo()->CanBatchSubmitCommandBuffers(); + return !GetWorkarounds(*context->GetDriverInfo()) + .batch_submit_command_buffer_timeout; } TEST(DriverInfoVKTest, CanBatchSubmitCommandBuffers) { @@ -93,6 +94,35 @@ TEST(DriverInfoVKTest, CanBatchSubmitCommandBuffers) { EXPECT_TRUE(CanBatchSubmitTest("Adreno (TM) 750", true)); } +bool CanUsePrimitiveRestartSubmitTest(std::string_view driver_name, + bool qc = true) { + auto const context = + MockVulkanContextBuilder() + .SetPhysicalPropertiesCallback( + [&driver_name, qc](VkPhysicalDevice device, + VkPhysicalDeviceProperties* prop) { + if (qc) { + prop->vendorID = 0x168C; // Qualcomm + } else { + prop->vendorID = 0x13B5; // ARM + } + driver_name.copy(prop->deviceName, driver_name.size()); + prop->deviceType = VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU; + }) + .Build(); + return !GetWorkarounds(*context->GetDriverInfo()) + .slow_primitive_restart_performance; +} + +TEST(DriverInfoVKTest, CanUsePrimitiveRestart) { + // Adreno no primitive restart + EXPECT_FALSE(CanUsePrimitiveRestartSubmitTest("Adreno (TM) 540", true)); + EXPECT_FALSE(CanUsePrimitiveRestartSubmitTest("Adreno (TM) 750", true)); + + // Mali A-OK + EXPECT_TRUE(CanUsePrimitiveRestartSubmitTest("Mali-G51", false)); +} + TEST(DriverInfoVKTest, DriverParsingMali) { EXPECT_EQ(GetMaliVersion("Mali-G51-MORE STUFF"), MaliGPU::kG51); EXPECT_EQ(GetMaliVersion("Mali-G51"), MaliGPU::kG51); diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc index 4ad8384982..618f333488 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc @@ -362,6 +362,7 @@ fml::StatusOr MakePipeline( const auto topology = ToVKPrimitiveTopology(desc.GetPrimitiveType()); input_assembly.setTopology(topology); input_assembly.setPrimitiveRestartEnable( + caps->SupportsPrimitiveRestart() && PrimitiveTopologySupportsPrimitiveRestart(desc.GetPrimitiveType())); pipeline_info.setPInputAssemblyState(&input_assembly); diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/workarounds_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/workarounds_vk.cc new file mode 100644 index 0000000000..d77ef7d1bb --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/workarounds_vk.cc @@ -0,0 +1,28 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/renderer/backend/vulkan/workarounds_vk.h" + +namespace impeller { + +WorkaroundsVK GetWorkarounds(DriverInfoVK& driver_info) { + WorkaroundsVK workarounds; + + const auto& adreno_gpu = driver_info.GetAdrenoGPUInfo(); + const auto& mali_gpu = driver_info.GetMaliGPUInfo(); + + workarounds.batch_submit_command_buffer_timeout = true; + if (adreno_gpu.has_value()) { + workarounds.slow_primitive_restart_performance = true; + + if (adreno_gpu.value() >= AdrenoGPU::kAdreno702) { + workarounds.batch_submit_command_buffer_timeout = false; + } + } else if (mali_gpu.has_value()) { + workarounds.batch_submit_command_buffer_timeout = false; + } + return workarounds; +} + +} // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/workarounds_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/workarounds_vk.h new file mode 100644 index 0000000000..72e8166c26 --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/workarounds_vk.h @@ -0,0 +1,30 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_WORKAROUNDS_VK_H_ +#define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_WORKAROUNDS_VK_H_ + +#include "impeller/renderer/backend/vulkan/driver_info_vk.h" + +namespace impeller { + +/// A non-exhaustive set of driver specific workarounds. +struct WorkaroundsVK { + // Adreno GPUs exhibit terrible performance when primitive + // restart is used. This was confirmed up to Adreno 640 (Pixel 4). + // Because this feature is fairly marginal, we disable it for _all_ + // Adreno GPUs until we have an upper bound for this bug. + bool slow_primitive_restart_performance = false; + + /// Early 600 series Adreno drivers would deadlock if a command + /// buffer submission had too much work attached to it, this + /// requires the renderer to split up command buffers that could + /// be logically combined. + bool batch_submit_command_buffer_timeout = false; +}; +WorkaroundsVK GetWorkarounds(DriverInfoVK& driver_info); + +} // namespace impeller + +#endif // FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_WORKAROUNDS_VK_H_