From 37de16cc57de33cde196fa35d93ffb73602176f0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 15 Dec 2024 13:35:07 -0800 Subject: [PATCH 1/5] [Impeller] fix image layout when resolve texture has mip levels. --- .../renderer/backend/vulkan/render_pass_vk.cc | 35 +++++++++++++------ .../renderer/backend/vulkan/render_pass_vk.h | 1 - 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index b4a939d909fd2..368fae77365a9 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -24,8 +24,6 @@ #include "impeller/renderer/backend/vulkan/sampler_vk.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" -#include "vulkan/vulkan.hpp" -#include "vulkan/vulkan_handles.hpp" namespace impeller { @@ -91,15 +89,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( attachment.texture->GetTextureDescriptor().format, // attachment.texture->GetTextureDescriptor().sample_count, // attachment.load_action, // - attachment.store_action, // - TextureVK::Cast(*attachment.texture).GetLayout() // + attachment.store_action // ); - TextureVK::Cast(*attachment.texture) - .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); - if (attachment.resolve_texture) { - TextureVK::Cast(*attachment.resolve_texture) - .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); - } return true; }); @@ -202,6 +193,30 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, pass_info.setPClearValues(clears.data()); pass_info.setClearValueCount(clear_count); + if (resolve_image_vk_) { + // If the resolve image has mip levels, only mip level 0 will be + // transitioned by the render pass. All subsequent mip levels must have an + // explicit barrier inserted to transition from undefined. + if (resolve_image_vk_->GetTextureDescriptor().mip_count > 1) { + BarrierVK barrier; + barrier.new_layout = vk::ImageLayout::eGeneral; + barrier.cmd_buffer = command_buffer_vk_; + barrier.src_access = vk::AccessFlagBits::eShaderRead; + barrier.src_stage = vk::PipelineStageFlagBits::eFragmentShader; + barrier.dst_access = vk::AccessFlagBits::eColorAttachmentWrite | + vk::AccessFlagBits::eTransferWrite; + barrier.dst_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | + vk::PipelineStageFlagBits::eTransfer; + TextureVK::Cast(*resolve_image_vk_).SetLayout(barrier); + } else { + TextureVK::Cast(*resolve_image_vk_) + .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); + } + } else if (color_image_vk_) { + TextureVK::Cast(*resolve_image_vk_) + .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); + } + command_buffer_vk_.beginRenderPass(pass_info, vk::SubpassContents::eInline); // Set the initial viewport. diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index a467652c62485..80d6d93f0c7b7 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -12,7 +12,6 @@ #include "impeller/renderer/command_buffer.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" -#include "vulkan/vulkan_handles.hpp" namespace impeller { From 33b9082ce8d2d484707d7aa9e3d34dbd786d551d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 15 Dec 2024 13:38:47 -0800 Subject: [PATCH 2/5] add unit test. --- impeller/display_list/aiks_dl_unittests.cc | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/impeller/display_list/aiks_dl_unittests.cc b/impeller/display_list/aiks_dl_unittests.cc index 6c389981031f2..c4c2c4fc9faf7 100644 --- a/impeller/display_list/aiks_dl_unittests.cc +++ b/impeller/display_list/aiks_dl_unittests.cc @@ -959,6 +959,31 @@ TEST_P(AiksTest, CanPictureConvertToImage) { ASSERT_TRUE(OpenPlaygroundHere(canvas.Build())); } +TEST_P(AiksTest, CanPictureConvertToImageWithMips) { + DisplayListBuilder recorder_canvas; + DlPaint paint; + paint.setColor(DlColor::RGBA(0.9568, 0.2627, 0.2118, 1.0)); + recorder_canvas.DrawRect(SkRect::MakeXYWH(100.0, 100.0, 600, 600), paint); + paint.setColor(DlColor::RGBA(0.1294, 0.5882, 0.9529, 1.0)); + recorder_canvas.DrawRect(SkRect::MakeXYWH(200.0, 200.0, 600, 600), paint); + + DisplayListBuilder canvas; + AiksContext renderer(GetContext(), nullptr); + paint.setColor(DlColor::kTransparent()); + canvas.DrawPaint(paint); + + auto image = + DisplayListToTexture(recorder_canvas.Build(), {1000, 1000}, renderer, + /*reset_host_buffer=*/true, /*generate_mips=*/true); + if (image) { + canvas.DrawImage(DlImageImpeller::Make(image), SkPoint{}, {}); + paint.setColor(DlColor::RGBA(0.1, 0.1, 0.1, 0.2)); + canvas.DrawRect(SkRect::MakeSize({1000, 1000}), paint); + } + + ASSERT_TRUE(OpenPlaygroundHere(canvas.Build())); +} + // Regression test for https://github.com/flutter/flutter/issues/142358 . // Without a change to force render pass construction the image is left in an // undefined layout and triggers a validation error. From 615f876fc7fd886764aa43b0dd5cf89f8f24a6ff Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 16 Dec 2024 14:00:09 -0800 Subject: [PATCH 3/5] disable mip thing and test. --- .../renderer/backend/vulkan/render_pass_vk.cc | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 368fae77365a9..798fec4e6e96f 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -194,25 +194,10 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, pass_info.setClearValueCount(clear_count); if (resolve_image_vk_) { - // If the resolve image has mip levels, only mip level 0 will be - // transitioned by the render pass. All subsequent mip levels must have an - // explicit barrier inserted to transition from undefined. - if (resolve_image_vk_->GetTextureDescriptor().mip_count > 1) { - BarrierVK barrier; - barrier.new_layout = vk::ImageLayout::eGeneral; - barrier.cmd_buffer = command_buffer_vk_; - barrier.src_access = vk::AccessFlagBits::eShaderRead; - barrier.src_stage = vk::PipelineStageFlagBits::eFragmentShader; - barrier.dst_access = vk::AccessFlagBits::eColorAttachmentWrite | - vk::AccessFlagBits::eTransferWrite; - barrier.dst_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | - vk::PipelineStageFlagBits::eTransfer; - TextureVK::Cast(*resolve_image_vk_).SetLayout(barrier); - } else { - TextureVK::Cast(*resolve_image_vk_) - .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); - } - } else if (color_image_vk_) { + TextureVK::Cast(*resolve_image_vk_) + .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); + } + if (color_image_vk_) { TextureVK::Cast(*resolve_image_vk_) .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); } From 54518ddd0ce2418ee9093e70d263f22c2ebea94e Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 17 Dec 2024 08:53:34 -0800 Subject: [PATCH 4/5] Update aiks_dl_unittests.cc --- impeller/display_list/aiks_dl_unittests.cc | 25 ---------------------- 1 file changed, 25 deletions(-) diff --git a/impeller/display_list/aiks_dl_unittests.cc b/impeller/display_list/aiks_dl_unittests.cc index c4c2c4fc9faf7..6c389981031f2 100644 --- a/impeller/display_list/aiks_dl_unittests.cc +++ b/impeller/display_list/aiks_dl_unittests.cc @@ -959,31 +959,6 @@ TEST_P(AiksTest, CanPictureConvertToImage) { ASSERT_TRUE(OpenPlaygroundHere(canvas.Build())); } -TEST_P(AiksTest, CanPictureConvertToImageWithMips) { - DisplayListBuilder recorder_canvas; - DlPaint paint; - paint.setColor(DlColor::RGBA(0.9568, 0.2627, 0.2118, 1.0)); - recorder_canvas.DrawRect(SkRect::MakeXYWH(100.0, 100.0, 600, 600), paint); - paint.setColor(DlColor::RGBA(0.1294, 0.5882, 0.9529, 1.0)); - recorder_canvas.DrawRect(SkRect::MakeXYWH(200.0, 200.0, 600, 600), paint); - - DisplayListBuilder canvas; - AiksContext renderer(GetContext(), nullptr); - paint.setColor(DlColor::kTransparent()); - canvas.DrawPaint(paint); - - auto image = - DisplayListToTexture(recorder_canvas.Build(), {1000, 1000}, renderer, - /*reset_host_buffer=*/true, /*generate_mips=*/true); - if (image) { - canvas.DrawImage(DlImageImpeller::Make(image), SkPoint{}, {}); - paint.setColor(DlColor::RGBA(0.1, 0.1, 0.1, 0.2)); - canvas.DrawRect(SkRect::MakeSize({1000, 1000}), paint); - } - - ASSERT_TRUE(OpenPlaygroundHere(canvas.Build())); -} - // Regression test for https://github.com/flutter/flutter/issues/142358 . // Without a change to force render pass construction the image is left in an // undefined layout and triggers a validation error. From a69e12ac770ca2127289e5bd7122d6ee86713afe Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 17 Dec 2024 09:48:24 -0800 Subject: [PATCH 5/5] ++ --- impeller/renderer/backend/vulkan/render_pass_vk.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 798fec4e6e96f..1ace0c64f30ed 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -198,7 +198,7 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); } if (color_image_vk_) { - TextureVK::Cast(*resolve_image_vk_) + TextureVK::Cast(*color_image_vk_) .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); }