Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit b580325

Browse files
author
Jonah Williams
authored
Revert "[Impeller] iOS/macOS: Only wait for command scheduling prior to present" (#40895)
Reverts #40781 Fixes flutter/flutter#124056 When unmerging threads we appear to get stuck for multiple seconds. Haven't debugged further, but bisected to this commit.
1 parent 2e5cdea commit b580325

File tree

5 files changed

+27
-27
lines changed

5 files changed

+27
-27
lines changed

impeller/renderer/backend/metal/command_buffer_mtl.mm

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,27 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
167167

168168
[buffer_ commit];
169169

170+
#if (FML_OS_MACOSX || FML_OS_IOS_SIMULATOR)
171+
// We're using waitUntilScheduled on macOS and iOS simulator to force a hard
172+
// barrier between the execution of different command buffers. This forces all
173+
// renderable texture access to be synchronous (i.e. a write from a previous
174+
// command buffer will not get scheduled to happen at the same time as a read
175+
// in a future command buffer).
176+
//
177+
// Metal hazard tracks shared memory resources by default, and we don't need
178+
// to do any additional work to synchronize access to MTLTextures and
179+
// MTLBuffers on iOS devices with UMA. However, shared textures are disallowed
180+
// on macOS according to the documentation:
181+
// https://developer.apple.com/documentation/metal/mtlstoragemode/shared
182+
// And so this is a stopgap solution that has been present in Impeller since
183+
// multi-pass rendering/SaveLayer support was first set up.
184+
//
185+
// TODO(bdero): Remove this for all targets once a solution for resource
186+
// tracking that works everywhere is established:
187+
// https://github.com/flutter/flutter/issues/120406
188+
[buffer_ waitUntilScheduled];
189+
#endif
190+
170191
buffer_ = nil;
171192
return true;
172193
}

impeller/renderer/backend/metal/context_mtl.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ class ContextMTL final : public Context,
6464
// |Context|
6565
bool UpdateOffscreenLayerPixelFormat(PixelFormat format) override;
6666

67-
id<MTLCommandBuffer> CreateMTLCommandBuffer() const;
68-
6967
private:
7068
id<MTLDevice> device_ = nullptr;
7169
id<MTLCommandQueue> command_queue_ = nullptr;

impeller/renderer/backend/metal/context_mtl.mm

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,4 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
291291
return true;
292292
}
293293

294-
id<MTLCommandBuffer> ContextMTL::CreateMTLCommandBuffer() const {
295-
return [command_queue_ commandBuffer];
296-
}
297-
298294
} // namespace impeller

impeller/renderer/backend/metal/surface_mtl.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,9 @@ class SurfaceMTL final : public Surface {
4343
id<MTLDrawable> drawable() const { return drawable_; }
4444

4545
private:
46-
std::weak_ptr<Context> context_;
4746
id<MTLDrawable> drawable_ = nil;
4847

49-
SurfaceMTL(const std::weak_ptr<Context>& context,
50-
const RenderTarget& target,
51-
id<MTLDrawable> drawable);
48+
SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable);
5249

5350
// |Surface|
5451
bool Present() const override;

impeller/renderer/backend/metal/surface_mtl.mm

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
#include "flutter/fml/trace_event.h"
88
#include "impeller/base/validation.h"
9-
#include "impeller/renderer/backend/metal/context_mtl.h"
109
#include "impeller/renderer/backend/metal/formats_mtl.h"
1110
#include "impeller/renderer/backend/metal/texture_mtl.h"
1211
#include "impeller/renderer/render_target.h"
@@ -112,14 +111,12 @@
112111
render_target_desc.SetStencilAttachment(stencil0);
113112

114113
// The constructor is private. So make_unique may not be used.
115-
return std::unique_ptr<SurfaceMTL>(new SurfaceMTL(
116-
context->weak_from_this(), render_target_desc, current_drawable));
114+
return std::unique_ptr<SurfaceMTL>(
115+
new SurfaceMTL(render_target_desc, current_drawable));
117116
}
118117

119-
SurfaceMTL::SurfaceMTL(const std::weak_ptr<Context>& context,
120-
const RenderTarget& target,
121-
id<MTLDrawable> drawable)
122-
: Surface(target), context_(context), drawable_(drawable) {}
118+
SurfaceMTL::SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable)
119+
: Surface(target), drawable_(drawable) {}
123120

124121
// |Surface|
125122
SurfaceMTL::~SurfaceMTL() = default;
@@ -130,16 +127,7 @@
130127
return false;
131128
}
132129

133-
auto context = context_.lock();
134-
if (!context) {
135-
return false;
136-
}
137-
138-
id<MTLCommandBuffer> command_buffer =
139-
ContextMTL::Cast(context.get())->CreateMTLCommandBuffer();
140-
[command_buffer presentDrawable:drawable_];
141-
[command_buffer commit];
142-
130+
[drawable_ present];
143131
return true;
144132
}
145133
#pragma GCC diagnostic pop

0 commit comments

Comments
 (0)