Skip to content

Commit

Permalink
Sync MTLDevices between ANGLE/Metal and Dawn.
Browse files Browse the repository at this point in the history
Depends on enga's:
https://dawn-review.googlesource.com/c/dawn/+/106760

Use new primitives in Dawn, and the EGL_ANGLE_metal_shared_event_sync
extension in ANGLE's Metal backend, to use MTLSharedEvents for
synchronization between these two unrelated MTLDevices.

Update EGL bindings to version 1.5 to pick up needed eglCreateSync;
eglCreateSyncKHR's signature does not work for
EGL_ANGLE_metal_shared_event_sync. (Needs EGLAttrib, not EGLint.)
Also expose eglCopyMetalSharedEventANGLE. (Could rewrite this code to
manually instantiate and wrap a singleton MTLSharedEvent.)

Fixes the flickering in the test case attached to the bug. Need to
figure out how to catch this bug in an automated test case.

Bug: b/252731382
Change-Id: Ib5d4deeba867139af12c79ecfe364da6d7338564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3995503
Reviewed-by: ccameron chromium <ccameron@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1068955}
  • Loading branch information
kenrussell authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent bfe23fc commit 57b2ff7
Show file tree
Hide file tree
Showing 18 changed files with 918 additions and 2 deletions.
7 changes: 6 additions & 1 deletion components/metal_util/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ assert(is_mac)

import("//testing/test.gni")

source_set("metal_util_types") {
sources = [ "types.h" ]
}

