From 7cc21c27d71d2419038ff59db75c857223eb6037 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 3 Aug 2023 15:40:40 -0700 Subject: [PATCH 1/7] Revert "[Impeller] Treat SubOptimalKHR as rotated. (#43214)" This reverts commit e32f60ad092202e19791e258408257bf42770867. --- .../backend/vulkan/swapchain_impl_vk.cc | 24 ++++--------------- .../backend/vulkan/swapchain_impl_vk.h | 7 ------ .../renderer/backend/vulkan/swapchain_vk.cc | 5 +--- 3 files changed, 5 insertions(+), 31 deletions(-) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 504f2a538bce1..d6ef9b1a73e76 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -124,15 +124,13 @@ static std::optional ChoosePresentQueue( std::shared_ptr SwapchainImplVK::Create( const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - bool was_rotated, vk::SwapchainKHR old_swapchain) { - return std::shared_ptr(new SwapchainImplVK( - context, std::move(surface), was_rotated, old_swapchain)); + return std::shared_ptr( + new SwapchainImplVK(context, std::move(surface), old_swapchain)); } SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - bool was_rotated, vk::SwapchainKHR old_swapchain) { if (!context) { return; @@ -278,8 +276,6 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, synchronizers_ = std::move(synchronizers); current_frame_ = synchronizers_.size() - 1u; is_valid_ = true; - was_rotated_ = was_rotated; - is_rotated_ = was_rotated; } SwapchainImplVK::~SwapchainImplVK() { @@ -321,10 +317,6 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { return {}; } - if (was_rotated_ != is_rotated_) { - return AcquireResult{true /* out of date */}; - } - const auto& context = ContextVK::Cast(*context_strong); current_frame_ = (current_frame_ + 1u) % synchronizers_.size(); @@ -349,12 +341,8 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { nullptr // fence ); - if (acq_result == vk::Result::eSuboptimalKHR) { - is_rotated_ = true; - return AcquireResult{true /* out of date */}; - } - - if (acq_result == vk::Result::eErrorOutOfDateKHR) { + if (acq_result == vk::Result::eSuboptimalKHR || + acq_result == vk::Result::eErrorOutOfDateKHR) { return AcquireResult{true /* out of date */}; } @@ -470,10 +458,6 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, // complete successfully. [[fallthrough]]; case vk::Result::eSuccess: - is_rotated_ = false; - return; - case vk::Result::eSuboptimalKHR: - is_rotated_ = true; return; default: VALIDATION_LOG << "Could not present queue: " diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.h b/impeller/renderer/backend/vulkan/swapchain_impl_vk.h index 7f061971acc1b..0df1a2f7a200c 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.h @@ -31,7 +31,6 @@ class SwapchainImplVK final static std::shared_ptr Create( const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - bool was_rotated, vk::SwapchainKHR old_swapchain = VK_NULL_HANDLE); ~SwapchainImplVK(); @@ -48,8 +47,6 @@ class SwapchainImplVK final : surface(std::move(p_surface)) {} }; - bool GetIsRotated() const { return is_rotated_; } - AcquireResult AcquireNextDrawable(); vk::Format GetSurfaceFormat() const; @@ -69,12 +66,8 @@ class SwapchainImplVK final size_t current_frame_ = 0u; bool is_valid_ = false; - bool was_rotated_ = false; - bool is_rotated_ = false; - SwapchainImplVK(const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - bool was_rotated, vk::SwapchainKHR old_swapchain); bool Present(const std::shared_ptr& image, uint32_t index); diff --git a/impeller/renderer/backend/vulkan/swapchain_vk.cc b/impeller/renderer/backend/vulkan/swapchain_vk.cc index ee958a44003c8..0bc6f9aca2ed3 100644 --- a/impeller/renderer/backend/vulkan/swapchain_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_vk.cc @@ -12,8 +12,7 @@ namespace impeller { std::shared_ptr SwapchainVK::Create( const std::shared_ptr& context, vk::UniqueSurfaceKHR surface) { - auto impl = SwapchainImplVK::Create(context, std::move(surface), - /*was_rotated=*/false); + auto impl = SwapchainImplVK::Create(context, std::move(surface)); if (!impl || !impl->IsValid()) { return nullptr; } @@ -46,12 +45,10 @@ std::unique_ptr SwapchainVK::AcquireNextDrawable() { // This swapchain implementation indicates that it is out of date. Tear it // down and make a new one. auto context = impl_->GetContext(); - auto was_rotated = impl_->GetIsRotated(); auto [surface, old_swapchain] = impl_->DestroySwapchain(); auto new_impl = SwapchainImplVK::Create(context, // std::move(surface), // - was_rotated, // *old_swapchain // ); if (!new_impl || !new_impl->IsValid()) { From 5cf84775a1ae7172b4eb01fed208f4356c6f0ec1 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Aug 2023 13:40:25 -0700 Subject: [PATCH 2/7] Update malioc. --- impeller/tools/malioc.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/impeller/tools/malioc.json b/impeller/tools/malioc.json index 49e15fa8ff1db..a846fba9c456b 100644 --- a/impeller/tools/malioc.json +++ b/impeller/tools/malioc.json @@ -2147,9 +2147,9 @@ "varying" ], "shortest_path_cycles": [ - 0.1875, + 0.21875, 0.0625, - 0.1875, + 0.21875, 0.0, 0.0, 0.25, @@ -2159,10 +2159,10 @@ "load_store" ], "total_cycles": [ - 1.3125, - 0.8125, - 1.3125, - 0.375, + 1.4874999523162842, + 0.875, + 1.4874999523162842, + 0.875, 4.0, 0.25, 0.0 @@ -2170,8 +2170,8 @@ }, "stack_spill_bytes": 0, "thread_occupancy": 100, - "uniform_registers_used": 36, - "work_registers_used": 15 + "uniform_registers_used": 46, + "work_registers_used": 20 } } } From af80ca9c50cc95b814f1a9e47114c6a3ba642d8b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Aug 2023 13:42:12 -0700 Subject: [PATCH 3/7] Revert "Update malioc." This reverts commit 5cf84775a1ae7172b4eb01fed208f4356c6f0ec1. --- impeller/tools/malioc.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/impeller/tools/malioc.json b/impeller/tools/malioc.json index a846fba9c456b..49e15fa8ff1db 100644 --- a/impeller/tools/malioc.json +++ b/impeller/tools/malioc.json @@ -2147,9 +2147,9 @@ "varying" ], "shortest_path_cycles": [ - 0.21875, + 0.1875, 0.0625, - 0.21875, + 0.1875, 0.0, 0.0, 0.25, @@ -2159,10 +2159,10 @@ "load_store" ], "total_cycles": [ - 1.4874999523162842, - 0.875, - 1.4874999523162842, - 0.875, + 1.3125, + 0.8125, + 1.3125, + 0.375, 4.0, 0.25, 0.0 @@ -2170,8 +2170,8 @@ }, "stack_spill_bytes": 0, "thread_occupancy": 100, - "uniform_registers_used": 46, - "work_registers_used": 20 + "uniform_registers_used": 36, + "work_registers_used": 15 } } } From 7fcdb7fb27025eb2f7f851f129b891d2e42ce4d8 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Aug 2023 15:22:50 -0700 Subject: [PATCH 4/7] Implement polling per-frame. --- .../backend/vulkan/swapchain_impl_vk.cc | 45 +++++++++++++++---- .../backend/vulkan/swapchain_impl_vk.h | 12 ++++- .../renderer/backend/vulkan/swapchain_vk.cc | 7 +-- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index d6ef9b1a73e76..252b11c795124 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -10,10 +10,12 @@ #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/surface_vk.h" #include "impeller/renderer/backend/vulkan/swapchain_image_vk.h" +#include "vulkan/vulkan_structs.hpp" namespace impeller { static constexpr size_t kMaxFramesInFlight = 3u; +static constexpr size_t kPollFramesForOrientation = 1u; struct FrameSynchronizer { vk::UniqueFence acquire; @@ -124,14 +126,17 @@ static std::optional ChoosePresentQueue( std::shared_ptr SwapchainImplVK::Create( const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - vk::SwapchainKHR old_swapchain) { - return std::shared_ptr( - new SwapchainImplVK(context, std::move(surface), old_swapchain)); + vk::SwapchainKHR old_swapchain, + vk::SurfaceTransformFlagBitsKHR last_transform) { + return std::shared_ptr(new SwapchainImplVK( + context, std::move(surface), old_swapchain, last_transform)); } -SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, - vk::UniqueSurfaceKHR surface, - vk::SwapchainKHR old_swapchain) { +SwapchainImplVK::SwapchainImplVK( + const std::shared_ptr& context, + vk::UniqueSurfaceKHR surface, + vk::SwapchainKHR old_swapchain, + vk::SurfaceTransformFlagBitsKHR last_transform) { if (!context) { return; } @@ -276,6 +281,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, synchronizers_ = std::move(synchronizers); current_frame_ = synchronizers_.size() - 1u; is_valid_ = true; + transform_if_changed_discard_swapchain_ = last_transform; } SwapchainImplVK::~SwapchainImplVK() { @@ -307,6 +313,10 @@ vk::Format SwapchainImplVK::GetSurfaceFormat() const { return surface_format_; } +vk::SurfaceTransformFlagBitsKHR SwapchainImplVK::GetLastTransform() const { + return transform_if_changed_discard_swapchain_; +} + std::shared_ptr SwapchainImplVK::GetContext() const { return context_.lock(); } @@ -331,6 +341,26 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { return {}; } + //---------------------------------------------------------------------------- + /// Poll to see if the orientation has changed. + /// + /// https://developer.android.com/games/optimize/vulkan-prerotation#using_polling + current_transform_poll_count_++; + if (current_transform_poll_count_ >= kPollFramesForOrientation) { + current_transform_poll_count_ = 0u; + auto [caps_result, caps] = + context.GetPhysicalDevice().getSurfaceCapabilitiesKHR(*surface_); + if (caps_result != vk::Result::eSuccess) { + VALIDATION_LOG << "Could not get surface capabilities: " + << vk::to_string(caps_result); + return {}; + } + if (caps.currentTransform != transform_if_changed_discard_swapchain_) { + transform_if_changed_discard_swapchain_ = caps.currentTransform; + return AcquireResult{true /* out of date */}; + } + } + //---------------------------------------------------------------------------- /// Get the next image index. /// @@ -341,8 +371,7 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { nullptr // fence ); - if (acq_result == vk::Result::eSuboptimalKHR || - acq_result == vk::Result::eErrorOutOfDateKHR) { + if (acq_result == vk::Result::eErrorOutOfDateKHR) { return AcquireResult{true /* out of date */}; } diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.h b/impeller/renderer/backend/vulkan/swapchain_impl_vk.h index 0df1a2f7a200c..cacc6b2494b6b 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.h @@ -9,6 +9,7 @@ #include "flutter/fml/macros.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "vulkan/vulkan_enums.hpp" namespace impeller { @@ -31,7 +32,9 @@ class SwapchainImplVK final static std::shared_ptr Create( const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - vk::SwapchainKHR old_swapchain = VK_NULL_HANDLE); + vk::SwapchainKHR old_swapchain = VK_NULL_HANDLE, + vk::SurfaceTransformFlagBitsKHR last_transform = + vk::SurfaceTransformFlagBitsKHR::eIdentity); ~SwapchainImplVK(); @@ -51,6 +54,8 @@ class SwapchainImplVK final vk::Format GetSurfaceFormat() const; + vk::SurfaceTransformFlagBitsKHR GetLastTransform() const; + std::shared_ptr GetContext() const; std::pair DestroySwapchain(); @@ -65,10 +70,13 @@ class SwapchainImplVK final std::vector> synchronizers_; size_t current_frame_ = 0u; bool is_valid_ = false; + size_t current_transform_poll_count_ = 0u; + vk::SurfaceTransformFlagBitsKHR transform_if_changed_discard_swapchain_; SwapchainImplVK(const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - vk::SwapchainKHR old_swapchain); + vk::SwapchainKHR old_swapchain, + vk::SurfaceTransformFlagBitsKHR last_transform); bool Present(const std::shared_ptr& image, uint32_t index); diff --git a/impeller/renderer/backend/vulkan/swapchain_vk.cc b/impeller/renderer/backend/vulkan/swapchain_vk.cc index 0bc6f9aca2ed3..0a09a9eb371e0 100644 --- a/impeller/renderer/backend/vulkan/swapchain_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_vk.cc @@ -47,9 +47,10 @@ std::unique_ptr SwapchainVK::AcquireNextDrawable() { auto context = impl_->GetContext(); auto [surface, old_swapchain] = impl_->DestroySwapchain(); - auto new_impl = SwapchainImplVK::Create(context, // - std::move(surface), // - *old_swapchain // + auto new_impl = SwapchainImplVK::Create(context, // + std::move(surface), // + *old_swapchain, // + impl_->GetLastTransform() // ); if (!new_impl || !new_impl->IsValid()) { VALIDATION_LOG << "Could not update swapchain."; From 5113140042a136535b41a8c6870f86901ffbefa9 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Aug 2023 16:00:35 -0700 Subject: [PATCH 5/7] Tweak switch case. --- impeller/renderer/backend/vulkan/swapchain_impl_vk.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 252b11c795124..b1a3f2ceef2b5 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -486,6 +486,11 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, // Vulkan guarantees that the set of queue operations will still // complete successfully. [[fallthrough]]; + case vk::Result::eSuboptimalKHR: + // In theory we shouldn't receive this (because we're polling above) + // but we shouldn't handle it as a validation error if we do receive + // it. + [[fallthrough]]; case vk::Result::eSuccess: return; default: From d23b8b62978df97583d2bcd6d629915a968edd23 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Aug 2023 16:01:47 -0700 Subject: [PATCH 6/7] Add comment. --- impeller/renderer/backend/vulkan/swapchain_impl_vk.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index b1a3f2ceef2b5..79c4cf0bc0d57 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -15,6 +15,10 @@ namespace impeller { static constexpr size_t kMaxFramesInFlight = 3u; + +// Number of frames to poll for orientation changes. For example `1u` means +// that the orientation will be polled every frame, while `2u` means that the +// orientation will be polled every other frame. static constexpr size_t kPollFramesForOrientation = 1u; struct FrameSynchronizer { From 5690f1f458039f9cf0662c57af26e79afc502e35 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 8 Aug 2023 16:07:36 -0700 Subject: [PATCH 7/7] Tweak message. --- impeller/renderer/backend/vulkan/swapchain_impl_vk.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 79c4cf0bc0d57..81871c54862c4 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -491,9 +491,10 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, // complete successfully. [[fallthrough]]; case vk::Result::eSuboptimalKHR: - // In theory we shouldn't receive this (because we're polling above) - // but we shouldn't handle it as a validation error if we do receive - // it. + // Even though we're handling rotation changes via polling, we + // still need to handle the case where the swapchain signals that + // it's suboptimal (i.e. every frame when we are rotated given we + // aren't doing Vulkan pre-rotation). [[fallthrough]]; case vk::Result::eSuccess: return;