From 6d53b01c8d7ee9030c92db8089f2663fc3a8a6bd Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 16 Dec 2014 15:15:22 -0800 Subject: [PATCH] Split surface id and simplify connecting to surfaces service Surface IDs are composed of a namespace component and a local identifier component. This splits up the two pieces in the mojom to make it clearer. This also simplifies the path for connecting to the surfaces service. In order to perform any operations with surfaces, each client must know what its id namespace value is. This was done by first connecting to a SurfacesService interface and then calling CreateSurfaceConnection which returned a Surface pointer and the namespace. Having to connect to this extra interface and receive the Surface impl asynchronously complicated startup code for applications by adding extra states in startup. Instead of that, this allows connecting to the Surface interface directly and promises that the ID namespace will be provided as the first message in the pipe directed at the SurfaceClient. Callers can then either block on startup or handle setting the ID namespace asynchronously depending on what other things that thread could be doing at the time. In a follow-up, I plan to define the id namespace value 0 as meaning "the namespace of this connection" which will allow creating surfaces and submitting frames before learning the connection's namespace. The only thing the namespace will be passing surface IDs to other clients for them to embed. This also removes the Size parameter from CreateSurface, which wasn't used by anything. The size of submitted frames is part of the embedder / embedee contract. R=esprehn@chromium.org, sky@chromium.org Review URL: https://codereview.chromium.org/807733002 --- engine/src/flutter/compositor/surface_allocator.cc | 12 +++++------- engine/src/flutter/compositor/surface_allocator.h | 3 ++- engine/src/flutter/compositor/surface_holder.cc | 7 +++++-- engine/src/flutter/compositor/surface_holder.h | 1 + 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/engine/src/flutter/compositor/surface_allocator.cc b/engine/src/flutter/compositor/surface_allocator.cc index 80cc7a1e1d..7b3d381b41 100644 --- a/engine/src/flutter/compositor/surface_allocator.cc +++ b/engine/src/flutter/compositor/surface_allocator.cc @@ -16,13 +16,11 @@ SurfaceAllocator::SurfaceAllocator(uint32_t id_namespace) SurfaceAllocator::~SurfaceAllocator() { } -uint64_t SurfaceAllocator::CreateSurfaceId() { - // Surface IDs are 64 integers. The high 32 bits are the namespace of the ID, - // which is assigned to us by the surfaces service. The lower 32 bits are ours - // to allocate as we see fit. For simplicity, we just allocate them - // sequentially. In principle, we could run out, but at 60 Hz, it takes - // several years to run out. - return static_cast(id_namespace_) << 32 | next_id_++; +mojo::SurfaceIdPtr SurfaceAllocator::CreateSurfaceId() { + auto id = mojo::SurfaceId::New(); + id->local = next_id_++; + id->id_namespace = id_namespace_; + return id.Pass(); } } // namespace sky diff --git a/engine/src/flutter/compositor/surface_allocator.h b/engine/src/flutter/compositor/surface_allocator.h index 98feb1118a..5da7c6f11e 100644 --- a/engine/src/flutter/compositor/surface_allocator.h +++ b/engine/src/flutter/compositor/surface_allocator.h @@ -6,6 +6,7 @@ #define SKY_COMPOSITOR_SURFACE_ALLOCATOR_H_ #include "base/basictypes.h" +#include "mojo/services/surfaces/public/interfaces/surface_id.mojom.h" namespace sky { @@ -14,7 +15,7 @@ class SurfaceAllocator { SurfaceAllocator(uint32_t id_namespace); ~SurfaceAllocator(); - uint64_t CreateSurfaceId(); + mojo::SurfaceIdPtr CreateSurfaceId(); private: uint32_t id_namespace_; diff --git a/engine/src/flutter/compositor/surface_holder.cc b/engine/src/flutter/compositor/surface_holder.cc index 26d2098c96..e8475802f3 100644 --- a/engine/src/flutter/compositor/surface_holder.cc +++ b/engine/src/flutter/compositor/surface_holder.cc @@ -51,13 +51,16 @@ void SurfaceHolder::SetSize(const gfx::Size& size) { surface_id_ = mojo::SurfaceId::New(); } - surface_id_->id = surface_allocator_->CreateSurfaceId(); - surface_->CreateSurface(surface_id_.Clone(), mojo::Size::From(size)); + surface_id_ = surface_allocator_->CreateSurfaceId(); + surface_->CreateSurface(surface_id_.Clone()); size_ = size; client_->OnSurfaceIdAvailable(surface_id_.Clone()); } +void SurfaceHolder::SetIdNamespace(uint32_t id_namespace) { +} + void SurfaceHolder::ReturnResources( mojo::Array resources) { // TODO(abarth): The surface service shouldn't spam us with empty calls. diff --git a/engine/src/flutter/compositor/surface_holder.h b/engine/src/flutter/compositor/surface_holder.h index e33bce31c9..9bf5ad25f4 100644 --- a/engine/src/flutter/compositor/surface_holder.h +++ b/engine/src/flutter/compositor/surface_holder.h @@ -43,6 +43,7 @@ class SurfaceHolder : public mojo::SurfaceClient { private: // mojo::SurfaceClient + void SetIdNamespace(uint32_t id_namespace) override; void ReturnResources( mojo::Array resources) override;