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

Platform View implemenation for Metal #11070

Merged
merged 18 commits into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ FILE: ../../../flutter/shell/common/vsync_waiter_fallback.cc
FILE: ../../../flutter/shell/common/vsync_waiter_fallback.h
FILE: ../../../flutter/shell/gpu/gpu_surface_gl.cc
FILE: ../../../flutter/shell/gpu/gpu_surface_gl.h
FILE: ../../../flutter/shell/gpu/gpu_surface_delegate.h
FILE: ../../../flutter/shell/gpu/gpu_surface_gl_delegate.cc
FILE: ../../../flutter/shell/gpu/gpu_surface_gl_delegate.h
FILE: ../../../flutter/shell/gpu/gpu_surface_metal.h
Expand Down
11 changes: 8 additions & 3 deletions shell/common/animator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,27 @@ Animator::Animator(Delegate& delegate,
waiter_(std::move(waiter)),
last_begin_frame_time_(),
dart_frame_deadline_(0),
#if FLUTTER_SHELL_ENABLE_METAL
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(2)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't an issue on Metal, I believe because we're controlling when we actually present the frame so that it always happens before the transaction is committed.

Can we do that on OpenGL?

Copy link
Member

@chinmaygarde chinmaygarde Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the issue was identified when using OpenGL. IIRC this was a speculative fix for the keyboard lag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - but the keyboard lag doesn't happen on metal. Without the wait added in this PR for metal, similar issues are seen but much worse.

#else // FLUTTER_SHELL_ENABLE_METAL
// TODO(dnfield): We should remove this logic and set the pipeline depth
// back to 2 in this case. See https://github.com/flutter/engine/pull/9132
// for discussion.
// back to 2 in this case. See
// https://github.com/flutter/engine/pull/9132 for discussion.
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(
task_runners.GetPlatformTaskRunner() ==
task_runners.GetGPUTaskRunner()
? 1
: 2)),
#endif // FLUTTER_SHELL_ENABLE_METAL
pending_frame_semaphore_(1),
frame_number_(1),
paused_(false),
regenerate_layer_tree_(false),
frame_scheduled_(false),
notify_idle_task_id_(0),
dimension_change_pending_(false),
weak_factory_(this) {}
weak_factory_(this) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This may require a clang-format pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how clang-format is putting it - maybe it's confused by the preprocessor macros?

}

Animator::~Animator() = default;

