Skip to content

Commit 5e8b624

Browse files
author
Jonah Williams
authored
[Impeller] workarounds for slow Adreno primitive restart performance. (#160683)
Fixes flutter/flutter#160593 Primitive Restart cannot be used on some (All?) Adreno's because it causes a dramatic performance regression. Opt out and use the GLES strategy. Refactors the batch submit command buffer capability into workarounds_vk
1 parent 4c8b0a3 commit 5e8b624

File tree

12 files changed

+135
-20
lines changed

12 files changed

+135
-20
lines changed

‎engine/src/flutter/ci/licenses_golden/licenses_flutter‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42738,6 +42738,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vertex_descriptor_vk.h
4273842738
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vk.h + ../../../flutter/LICENSE
4273942739
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vma.cc + ../../../flutter/LICENSE
4274042740
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/vma.h + ../../../flutter/LICENSE
42741+
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.cc + ../../../flutter/LICENSE
42742+
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.h + ../../../flutter/LICENSE
4274142743
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.cc + ../../../flutter/LICENSE
4274242744
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.h + ../../../flutter/LICENSE
4274342745
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
4567445676
FILE: ../../../flutter/impeller/renderer/backend/vulkan/vk.h
4567545677
FILE: ../../../flutter/impeller/renderer/backend/vulkan/vma.cc
4567645678
FILE: ../../../flutter/impeller/renderer/backend/vulkan/vma.h
45679+
FILE: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.cc
45680+
FILE: ../../../flutter/impeller/renderer/backend/vulkan/workarounds_vk.h
4567745681
FILE: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.cc
4567845682
FILE: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_library_vk.h
4567945683
FILE: ../../../flutter/impeller/renderer/backend/vulkan/yuv_conversion_vk.cc

‎engine/src/flutter/impeller/display_list/dl_dispatcher.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ void DlDispatcherBase::drawDiffRoundRect(const DlRoundRect& outer,
598598
PathBuilder builder;
599599
builder.AddRoundRect(outer);
600600
builder.AddRoundRect(inner);
601+
builder.SetBounds(outer.GetBounds().Union(inner.GetBounds()));
601602
GetCanvas().DrawPath(builder.TakePath(FillType::kOdd), paint_);
602603
}
603604

‎engine/src/flutter/impeller/renderer/backend/vulkan/BUILD.gn‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ impeller_component("vulkan") {
122122
"vk.h",
123123
"vma.cc",
124124
"vma.h",
125+
"workarounds_vk.cc",
126+
"workarounds_vk.h",
125127
"yuv_conversion_library_vk.cc",
126128
"yuv_conversion_library_vk.h",
127129
"yuv_conversion_vk.cc",

‎engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "impeller/base/validation.h"
1111
#include "impeller/core/formats.h"
1212
#include "impeller/renderer/backend/vulkan/vk.h"
13+
#include "impeller/renderer/backend/vulkan/workarounds_vk.h"
1314

1415
namespace impeller {
1516

@@ -482,7 +483,7 @@ bool CapabilitiesVK::HasExtension(const std::string& ext) const {
482483
}
483484

484485
bool CapabilitiesVK::SupportsPrimitiveRestart() const {
485-
return true;
486+
return has_primitive_restart_;
486487
}
487488

488489
void CapabilitiesVK::SetOffscreenFormat(PixelFormat pixel_format) const {
@@ -759,4 +760,8 @@ ISize CapabilitiesVK::GetMaximumRenderPassAttachmentSize() const {
759760
return max_render_pass_attachment_size_;
760761
}
761762

763+
void CapabilitiesVK::ApplyWorkarounds(const WorkaroundsVK& workarounds) {
764+
has_primitive_restart_ = !workarounds.slow_primitive_restart_performance;
765+
}
766+
762767
} // namespace impeller

‎engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "impeller/base/backend_cast.h"
1616
#include "impeller/core/texture_descriptor.h"
1717
#include "impeller/renderer/backend/vulkan/vk.h"
18+
#include "impeller/renderer/backend/vulkan/workarounds_vk.h"
1819
#include "impeller/renderer/capabilities.h"
1920

2021
namespace impeller {
@@ -281,6 +282,10 @@ class CapabilitiesVK final : public Capabilities,
281282
CompressionType compression_type,
282283
const FRCFormatDescriptor& desc) const;
283284

285+
//----------------------------------------------------------------------------
286+
/// @brief Update capabilities for the given set of workarounds.
287+
void ApplyWorkarounds(const WorkaroundsVK& workarounds);
288+
284289
private:
285290
bool validations_enabled_ = false;
286291
std::map<std::string, std::set<std::string>> exts_;
@@ -298,6 +303,7 @@ class CapabilitiesVK final : public Capabilities,
298303
bool supports_texture_fixed_rate_compression_ = false;
299304
ISize max_render_pass_attachment_size_ = ISize{0, 0};
300305
bool has_triangle_fans_ = true;
306+
bool has_primitive_restart_ = true;
301307
bool is_valid_ = false;
302308

303309
// The embedder.h API is responsible for providing the instance and device

‎engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "impeller/renderer/backend/vulkan/command_queue_vk.h"
1313
#include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h"
1414
#include "impeller/renderer/backend/vulkan/render_pass_builder_vk.h"
15+
#include "impeller/renderer/backend/vulkan/workarounds_vk.h"
1516
#include "impeller/renderer/render_target.h"
1617

1718
#ifdef FML_OS_ANDROID
@@ -457,10 +458,16 @@ void ContextVK::Setup(Settings settings) {
457458
//----------------------------------------------------------------------------
458459
/// All done!
459460
///
461+
462+
// Apply workarounds for broken drivers.
463+
auto driver_info =
464+
std::make_unique<DriverInfoVK>(device_holder->physical_device);
465+
WorkaroundsVK workarounds = GetWorkarounds(*driver_info);
466+
caps->ApplyWorkarounds(workarounds);
467+
460468
device_holder_ = std::move(device_holder);
461469
idle_waiter_vk_ = std::make_shared<IdleWaiterVK>(device_holder_);
462-
driver_info_ =
463-
std::make_unique<DriverInfoVK>(device_holder_->physical_device);
470+
driver_info_ = std::move(driver_info);
464471
debug_report_ = std::move(debug_report);
465472
allocator_ = std::move(allocator);
466473
shader_library_ = std::move(shader_library);
@@ -477,7 +484,7 @@ void ContextVK::Setup(Settings settings) {
477484
device_name_ = std::string(physical_device_properties.deviceName);
478485
command_queue_vk_ = std::make_shared<CommandQueueVK>(weak_from_this());
479486
should_disable_surface_control_ = settings.disable_surface_control;
480-
should_batch_cmd_buffers_ = driver_info_->CanBatchSubmitCommandBuffers();
487+
should_batch_cmd_buffers_ = !workarounds.batch_submit_command_buffer_timeout;
481488
is_valid_ = true;
482489

483490
// Create the GPU Tracer later because it depends on state from

‎engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.cc‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,6 @@ void DriverInfoVK::DumpToLog() const {
317317
FML_LOG(IMPORTANT) << stream.str();
318318
}
319319

320-
bool DriverInfoVK::CanBatchSubmitCommandBuffers() const {
321-
return vendor_ == VendorVK::kARM ||
322-
(adreno_gpu_.has_value() &&
323-
adreno_gpu_.value() >= AdrenoGPU::kAdreno702);
324-
}
325-
326320
bool DriverInfoVK::IsEmulator() const {
327321
#if FML_OS_ANDROID
328322
// Google SwiftShader on Android.
@@ -358,4 +352,12 @@ bool DriverInfoVK::IsKnownBadDriver() const {
358352
return false;
359353
}
360354

355+
std::optional<MaliGPU> DriverInfoVK::GetMaliGPUInfo() const {
356+
return mali_gpu_;
357+
}
358+
359+
std::optional<AdrenoGPU> DriverInfoVK::GetAdrenoGPUInfo() const {
360+
return adreno_gpu_;
361+
}
362+
361363
} // namespace impeller

‎engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk.h‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -239,17 +239,16 @@ class DriverInfoVK {
239239
bool IsKnownBadDriver() const;
240240

241241
//----------------------------------------------------------------------------
242-
/// @brief Determines if the driver can batch submit command buffers
243-
/// without triggering erronious deadlock errors.
242+
/// @brief Returns Mali GPU info if this is a Mali GPU, otherwise
243+
/// std::nullopt.
244244
///
245-
/// Early 600 series Adreno drivers would deadlock if a command
246-
/// buffer submission had too much work attached to it, this
247-
/// requires the renderer to split up command buffers that could
248-
/// be logically combined.
249-
///
250-
/// @return True if device can batch submit command buffers.
245+
std::optional<MaliGPU> GetMaliGPUInfo() const;
246+
247+
//----------------------------------------------------------------------------
248+
/// @brief Returns Adreno GPU info if this is a Adreno GPU, otherwise
249+
/// std::nullopt.
251250
///
252-
bool CanBatchSubmitCommandBuffers() const;
251+
std::optional<AdrenoGPU> GetAdrenoGPUInfo() const;
253252

254253
private:
255254
bool is_valid_ = false;

‎engine/src/flutter/impeller/renderer/backend/vulkan/driver_info_vk_unittests.cc‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ bool CanBatchSubmitTest(std::string_view driver_name, bool qc = true) {
8282
prop->deviceType = VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU;
8383
})
8484
.Build();
85-
return context->GetDriverInfo()->CanBatchSubmitCommandBuffers();
85+
return !GetWorkarounds(*context->GetDriverInfo())
86+
.batch_submit_command_buffer_timeout;
8687
}
8788

8889
TEST(DriverInfoVKTest, CanBatchSubmitCommandBuffers) {
@@ -93,6 +94,35 @@ TEST(DriverInfoVKTest, CanBatchSubmitCommandBuffers) {
9394
EXPECT_TRUE(CanBatchSubmitTest("Adreno (TM) 750", true));
9495
}
9596

97+
bool CanUsePrimitiveRestartSubmitTest(std::string_view driver_name,
98+
bool qc = true) {
99+
auto const context =
100+
MockVulkanContextBuilder()
101+
.SetPhysicalPropertiesCallback(
102+
[&driver_name, qc](VkPhysicalDevice device,
103+
VkPhysicalDeviceProperties* prop) {
104+
if (qc) {
105+
prop->vendorID = 0x168C; // Qualcomm
106+
} else {
107+
prop->vendorID = 0x13B5; // ARM
108+
}
109+
driver_name.copy(prop->deviceName, driver_name.size());
110+
prop->deviceType = VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU;
111+
})
112+
.Build();
113+
return !GetWorkarounds(*context->GetDriverInfo())
114+
.slow_primitive_restart_performance;
115+
}
116+
117+
TEST(DriverInfoVKTest, CanUsePrimitiveRestart) {
118+
// Adreno no primitive restart
119+
EXPECT_FALSE(CanUsePrimitiveRestartSubmitTest("Adreno (TM) 540", true));
120+
EXPECT_FALSE(CanUsePrimitiveRestartSubmitTest("Adreno (TM) 750", true));
121+
122+
// Mali A-OK
123+
EXPECT_TRUE(CanUsePrimitiveRestartSubmitTest("Mali-G51", false));
124+
}
125+
96126
TEST(DriverInfoVKTest, DriverParsingMali) {
97127
EXPECT_EQ(GetMaliVersion("Mali-G51-MORE STUFF"), MaliGPU::kG51);
98128
EXPECT_EQ(GetMaliVersion("Mali-G51"), MaliGPU::kG51);

‎engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@ fml::StatusOr<vk::UniquePipeline> MakePipeline(
362362
const auto topology = ToVKPrimitiveTopology(desc.GetPrimitiveType());
363363
input_assembly.setTopology(topology);
364364
input_assembly.setPrimitiveRestartEnable(
365+
caps->SupportsPrimitiveRestart() &&
365366
PrimitiveTopologySupportsPrimitiveRestart(desc.GetPrimitiveType()));
366367
pipeline_info.setPInputAssemblyState(&input_assembly);
367368

0 commit comments

Comments
 (0)