From 3ab6eab14c673956a453e982327a8e464fe0e19f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 14 Jan 2015 10:25:31 -0800 Subject: [PATCH] Use local ids for Surfaces APIs that can only apply to local surfaces Surfaces identifiers have a local and a namespace component. The namespace is particular to a connection. The local component is up to the endpoint of the connection to manage. In contexts where a surface that may be from anywhere is referenced, a fully qualified ID with both the local and namespace component is needed in order to be unambiguous. In contexts that can only apply to local surfaces only the local id is needed. This updates the mojo.Surface APIs that can only refer to local IDs to only take the local component. In particular creating, destroying, or submitting a frame can only refer to surfaces created on that connection. References to surfaces within a frame may refer to local or foreign surfaces so they use fully qualified IDs. This also skips the SurfacesService indirection since many applications can perform useful operations on a mojo.Surface interface such as create surfaces and submit frames without knowing their ID namespace. The namespace component is needed only to pass fully qualified IDs to other actors that may wish to reference the produced frame. R=sky@chromium.org Review URL: https://codereview.chromium.org/826423008 --- engine/src/flutter/compositor/layer_host.cc | 9 +--- engine/src/flutter/compositor/layer_host.h | 2 - .../src/flutter/compositor/surface_holder.cc | 52 ++++++++----------- .../src/flutter/compositor/surface_holder.h | 11 ++-- 4 files changed, 27 insertions(+), 47 deletions(-) diff --git a/engine/src/flutter/compositor/layer_host.cc b/engine/src/flutter/compositor/layer_host.cc index 1739c78e41..d8ca387d14 100644 --- a/engine/src/flutter/compositor/layer_host.cc +++ b/engine/src/flutter/compositor/layer_host.cc @@ -16,7 +16,7 @@ namespace sky { LayerHost::LayerHost(LayerHostClient* client) : client_(client), - state_(kWaitingForSurfaceService), + state_(kReadyForFrame), frame_requested_(false), surface_holder_(this, client->GetShell()), gl_context_owner_(client->GetShell()), @@ -46,13 +46,6 @@ void LayerHost::GetPixelsForTesting(std::vector* pixels) { return root_layer_->GetPixelsForTesting(pixels); } -void LayerHost::OnSurfaceConnectionCreated() { - DCHECK_EQ(state_, kWaitingForSurfaceService); - state_ = kReadyForFrame; - if (frame_requested_) - BeginFrameSoon(); -} - void LayerHost::OnSurfaceIdAvailable(mojo::SurfaceIdPtr surface_id) { client_->OnSurfaceIdAvailable(surface_id.Pass()); } diff --git a/engine/src/flutter/compositor/layer_host.h b/engine/src/flutter/compositor/layer_host.h index 390bc843c1..377dec95c3 100644 --- a/engine/src/flutter/compositor/layer_host.h +++ b/engine/src/flutter/compositor/layer_host.h @@ -45,13 +45,11 @@ class LayerHost : public SurfaceHolder::Client { private: enum State { - kWaitingForSurfaceService, kReadyForFrame, kWaitingForFrameAcknowldgement, }; // SurfaceHolder::Client - void OnSurfaceConnectionCreated() override; void OnSurfaceIdAvailable(mojo::SurfaceIdPtr surface_id) override; void ReturnResources( mojo::Array resources) override; diff --git a/engine/src/flutter/compositor/surface_holder.cc b/engine/src/flutter/compositor/surface_holder.cc index e8475802f3..f2ad3297a0 100644 --- a/engine/src/flutter/compositor/surface_holder.cc +++ b/engine/src/flutter/compositor/surface_holder.cc @@ -17,48 +17,50 @@ SurfaceHolder::Client::~Client() { } SurfaceHolder::SurfaceHolder(Client* client, mojo::Shell* shell) - : client_(client), weak_factory_(this) { + : client_(client), id_namespace_(0u), local_id_(0u), weak_factory_(this) { mojo::ServiceProviderPtr service_provider; shell->ConnectToApplication("mojo:surfaces_service", mojo::GetProxy(&service_provider)); - mojo::ConnectToService(service_provider.get(), &surfaces_service_); - - surfaces_service_->CreateSurfaceConnection(base::Bind( - &SurfaceHolder::OnSurfaceConnectionCreated, weak_factory_.GetWeakPtr())); + mojo::ConnectToService(service_provider.get(), &surface_); + surface_.set_client(this); } SurfaceHolder::~SurfaceHolder() { - if (surface_ && surface_id_) - surface_->DestroySurface(surface_id_.Clone()); -} - -bool SurfaceHolder::IsReadyForFrame() const { - return surface_; + if (local_id_ != 0u) + surface_->DestroySurface(local_id_); } void SurfaceHolder::SubmitFrame(mojo::FramePtr frame, const base::Closure& callback) { - surface_->SubmitFrame(surface_id_.Clone(), frame.Pass(), callback); + surface_->SubmitFrame(local_id_, frame.Pass(), callback); } void SurfaceHolder::SetSize(const gfx::Size& size) { - if (surface_id_ && size_ == size) + if (local_id_ != 0u && size_ == size) return; - if (surface_id_) { - surface_->DestroySurface(surface_id_.Clone()); - } else { - surface_id_ = mojo::SurfaceId::New(); - } + if (local_id_ != 0u) + surface_->DestroySurface(local_id_); - surface_id_ = surface_allocator_->CreateSurfaceId(); - surface_->CreateSurface(surface_id_.Clone()); + local_id_++; + surface_->CreateSurface(local_id_); size_ = size; - client_->OnSurfaceIdAvailable(surface_id_.Clone()); + if (id_namespace_ != 0u) + SetQualifiedId(); +} + +void SurfaceHolder::SetQualifiedId() { + auto qualified_id = mojo::SurfaceId::New(); + qualified_id->id_namespace = id_namespace_; + qualified_id->local = local_id_; + client_->OnSurfaceIdAvailable(qualified_id.Pass()); } void SurfaceHolder::SetIdNamespace(uint32_t id_namespace) { + id_namespace_ = id_namespace; + if (local_id_ != 0u) + SetQualifiedId(); } void SurfaceHolder::ReturnResources( @@ -69,12 +71,4 @@ void SurfaceHolder::ReturnResources( client_->ReturnResources(resources.Pass()); } -void SurfaceHolder::OnSurfaceConnectionCreated(mojo::SurfacePtr surface, - uint32_t id_namespace) { - surface_ = surface.Pass(); - surface_.set_client(this); - surface_allocator_.reset(new SurfaceAllocator(id_namespace)); - client_->OnSurfaceConnectionCreated(); -} - } // namespace sky diff --git a/engine/src/flutter/compositor/surface_holder.h b/engine/src/flutter/compositor/surface_holder.h index 9bf5ad25f4..78a073f7c5 100644 --- a/engine/src/flutter/compositor/surface_holder.h +++ b/engine/src/flutter/compositor/surface_holder.h @@ -24,7 +24,6 @@ class SurfaceHolder : public mojo::SurfaceClient { public: class Client { public: - virtual void OnSurfaceConnectionCreated() = 0; virtual void OnSurfaceIdAvailable(mojo::SurfaceIdPtr surface_id) = 0; virtual void ReturnResources( mojo::Array resources) = 0; @@ -36,8 +35,6 @@ class SurfaceHolder : public mojo::SurfaceClient { explicit SurfaceHolder(Client* client, mojo::Shell* shell); ~SurfaceHolder() override; - bool IsReadyForFrame() const; - void SetSize(const gfx::Size& size); void SubmitFrame(mojo::FramePtr frame, const base::Closure& callback); @@ -47,15 +44,13 @@ class SurfaceHolder : public mojo::SurfaceClient { void ReturnResources( mojo::Array resources) override; - void OnSurfaceConnectionCreated(mojo::SurfacePtr surface, - uint32_t id_namespace); + void SetQualifiedId(); Client* client_; gfx::Size size_; - scoped_ptr surface_allocator_; - mojo::SurfacesServicePtr surfaces_service_; + uint32_t id_namespace_; + uint32_t local_id_; mojo::SurfacePtr surface_; - mojo::SurfaceIdPtr surface_id_; base::WeakPtrFactory weak_factory_;