Expand Down
2 changes: 2 additions & 0 deletions shell/gpu/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ source_set("gpu_surface_software") {

source_set("gpu_surface_gl") {
sources = [
"$gpu_dir/gpu_surface_delegate.h",
"$gpu_dir/gpu_surface_gl.cc",
"$gpu_dir/gpu_surface_gl.h",
"$gpu_dir/gpu_surface_gl_delegate.cc",
Expand All @@ -50,6 +51,7 @@ source_set("gpu_surface_vulkan") {

source_set("gpu_surface_metal") {
sources = [
"$gpu_dir/gpu_surface_delegate.h",
"$gpu_dir/gpu_surface_metal.h",
"$gpu_dir/gpu_surface_metal.mm",
]
Expand Down
18 changes: 18 additions & 0 deletions shell/gpu/gpu_surface_delegate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#ifndef FLUTTER_SHELL_GPU_GPU_SURFACE_DELEGATE_H_
#define FLUTTER_SHELL_GPU_GPU_SURFACE_DELEGATE_H_

#include "flutter/flow/embedded_views.h"
#include "flutter/fml/macros.h"

namespace flutter {

class GPUSurfaceDelegate {
public:
// Get a reference to the external views embedder. This happens on the same
// thread that the renderer is operating on.
virtual ExternalViewEmbedder* GetExternalViewEmbedder() = 0;
};

} // namespace flutter

#endif // FLUTTER_SHELL_GPU_GPU_SURFACE_DELEGATE_H_
7 changes: 2 additions & 5 deletions shell/gpu/gpu_surface_gl_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@

#include "flutter/flow/embedded_views.h"
#include "flutter/fml/macros.h"
#include "flutter/shell/gpu/gpu_surface_delegate.h"
#include "third_party/skia/include/core/SkMatrix.h"
#include "third_party/skia/include/gpu/gl/GrGLInterface.h"

namespace flutter {

class GPUSurfaceGLDelegate {
class GPUSurfaceGLDelegate : public GPUSurfaceDelegate {
public:
// Called to make the main GL context current on the current thread.
virtual bool GLContextMakeCurrent() = 0;
Expand Down Expand Up @@ -42,10 +43,6 @@ class GPUSurfaceGLDelegate {
// flushed.
virtual SkMatrix GLContextSurfaceTransformation() const;

// Get a reference to the external views embedder. This happens on the same
// thread that the renderer is operating on.
virtual ExternalViewEmbedder* GetExternalViewEmbedder() = 0;

sk_sp<const GrGLInterface> GetGLInterface() const;

// TODO(chinmaygarde): The presence of this method is to work around the fact
Expand Down
10 changes: 9 additions & 1 deletion shell/gpu/gpu_surface_metal.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "flutter/fml/macros.h"
#include "flutter/fml/platform/darwin/scoped_nsobject.h"
#include "flutter/shell/common/surface.h"
#include "flutter/shell/gpu/gpu_surface_delegate.h"
#include "third_party/skia/include/gpu/GrContext.h"

@class CAMetalLayer;
Expand All @@ -18,11 +19,15 @@ namespace flutter {

class GPUSurfaceMetal : public Surface {
public:
GPUSurfaceMetal(fml::scoped_nsobject<CAMetalLayer> layer);
GPUSurfaceMetal(GPUSurfaceDelegate* delegate, fml::scoped_nsobject<CAMetalLayer> layer);
GPUSurfaceMetal(GPUSurfaceDelegate* delegate,
sk_sp<GrContext> gr_context,
fml::scoped_nsobject<CAMetalLayer> layer);

~GPUSurfaceMetal() override;

private:
GPUSurfaceDelegate* delegate_;
fml::scoped_nsobject<CAMetalLayer> layer_;
sk_sp<GrContext> context_;
fml::scoped_nsprotocol<id<MTLCommandQueue>> command_queue_;
Expand All @@ -39,6 +44,9 @@ class GPUSurfaceMetal : public Surface {
// |Surface|
GrContext* GetContext() override;

// |Surface|
flutter::ExternalViewEmbedder* GetExternalViewEmbedder() override;

// |Surface|
bool MakeRenderContextCurrent() override;

Expand Down
54 changes: 50 additions & 4 deletions shell/gpu/gpu_surface_metal.mm
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@

namespace flutter {

GPUSurfaceMetal::GPUSurfaceMetal(fml::scoped_nsobject<CAMetalLayer> layer)
: layer_(std::move(layer)) {
GPUSurfaceMetal::GPUSurfaceMetal(GPUSurfaceDelegate* delegate,
fml::scoped_nsobject<CAMetalLayer> layer)
: delegate_(delegate), layer_(std::move(layer)) {
if (!layer_) {
FML_LOG(ERROR) << "Could not create metal surface because of invalid layer.";
return;
Expand Down Expand Up @@ -41,6 +42,32 @@
context_ = context;
}

GPUSurfaceMetal::GPUSurfaceMetal(GPUSurfaceDelegate* delegate,
sk_sp<GrContext> gr_context,
fml::scoped_nsobject<CAMetalLayer> layer)
: delegate_(delegate), layer_(std::move(layer)), context_(gr_context) {
if (!layer_) {
FML_LOG(ERROR) << "Could not create metal surface because of invalid layer.";
return;
}
if (!context_) {
FML_LOG(ERROR) << "Could not create metal surface because of invalid Skia metal context.";
return;
}

layer.get().pixelFormat = MTLPixelFormatBGRA8Unorm;

auto metal_device = fml::scoped_nsprotocol<id<MTLDevice>>([layer_.get().device retain]);
auto metal_queue = fml::scoped_nsprotocol<id<MTLCommandQueue>>([metal_device newCommandQueue]);

if (!metal_device || !metal_queue) {
FML_LOG(ERROR) << "Could not create metal device or queue.";
return;
}

command_queue_ = metal_queue;
}

GPUSurfaceMetal::~GPUSurfaceMetal() = default;

// |Surface|
Expand Down Expand Up @@ -112,11 +139,25 @@ GrBackendRenderTarget metal_render_target(bounds.width * scale, // width
return nullptr;
}

auto submit_callback = [drawable = next_drawable, command_buffer](
// External views need to present with transaction. When presenting with
// transaction, we have to block, otherwise we risk presenting the drawable
// after the CATransaction has completed.
// See:
// https://developer.apple.com/documentation/quartzcore/cametallayer/1478157-presentswithtransaction
// TODO(dnfield): only do this if transactions are actually being used.
// https://github.com/flutter/flutter/issues/24133
bool wait = delegate_->GetExternalViewEmbedder() != nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: may be clearer if we call this hasExternalViewEmbedder and move the comment above the if.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


auto submit_callback = [drawable = next_drawable, command_buffer, wait](
const SurfaceFrame& surface_frame, SkCanvas* canvas) -> bool {
canvas->flush();
[command_buffer.get() presentDrawable:drawable.get()];
[command_buffer.get() commit];
if (!wait) {
Copy link
Member

@chinmaygarde chinmaygarde Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, presentDrawable is just waiting (asynchronously) for the command buffer to be scheduled and the and then presenting the same. Since you want the wait to happen now, the following code should be equivalent and more readable.

if (wait) {
 [command_buffer waitUntilScheduled];
}
[command_buffer presentDrawable:drawable];

Nit: Just a suggestion that I found more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little concerned that presentDrawable could end up unexpectedly yielding - the documentation on presentsWithTransaction says to use present instead of presentDrawable for this reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just had to update this - the current formulation doesn't work, we have to call the presentDrawable before we commit otherwise it throws an exception sometimes.

[command_buffer.get() presentDrawable:drawable.get()];
} else {
[command_buffer.get() waitUntilScheduled];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to not block here but instead finish rasterizing, and then do all the waits, present, and commit the transaction?

[Not asking to do it in this PR, just wondering whether it is possible and whether we think it can be more performant]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do as much work as we possibly can before getting the next drawable - guidelines from Apple say to hold it for as little time as possible. But if we use transactions, we have to block before presenting so that we are sure to present in the transaction. Alternatively, we have to make sure to not commit until we've presented in a callback.

[drawable.get() present];
}
return true;
};

Expand All @@ -137,6 +178,11 @@ GrBackendRenderTarget metal_render_target(bounds.width * scale, // width
return context_.get();
}

// |Surface|
flutter::ExternalViewEmbedder* GPUSurfaceMetal::GetExternalViewEmbedder() {
return delegate_->GetExternalViewEmbedder();
}

// |Surface|
bool GPUSurfaceMetal::MakeRenderContextCurrent() {
// This backend has no such concept.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@

- (instancetype)init NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithContentsScale:(CGFloat)contentsScale;
- (std::unique_ptr<flutter::IOSSurface>)createSoftwareSurface;
- (std::unique_ptr<flutter::IOSSurfaceGL>)createGLSurfaceWithContext:
- (std::unique_ptr<flutter::IOSSurface>)createSurface:
(std::shared_ptr<flutter::IOSGLContext>)gl_context;

@end
Expand Down
48 changes: 34 additions & 14 deletions shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "flutter/shell/common/platform_view.h"
#include "flutter/shell/common/rasterizer.h"
#include "flutter/shell/platform/darwin/ios/ios_surface_gl.h"
#include "flutter/shell/platform/darwin/ios/ios_surface_metal.h"
#include "flutter/shell/platform/darwin/ios/ios_surface_software.h"
#include "third_party/skia/include/utils/mac/SkCGUtils.h"

Expand Down Expand Up @@ -51,6 +52,13 @@ - (instancetype)initWithContentsScale:(CGFloat)contentsScale {
layer.allowsGroupOpacity = NO;
layer.contentsScale = contentsScale;
layer.rasterizationScale = contentsScale;
#if FLUTTER_SHELL_ENABLE_METAL
} else if ([self.layer isKindOfClass:[CAMetalLayer class]]) {
CAMetalLayer* layer = reinterpret_cast<CAMetalLayer*>(self.layer);
layer.allowsGroupOpacity = NO;
layer.contentsScale = contentsScale;
layer.rasterizationScale = contentsScale;
#endif // FLUTTER_SHELL_ENABLE_METAL
}

return self;
Expand All @@ -59,27 +67,39 @@ - (instancetype)initWithContentsScale:(CGFloat)contentsScale {
+ (Class)layerClass {
#if TARGET_IPHONE_SIMULATOR
return [CALayer class];
#else // TARGET_IPHONE_SIMULATOR
#else // TARGET_IPHONE_SIMULATOR
#if FLUTTER_SHELL_ENABLE_METAL
return [CAMetalLayer class];
#else // FLUTTER_SHELL_ENABLE_METAL
return [CAEAGLLayer class];
#endif // FLUTTER_SHELL_ENABLE_METAL
#endif // TARGET_IPHONE_SIMULATOR
}

- (std::unique_ptr<flutter::IOSSurface>)createSoftwareSurface {
fml::scoped_nsobject<CALayer> layer(reinterpret_cast<CALayer*>([self.layer retain]));
return std::make_unique<flutter::IOSSurfaceSoftware>(std::move(layer), nullptr);
}

- (std::unique_ptr<flutter::IOSSurfaceGL>)createGLSurfaceWithContext:
- (std::unique_ptr<flutter::IOSSurface>)createSurface:
(std::shared_ptr<flutter::IOSGLContext>)gl_context {
fml::scoped_nsobject<CAEAGLLayer> eagl_layer(reinterpret_cast<CAEAGLLayer*>([self.layer retain]));
// TODO(amirh): We can lower this to iOS 8.0 once we have a Metal rendering backend.
// https://github.com/flutter/flutter/issues/24132
if (@available(iOS 9.0, *)) {
eagl_layer.get().presentsWithTransaction = YES;
if ([self.layer isKindOfClass:[CAEAGLLayer class]]) {
fml::scoped_nsobject<CAEAGLLayer> eagl_layer(
reinterpret_cast<CAEAGLLayer*>([self.layer retain]));
if (@available(iOS 9.0, *)) {
eagl_layer.get().presentsWithTransaction = YES;
}
return std::make_unique<flutter::IOSSurfaceGL>(std::move(eagl_layer), gl_context);
#if FLUTTER_SHELL_ENABLE_METAL
} else if ([self.layer isKindOfClass:[CAMetalLayer class]]) {
fml::scoped_nsobject<CAMetalLayer> metalLayer(
reinterpret_cast<CAMetalLayer*>([self.layer retain]));
if (@available(iOS 8.0, *)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woohoo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a guard for iOS 8 features?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, but it doesn't hurt either right? I can remove if you want.

metalLayer.get().presentsWithTransaction = YES;
}
return std::make_unique<flutter::IOSSurfaceMetal>(std::move(metalLayer));
#endif // FLUTTER_SHELL_ENABLE_METAL
} else {
fml::scoped_nsobject<CALayer> layer(reinterpret_cast<CALayer*>([self.layer retain]));
return std::make_unique<flutter::IOSSurfaceSoftware>(std::move(layer), nullptr);
}
return std::make_unique<flutter::IOSSurfaceGL>(eagl_layer, std::move(gl_context));
}

// TODO(amirh): implement drawLayer to suppoer snapshotting.
// TODO(amirh): implement drawLayer to support snapshotting.

@end
Loading