Skip to content

Commit 8893670

Browse files
auto-submit[bot]auto-submit[bot]
andauthored
Reverts "[Impeller] remove transfer barriers from render pass, drop blit, tighten up graphics on level 3. (#165584)" (#165898)
<!-- start_original_pr_link --> Reverts: flutter/flutter#165584 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jonahwilliams <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: render pass compatibility issue. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: jonahwilliams <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {gaaclarke} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: The change to fix PowerVR barriers regressed Mali (see flutter/flutter#165538) We need to tighten the barriers to remove the transfer barrier to get back mali performance. however, this requires us to drop the usage of the blit for onscreen restore. not a huge loss imo. At any rate, all of these changes should be visible in benchmarks. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
1 parent b02c436 commit 8893670

9 files changed

Lines changed: 33 additions & 38 deletions

File tree

engine/src/flutter/impeller/display_list/canvas.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,8 +1718,8 @@ bool Canvas::SupportsBlitToOnscreen() const {
17181718
return renderer_.GetContext()
17191719
->GetCapabilities()
17201720
->SupportsTextureToTextureBlits() &&
1721-
renderer_.GetContext()->GetBackendType() ==
1722-
Context::BackendType::kMetal;
1721+
renderer_.GetContext()->GetBackendType() !=
1722+
Context::BackendType::kOpenGLES;
17231723
}
17241724

17251725
bool Canvas::BlitToOnscreen(bool is_onscreen) {

engine/src/flutter/impeller/display_list/canvas_unittests.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ TEST_P(AiksTest, SupportsBlitToOnscreen) {
379379
auto canvas = CreateTestCanvas(context, Rect::MakeLTRB(0, 0, 100, 100),
380380
/*requires_readback=*/true);
381381

382-
if (GetBackend() != PlaygroundBackend::kMetal) {
382+
if (GetBackend() == PlaygroundBackend::kOpenGLES) {
383383
EXPECT_FALSE(canvas->SupportsBlitToOnscreen());
384384
} else {
385385
EXPECT_TRUE(canvas->SupportsBlitToOnscreen());

engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ vk::UniqueImage CreateVKImageWrapperForAndroidHarwareBuffer(
5656
}
5757
if (ahb_desc.usage & AHARDWAREBUFFER_USAGE_GPU_FRAMEBUFFER) {
5858
image_usage_flags |= vk::ImageUsageFlagBits::eColorAttachment;
59+
}
60+
if (ahb_desc.usage & AHARDWAREBUFFER_USAGE_COMPOSER_OVERLAY) {
61+
image_usage_flags |= vk::ImageUsageFlagBits::eColorAttachment;
5962
image_usage_flags |= vk::ImageUsageFlagBits::eInputAttachment;
63+
image_usage_flags |= vk::ImageUsageFlagBits::eTransferDst;
6064
}
6165

6266
vk::ImageCreateFlags image_create_flags;
@@ -298,7 +302,7 @@ TextureDescriptor ToTextureDescriptor(const AHardwareBuffer_Desc& ahb_desc) {
298302
desc.mip_count = (ahb_desc.usage & AHARDWAREBUFFER_USAGE_GPU_MIPMAP_COMPLETE)
299303
? ahb_size.MipCount()
300304
: 1u;
301-
if (ahb_desc.usage & AHARDWAREBUFFER_USAGE_GPU_FRAMEBUFFER) {
305+
if (ahb_desc.usage & AHARDWAREBUFFER_USAGE_COMPOSER_OVERLAY) {
302306
desc.usage = TextureUsage::kRenderTarget;
303307
}
304308
return desc;

engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_builder_vk.cc

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -196,20 +196,12 @@ vk::UniqueRenderPass RenderPassBuilderVK::Build(
196196
// to the onscreen.
197197
deps[0].srcSubpass = VK_SUBPASS_EXTERNAL;
198198
deps[0].dstSubpass = 0u;
199-
// If this render pass is performed using the onscreen attachment, then we
200-
// know that the previous stage does not include sampling - only color
201-
// attachment. According to various vulkan documentation, the correct access
202-
// flag bits for this stage with a queue submit in-between is `{}` as the
203-
// access is not otherwise expressable.
204-
if (is_swapchain_) {
205-
deps[0].srcStageMask = vk::PipelineStageFlagBits::eColorAttachmentOutput;
206-
deps[0].srcAccessMask = {};
207-
} else {
208-
deps[0].srcStageMask = vk::PipelineStageFlagBits::eColorAttachmentOutput |
209-
vk::PipelineStageFlagBits::eFragmentShader;
210-
deps[0].srcAccessMask = vk::AccessFlagBits::eShaderRead |
211-
vk::AccessFlagBits::eColorAttachmentWrite;
212-
}
199+
deps[0].srcStageMask = vk::PipelineStageFlagBits::eColorAttachmentOutput |
200+
vk::PipelineStageFlagBits::eFragmentShader |
201+
vk::PipelineStageFlagBits::eTransfer;
202+
deps[0].srcAccessMask = vk::AccessFlagBits::eShaderRead |
203+
vk::AccessFlagBits::eTransferRead |
204+
vk::AccessFlagBits::eColorAttachmentWrite;
213205
deps[0].dstStageMask = vk::PipelineStageFlagBits::eColorAttachmentOutput;
214206
deps[0].dstAccessMask = vk::AccessFlagBits::eColorAttachmentWrite;
215207
deps[0].dependencyFlags = kSelfDependencyFlags;
@@ -225,23 +217,22 @@ vk::UniqueRenderPass RenderPassBuilderVK::Build(
225217
deps[1].dependencyFlags = kSelfDependencyFlags;
226218

227219
// Outgoing dependency. The resolve step or color attachment must complete
228-
// before we can sample from the image. This dependency is ignored for the
229-
// onscreen as we will already insert a barrier before presenting the
230-
// swapchain.
220+
// before we can sample from the image, or use it as a blit src.
231221
deps[2].srcSubpass = 0u; // first subpass
232222
deps[2].dstSubpass = VK_SUBPASS_EXTERNAL;
233223
deps[2].srcStageMask = vk::PipelineStageFlagBits::eColorAttachmentOutput;
234224
deps[2].srcAccessMask = vk::AccessFlagBits::eColorAttachmentWrite;
235-
deps[2].dstStageMask = vk::PipelineStageFlagBits::eFragmentShader;
236-
deps[2].dstAccessMask = vk::AccessFlagBits::eShaderRead;
225+
deps[2].dstStageMask = vk::PipelineStageFlagBits::eFragmentShader |
226+
vk::PipelineStageFlagBits::eTransfer;
227+
deps[2].dstAccessMask =
228+
vk::AccessFlagBits::eShaderRead | vk::AccessFlagBits::eTransferRead;
237229
deps[2].dependencyFlags = kSelfDependencyFlags;
238230

239231
vk::RenderPassCreateInfo render_pass_desc;
240232
render_pass_desc.setPAttachments(attachments.data());
241233
render_pass_desc.setAttachmentCount(attachments_index);
242234
render_pass_desc.setSubpasses(subpass0);
243-
render_pass_desc.setPDependencies(deps);
244-
render_pass_desc.setDependencyCount(3);
235+
render_pass_desc.setDependencies(deps);
245236

246237
auto [result, pass] = device.createRenderPassUnique(render_pass_desc);
247238
if (result != vk::Result::eSuccess) {
@@ -281,10 +272,6 @@ void InsertBarrierForInputAttachmentRead(const vk::CommandBuffer& buffer,
281272
);
282273
}
283274

284-
void RenderPassBuilderVK::SetSwapchain(bool value) {
285-
is_swapchain_ = value;
286-
}
287-
288275
const std::map<size_t, vk::AttachmentDescription>&
289276
RenderPassBuilderVK::GetColorAttachments() const {
290277
return colors_;

engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_builder_vk.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@ class RenderPassBuilderVK {
2828

2929
RenderPassBuilderVK& operator=(const RenderPassBuilderVK&) = delete;
3030

31-
/// Whether this builder should configure barriers for the onscreen render
32-
/// pass.
33-
void SetSwapchain(bool value);
34-
3531
RenderPassBuilderVK& SetColorAttachment(
3632
size_t index,
3733
PixelFormat format,
@@ -70,7 +66,6 @@ class RenderPassBuilderVK {
7066
std::optional<vk::AttachmentDescription> GetColor0Resolve() const;
7167

7268
private:
73-
bool is_swapchain_ = false;
7469
std::optional<vk::AttachmentDescription> color0_;
7570
std::optional<vk::AttachmentDescription> color0_resolve_;
7671
std::optional<vk::AttachmentDescription> depth_stencil_;

engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_vk.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
8181
const std::shared_ptr<CommandBufferVK>& command_buffer,
8282
bool is_swapchain) const {
8383
RenderPassBuilderVK builder;
84-
builder.SetSwapchain(is_swapchain);
8584

8685
render_target_.IterateAllColorAttachments([&](size_t bind_point,
8786
const ColorAttachment&

engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/khr/khr_swapchain_impl_vk.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,10 @@ KHRSwapchainImplVK::KHRSwapchainImplVK(const std::shared_ptr<Context>& context,
186186
: surface_caps.maxImageCount // max zero means no limit
187187
);
188188
swapchain_info.imageArrayLayers = 1u;
189-
// Swapchain images are primarily used as color attachments (via resolve) or
190-
// input attachments.
189+
// Swapchain images are primarily used as color attachments (via resolve),
190+
// blit targets, or input attachments.
191191
swapchain_info.imageUsage = vk::ImageUsageFlagBits::eColorAttachment |
192+
vk::ImageUsageFlagBits::eTransferDst |
192193
vk::ImageUsageFlagBits::eInputAttachment;
193194
swapchain_info.preTransform = vk::SurfaceTransformFlagBitsKHR::eIdentity;
194195
swapchain_info.compositeAlpha = composite.value();

engine/src/flutter/impeller/renderer/backend/vulkan/texture_source_vk.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,24 @@ std::shared_ptr<YUVConversionVK> TextureSourceVK::GetYUVConversion() const {
1919
}
2020

2121
vk::ImageLayout TextureSourceVK::GetLayout() const {
22+
ReaderLock lock(layout_mutex_);
2223
return layout_;
2324
}
2425

2526
vk::ImageLayout TextureSourceVK::SetLayoutWithoutEncoding(
2627
vk::ImageLayout layout) const {
28+
WriterLock lock(layout_mutex_);
2729
const auto old_layout = layout_;
2830
layout_ = layout;
2931
return old_layout;
3032
}
3133

3234
fml::Status TextureSourceVK::SetLayout(const BarrierVK& barrier) const {
3335
const auto old_layout = SetLayoutWithoutEncoding(barrier.new_layout);
36+
if (barrier.new_layout == old_layout) {
37+
return {};
38+
}
39+
3440
vk::ImageMemoryBarrier image_barrier;
3541
image_barrier.srcAccessMask = barrier.src_access;
3642
image_barrier.dstAccessMask = barrier.dst_access;

engine/src/flutter/impeller/renderer/backend/vulkan/texture_source_vk.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_TEXTURE_SOURCE_VK_H_
77

88
#include "flutter/fml/status.h"
9+
#include "impeller/base/thread.h"
910
#include "impeller/core/texture_descriptor.h"
1011
#include "impeller/renderer/backend/vulkan/barrier_vk.h"
1112
#include "impeller/renderer/backend/vulkan/formats_vk.h"
@@ -158,7 +159,9 @@ class TextureSourceVK {
158159
private:
159160
SharedHandleVK<vk::Framebuffer> framebuffer_;
160161
SharedHandleVK<vk::RenderPass> render_pass_;
161-
mutable vk::ImageLayout layout_ = vk::ImageLayout::eUndefined;
162+
mutable RWMutex layout_mutex_;
163+
mutable vk::ImageLayout layout_ IPLR_GUARDED_BY(layout_mutex_) =
164+
vk::ImageLayout::eUndefined;
162165
};
163166

164167
} // namespace impeller

0 commit comments

Comments
 (0)