component("metal_util") {
output_name = "metal"

Expand All @@ -21,9 +25,10 @@ component("metal_util") {
"metal_util_export.h",
"test_shader.h",
"test_shader.mm",
"types.h",
]

public_deps = [ ":metal_util_types" ]

deps = [
"//base",
"//components/crash/core/common:crash_key",
Expand Down
4 changes: 4 additions & 0 deletions components/metal_util/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#if __OBJC__
@protocol MTLDevice;
@protocol MTLSharedEvent;
#endif

namespace metal {
Expand All @@ -18,10 +19,13 @@ namespace metal {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunguarded-availability"
using MTLDevicePtr = id<MTLDevice>;
using MTLSharedEventPtr = id<MTLSharedEvent>;
#pragma clang diagnostic pop
#else
class MTLDeviceProtocol;
using MTLDevicePtr = MTLDeviceProtocol*;
class MTLSharedEventProtocol;
using MTLSharedEventPtr = MTLSharedEventProtocol*;
#endif

} // namespace metal
Expand Down
27 changes: 27 additions & 0 deletions gpu/command_buffer/service/shared_image/iosurface_image_backing.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,27 @@ class MemoryIOSurfaceRepresentation : public MemoryImageRepresentation {
scoped_refptr<gl::GLImageMemory> image_memory_;
};

// This class is only put into unique_ptrs and is never copied or assigned.
class SharedEventAndSignalValue {
public:
SharedEventAndSignalValue(id shared_event, uint64_t signaled_value);
~SharedEventAndSignalValue();
SharedEventAndSignalValue(const SharedEventAndSignalValue& other) = delete;
SharedEventAndSignalValue(SharedEventAndSignalValue&& other) = delete;
SharedEventAndSignalValue& operator=(const SharedEventAndSignalValue& other) =
delete;

// Return value is actually id<MTLSharedEvent>.
id shared_event() const { return shared_event_; }

// This is the value which will be signaled on the associated MTLSharedEvent.
uint64_t signaled_value() const { return signaled_value_; }

private:
id shared_event_;
uint64_t signaled_value_;
};

// Implementation of SharedImageBacking that creates a GL Texture that is backed
// by a GLImage and stores it as a gles2::Texture. Can be used with the legacy
// mailbox implementation.
Expand Down Expand Up @@ -165,6 +186,9 @@ class GPU_GLES2_EXPORT IOSurfaceImageBacking
std::unique_ptr<gfx::GpuFence> GetLastWriteGpuFence();
void SetReleaseFence(gfx::GpuFenceHandle release_fence);

void AddSharedEventAndSignalValue(id sharedEvent, uint64_t signalValue);
std::vector<std::unique_ptr<SharedEventAndSignalValue>> TakeSharedEvents();

private:
// SharedImageBacking:
void OnMemoryDump(const std::string& dump_name,
Expand Down Expand Up @@ -234,6 +258,9 @@ class GPU_GLES2_EXPORT IOSurfaceImageBacking
// Wait on this fence before allowing another access.
gfx::GpuFenceHandle release_fence_;

std::vector<std::unique_ptr<SharedEventAndSignalValue>>
shared_events_and_signal_values_;

base::WeakPtrFactory<IOSurfaceImageBacking> weak_factory_;
};

Expand Down
74 changes: 74 additions & 0 deletions gpu/command_buffer/service/shared_image/iosurface_image_backing.mm
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
#include "ui/gl/scoped_binders.h"
#include "ui/gl/trace_util.h"

#include <EGL/egl.h>

#import <Metal/Metal.h>

namespace gpu {

namespace {
Expand Down Expand Up @@ -266,6 +270,28 @@
static_cast<gl::GLImageIOSurface*>(gl_image_.get())->io_surface());
}

////////////////////////////////////////////////////////////////////////////////
// SharedEventAndSignalValue

SharedEventAndSignalValue::SharedEventAndSignalValue(id shared_event,
uint64_t signaled_value)
: shared_event_(shared_event), signaled_value_(signaled_value) {
if (@available(macOS 10.14, *)) {
if (shared_event_) {
[static_cast<id<MTLSharedEvent>>(shared_event_) retain];
}
}
}

SharedEventAndSignalValue::~SharedEventAndSignalValue() {
if (@available(macOS 10.14, *)) {
if (shared_event_) {
[static_cast<id<MTLSharedEvent>>(shared_event_) release];
}
}
shared_event_ = nil;
}

///////////////////////////////////////////////////////////////////////////////
// IOSurfaceImageBacking

Expand Down Expand Up @@ -379,6 +405,18 @@ ScopedRestoreTexture scoped_restore(gl::g_current_gl_context,
release_fence_ = std::move(release_fence);
}

void IOSurfaceImageBacking::AddSharedEventAndSignalValue(
id shared_event,
uint64_t signal_value) {
shared_events_and_signal_values_.push_back(
std::make_unique<SharedEventAndSignalValue>(shared_event, signal_value));
}

std::vector<std::unique_ptr<SharedEventAndSignalValue>>
IOSurfaceImageBacking::TakeSharedEvents() {
return std::move(shared_events_and_signal_values_);
}

void IOSurfaceImageBacking::OnMemoryDump(
const std::string& dump_name,
base::trace_event::MemoryAllocatorDumpGuid client_guid,
Expand Down Expand Up @@ -551,6 +589,24 @@ ScopedRestoreTexture scoped_restore(gl::g_current_gl_context,
if (!gl_texture_->is_bind_pending())
return true;

if (usage() & SHARED_IMAGE_USAGE_WEBGPU &&
gl::GetANGLEImplementation() == gl::ANGLEImplementation::kMetal) {
// If this image could potentially be shared with WebGPU's Metal
// device, it's necessary to synchronize between the two devices.
// If any Metal shared events have been enqueued (the assumption
// is that this was done by the Dawn representation), wait on
// them.
gl::GLDisplayEGL* display = gl::GLDisplayEGL::GetDisplayForCurrentContext();
if (display && display->IsANGLEMetalSharedEventSyncSupported()) {
std::vector<std::unique_ptr<SharedEventAndSignalValue>> signals =
TakeSharedEvents();
for (const auto& signal : signals) {
display->WaitForMetalSharedEvent(signal->shared_event(),
signal->signaled_value());
}
}
}

// Create the EGL surface to bind to the GL texture, if it doesn't exist
// already.
if (!egl_surface_) {
Expand Down Expand Up @@ -642,6 +698,24 @@ ScopedRestoreTexture scoped_restore(gl::g_current_gl_context, GetGLTarget(),

bool needs_synchronization = needs_sync_for_swangle || needs_sync_for_metal;
if (needs_synchronization) {
if (needs_sync_for_metal) {
if (@available(macOS 10.14, *)) {
if (egl_surface_) {
gl::GLDisplayEGL* display =
gl::GLDisplayEGL::GetDisplayForCurrentContext();
if (display) {
metal::MTLSharedEventPtr shared_event = nullptr;
uint64_t signal_value = 0;
if (display->CreateMetalSharedEvent(&shared_event, &signal_value)) {
AddSharedEventAndSignalValue(shared_event, signal_value);
} else {
LOG(DFATAL) << "Failed to create Metal shared event";
}
}
}
}
}

if (!gl_texture_->is_bind_pending()) {
if (egl_surface_) {
ScopedRestoreTexture scoped_restore(gl::g_current_gl_context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "gpu/command_buffer/common/shared_image_usage.h"
#include "gpu/command_buffer/service/mailbox_manager.h"
#include "gpu/command_buffer/service/shared_context_state.h"
#include "gpu/command_buffer/service/shared_image/iosurface_image_backing.h"
#include "gpu/command_buffer/service/shared_image/shared_image_backing.h"
#include "gpu/command_buffer/service/shared_image/shared_image_representation.h"
#include "gpu/command_buffer/service/skia_utils.h"
Expand All @@ -24,6 +25,7 @@
#include "ui/gl/buildflags.h"
#include "ui/gl/gl_context.h"
#include "ui/gl/gl_image_io_surface.h"
#include "ui/gl/gl_implementation.h"

#import <Metal/Metal.h>

Expand Down Expand Up @@ -141,6 +143,30 @@ WGPUTexture BeginAccess(WGPUTextureUsage usage) final {
descriptor.ioSurface = io_surface_.get();
descriptor.plane = 0;

// If the backing is compatible - essentially, a GLImageIOSurface -
// then synchronize with all of the MTLSharedEvents which have been
// stored in it as a consequence of earlier BeginAccess/EndAccess calls
// against other representations.
if (gl::GetANGLEImplementation() == gl::ANGLEImplementation::kMetal) {
if (@available(macOS 10.14, *)) {
SharedImageBacking* backing = this->backing();
// Not possible to reach this with any other type of backing.
DCHECK_EQ(backing->GetType(), SharedImageBackingType::kIOSurface);
IOSurfaceImageBacking* iosurface_backing =
static_cast<IOSurfaceImageBacking*>(backing);
std::vector<std::unique_ptr<SharedEventAndSignalValue>> signals =
iosurface_backing->TakeSharedEvents();
for (const auto& signal : signals) {
dawn::native::metal::ExternalImageMTLSharedEventDescriptor
external_desc;
external_desc.sharedEvent =
static_cast<id<MTLSharedEvent>>(signal->shared_event());
external_desc.signaledValue = signal->signaled_value();
descriptor.waitEvents.push_back(external_desc);
}
}
}

texture_ = dawn::native::metal::WrapIOSurface(device_, &descriptor);
return texture_;
}
Expand All @@ -150,14 +176,39 @@ void EndAccess() final {
return;
}

if (dawn::native::IsTextureSubresourceInitialized(texture_, 0, 1, 0, 1)) {
dawn::native::metal::ExternalImageIOSurfaceEndAccessDescriptor descriptor;
dawn::native::metal::IOSurfaceEndAccess(texture_, &descriptor);

if (descriptor.isInitialized) {
SetCleared();
}

if (gl::GetANGLEImplementation() == gl::ANGLEImplementation::kMetal) {
if (@available(macOS 10.14, *)) {
SharedImageBacking* backing = this->backing();
// Not possible to reach this with any other type of backing.
DCHECK_EQ(backing->GetType(), SharedImageBackingType::kIOSurface);
IOSurfaceImageBacking* iosurface_backing =
static_cast<IOSurfaceImageBacking*>(backing);
// Dawn's Metal backend has enqueued a MTLSharedEvent which
// consumers of the IOSurface must wait upon before attempting to
// use that IOSurface on another MTLDevice. Store this event in
// the underlying SharedImageBacking.
iosurface_backing->AddSharedEventAndSignalValue(
descriptor.sharedEvent, descriptor.signaledValue);
}
}

// All further operations on the textures are errors (they would be racy
// with other backings).
dawn_procs_.textureDestroy(texture_);

// TODO(b/252731382): the following WaitForCommandsToBeScheduled call should
// no longer be necessary, but for some reason it is. Removing it
// reintroduces intermittent renders of black frames to the WebGPU canvas.
// This points to another synchronization bug not resolved by the use of
// MTLSharedEvent between Dawn and ANGLE's Metal backend.
//
// macOS has a global GPU command queue so synchronization between APIs and
// devices is automatic. However on Metal, wgpuQueueSubmit "commits" the
// Metal command buffers but they aren't "scheduled" in the global queue
Expand Down
3 changes: 3 additions & 0 deletions ui/gl/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ component("gl") {
"egl_surface_io_surface.h",
"gl_context_cgl.cc",
"gl_context_cgl.h",
"gl_display_egl.mm",
"gl_fence_apple.cc",
"gl_fence_apple.h",
"gl_image_io_surface.h",
Expand Down Expand Up @@ -426,6 +427,8 @@ component("gl") {
"//third_party/swiftshader/src/Vulkan:swiftshader_libvulkan",
]
}

deps += [ "//components/metal_util:metal_util_types" ]
}
if (is_android) {
defines += [
Expand Down
1 change: 1 addition & 0 deletions ui/gl/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_rules = [
"+cc/base",
"+components/metal_util/types.h",
"+components/viz/common/resources/resource_format.h",
"+mojo/public/cpp/bindings",
"+third_party/khronos",
Expand Down

0 comments on commit 57b2ff7

Please sign in to comment.