From 94f0e83a417cef9a5a39fdc6061e9b6b8e5947c6 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 27 Mar 2023 17:33:16 -0700 Subject: [PATCH] [Impeller] Cleanup unusued Vulkan APIs and enable more tests. `CanRenderToTexture` was letting go of the command buffer and then adding to a pass which was tripping an assertion. --- impeller/aiks/aiks_unittests.cc | 3 -- .../renderer/backend/vulkan/allocator_vk.cc | 31 ------------------- .../renderer/backend/vulkan/context_vk.cc | 1 + .../backend/vulkan/swapchain_impl_vk.cc | 12 ++++--- .../backend/vulkan/texture_source_vk.cc | 7 ----- .../backend/vulkan/texture_source_vk.h | 5 --- impeller/renderer/renderer_unittests.cc | 8 ++--- 7 files changed, 11 insertions(+), 56 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 2eba6cd0ef729..2a5e704cf69cb 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1877,9 +1877,6 @@ TEST_P(AiksTest, SaveLayerFiltersScaleWithTransform) { } TEST_P(AiksTest, SceneColorSource) { - if (GetBackend() == PlaygroundBackend::kVulkan) { - GTEST_SKIP_("Temporarily disabled."); - } // Load up the scene. auto mapping = flutter::testing::OpenFixtureAsMapping("flutter_logo_baked.glb.ipscene"); diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index df2ed86359d9b..087e6b5d7fa9f 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -311,37 +311,6 @@ class AllocatedTextureSourceVK final : public TextureSourceVK { } } - bool SetContents(const TextureDescriptor& desc, - const uint8_t* contents, - size_t length, - size_t slice) override { - void* data = nullptr; - if (::vmaMapMemory(allocator_, allocation_, &data) != VK_SUCCESS) { - VALIDATION_LOG << "Could not map texture memory to write to."; - return false; - } - - std::memcpy(static_cast(data) + (length * slice), // - contents, // - length // - ); - - const auto flushed = ::vmaFlushAllocation(allocator_, // allocator - allocation_, // allocation - length * slice, // offset - length // size - ) == VK_SUCCESS; - - ::vmaUnmapMemory(allocator_, allocation_); - - if (!flushed) { - VALIDATION_LOG << "Could not flush written mapped memory."; - return false; - } - - return true; - } - bool IsValid() const { return is_valid_; } vk::Image GetImage() const override { return image_; } diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 3aff93de587be..3ab610373de63 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -397,6 +397,7 @@ vk::Device ContextVK::GetDevice() const { } std::unique_ptr ContextVK::AcquireNextSurface() { + TRACE_EVENT0("impeller", __FUNCTION__); auto surface = swapchain_ ? swapchain_->AcquireNextDrawable() : nullptr; if (surface && pipeline_library_) { pipeline_library_->DidAcquireSurfaceFrame(); diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 56a255d141f80..88d8c4b93a6b0 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -29,7 +29,7 @@ struct FrameSynchronizer { if (acquire_res.result != vk::Result::eSuccess || render_res.result != vk::Result::eSuccess || present_res.result != vk::Result::eSuccess) { - VALIDATION_LOG << "Could not create synchornizer."; + VALIDATION_LOG << "Could not create synchronizer."; return; } acquire = std::move(acquire_res.value); @@ -41,14 +41,18 @@ struct FrameSynchronizer { ~FrameSynchronizer() = default; bool WaitForFence(const vk::Device& device) { - if (device.waitForFences( + if (auto result = device.waitForFences( *acquire, // fence true, // wait all std::numeric_limits::max() // timeout (ns) - ) != vk::Result::eSuccess) { + ); + result != vk::Result::eSuccess) { + VALIDATION_LOG << "Fence wait failed: " << vk::to_string(result); return false; } - if (device.resetFences(*acquire) != vk::Result::eSuccess) { + if (auto result = device.resetFences(*acquire); + result != vk::Result::eSuccess) { + VALIDATION_LOG << "Could not reset fence: " << vk::to_string(result); return false; } return true; diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.cc b/impeller/renderer/backend/vulkan/texture_source_vk.cc index 5e509a81ce69c..84481ad9f3394 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_source_vk.cc @@ -58,11 +58,4 @@ bool TextureSourceVK::SetLayout(const LayoutTransition& transition) const { return true; } -bool TextureSourceVK::SetContents(const TextureDescriptor& desc, - const uint8_t* contents, - size_t length, - size_t slice) { - return false; -} - } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h index 4b65739427c9b..2ec7d5876563e 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.h +++ b/impeller/renderer/backend/vulkan/texture_source_vk.h @@ -18,11 +18,6 @@ class TextureSourceVK { const TextureDescriptor& GetTextureDescriptor() const; - virtual bool SetContents(const TextureDescriptor& desc, - const uint8_t* contents, - size_t length, - size_t slice); - virtual vk::Image GetImage() const = 0; virtual vk::ImageView GetImageView() const = 0; diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 0cd1bc04e6ce5..8eabd143a21db 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -278,9 +278,6 @@ TEST_P(RendererTest, CanRenderMultiplePrimitives) { } TEST_P(RendererTest, CanRenderToTexture) { - if (GetBackend() == PlaygroundBackend::kVulkan) { - GTEST_SKIP_("Temporarily disabled."); - } using VS = BoxFadeVertexShader; using FS = BoxFadeFragmentShader; auto context = GetContext(); @@ -312,9 +309,9 @@ TEST_P(RendererTest, CanRenderToTexture) { ASSERT_TRUE(bridge && boston); auto sampler = context->GetSamplerLibrary()->GetSampler({}); ASSERT_TRUE(sampler); - std::shared_ptr r2t_pass; - + auto cmd_buffer = context->CreateCommandBuffer(); + ASSERT_TRUE(cmd_buffer); { ColorAttachment color0; color0.load_action = LoadAction::kClear; @@ -352,7 +349,6 @@ TEST_P(RendererTest, CanRenderToTexture) { RenderTarget r2t_desc; r2t_desc.SetColorAttachment(color0, 0u); r2t_desc.SetStencilAttachment(stencil0); - auto cmd_buffer = context->CreateCommandBuffer(); r2t_pass = cmd_buffer->CreateRenderPass(r2t_desc); ASSERT_TRUE(r2t_pass && r2t_pass->IsValid()); }