[Impeller] iOS/macOS: Only wait for command scheduling prior to present (redux) (#41501)
Reverts https://github.com/flutter/engine/pull/40895.
Resolves https://github.com/flutter/flutter/issues/120399 (again).
A bunch of frames get pumped on the main thread _without a transaction_
just before unmerging occurs (I don't know why this happens), and so
checking the current thread to determine whether we need to present with
a transaction or not isn't sufficient. In the prior fix, after the
unmerge, the raster thread would hang for one second while waiting for
the next drawable to get freed up (happens on the second raster thread
frame post-unmerge), and then subsequent presents would just do nothing
for a while, but eventually recover.
`presentsWithTransaction` works whether the `CATransaction` stack is
empty or not, and so the only difference here is that
`presentsWithTransaction` is always turned on and `presentDrawable` is
always avoided (otherwise it tries to present too early and nothing
renders when platform views are present).
diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.mm b/impeller/renderer/backend/metal/command_buffer_mtl.mm
index 7554067..bd1b71f 100644
--- a/impeller/renderer/backend/metal/command_buffer_mtl.mm
+++ b/impeller/renderer/backend/metal/command_buffer_mtl.mm
@@ -167,27 +167,6 @@
[buffer_ commit];
-#if (FML_OS_MACOSX || FML_OS_IOS_SIMULATOR)
- // We're using waitUntilScheduled on macOS and iOS simulator to force a hard
- // barrier between the execution of different command buffers. This forces all
- // renderable texture access to be synchronous (i.e. a write from a previous
- // command buffer will not get scheduled to happen at the same time as a read
- // in a future command buffer).
- //
- // Metal hazard tracks shared memory resources by default, and we don't need
- // to do any additional work to synchronize access to MTLTextures and
- // MTLBuffers on iOS devices with UMA. However, shared textures are disallowed
- // on macOS according to the documentation:
- // https://developer.apple.com/documentation/metal/mtlstoragemode/shared
- // And so this is a stopgap solution that has been present in Impeller since
- // multi-pass rendering/SaveLayer support was first set up.
- //
- // TODO(bdero): Remove this for all targets once a solution for resource
- // tracking that works everywhere is established:
- // https://github.com/flutter/flutter/issues/120406
- [buffer_ waitUntilScheduled];
-#endif
-
buffer_ = nil;
return true;
}
diff --git a/impeller/renderer/backend/metal/context_mtl.h b/impeller/renderer/backend/metal/context_mtl.h
index 86f9114..c0e86de 100644
--- a/impeller/renderer/backend/metal/context_mtl.h
+++ b/impeller/renderer/backend/metal/context_mtl.h
@@ -63,6 +63,8 @@
// |Context|
bool UpdateOffscreenLayerPixelFormat(PixelFormat format) override;
+ id<MTLCommandBuffer> CreateMTLCommandBuffer() const;
+
private:
id<MTLDevice> device_ = nullptr;
id<MTLCommandQueue> command_queue_ = nullptr;
diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm
index 38c09ff..c83f5b7 100644
--- a/impeller/renderer/backend/metal/context_mtl.mm
+++ b/impeller/renderer/backend/metal/context_mtl.mm
@@ -289,4 +289,8 @@
return true;
}
+id<MTLCommandBuffer> ContextMTL::CreateMTLCommandBuffer() const {
+ return [command_queue_ commandBuffer];
+}
+
} // namespace impeller
diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h
index e641dd6..157e4e6 100644
--- a/impeller/renderer/backend/metal/surface_mtl.h
+++ b/impeller/renderer/backend/metal/surface_mtl.h
@@ -43,9 +43,12 @@
id<MTLDrawable> drawable() const { return drawable_; }
private:
+ std::weak_ptr<Context> context_;
id<MTLDrawable> drawable_ = nil;
- SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable);
+ SurfaceMTL(const std::weak_ptr<Context>& context,
+ const RenderTarget& target,
+ id<MTLDrawable> drawable);
// |Surface|
bool Present() const override;
diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm
index dccdac1..672923c 100644
--- a/impeller/renderer/backend/metal/surface_mtl.mm
+++ b/impeller/renderer/backend/metal/surface_mtl.mm
@@ -6,6 +6,7 @@
#include "flutter/fml/trace_event.h"
#include "impeller/base/validation.h"
+#include "impeller/renderer/backend/metal/context_mtl.h"
#include "impeller/renderer/backend/metal/formats_mtl.h"
#include "impeller/renderer/backend/metal/texture_mtl.h"
#include "impeller/renderer/render_target.h"
@@ -111,12 +112,14 @@
render_target_desc.SetStencilAttachment(stencil0);
// The constructor is private. So make_unique may not be used.
- return std::unique_ptr<SurfaceMTL>(
- new SurfaceMTL(render_target_desc, current_drawable));
+ return std::unique_ptr<SurfaceMTL>(new SurfaceMTL(
+ context->weak_from_this(), render_target_desc, current_drawable));
}
-SurfaceMTL::SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable)
- : Surface(target), drawable_(drawable) {}
+SurfaceMTL::SurfaceMTL(const std::weak_ptr<Context>& context,
+ const RenderTarget& target,
+ id<MTLDrawable> drawable)
+ : Surface(target), context_(context), drawable_(drawable) {}
// |Surface|
SurfaceMTL::~SurfaceMTL() = default;
@@ -127,7 +130,21 @@
return false;
}
+ auto context = context_.lock();
+ if (!context) {
+ return false;
+ }
+
+ // If a transaction is present, `presentDrawable` will present too early. And
+ // so we wait on an empty command buffer to get scheduled instead, which
+ // forces us to also wait for all of the previous command buffers in the queue
+ // to get scheduled.
+ id<MTLCommandBuffer> command_buffer =
+ ContextMTL::Cast(context.get())->CreateMTLCommandBuffer();
+ [command_buffer commit];
+ [command_buffer waitUntilScheduled];
[drawable_ present];
+
return true;
}
#pragma GCC diagnostic pop
diff --git a/shell/platform/darwin/ios/ios_surface_metal_impeller.mm b/shell/platform/darwin/ios/ios_surface_metal_impeller.mm
index 427b91e..b025f4d 100644
--- a/shell/platform/darwin/ios/ios_surface_metal_impeller.mm
+++ b/shell/platform/darwin/ios/ios_surface_metal_impeller.mm
@@ -59,7 +59,7 @@
// When there are platform views in the scene, the drawable needs to be presented in the same
// transaction as the one created for platform views. When the drawable are being presented from
// the raster thread, there is no such transaction.
- layer.presentsWithTransaction = [[NSThread currentThread] isMainThread];
+ layer.presentsWithTransaction = YES;
return layer;
}