From 5d4e44fa925a42afcdffdf1999d22d313ae2630d Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Fri, 18 Sep 2015 12:06:27 -0700 Subject: [PATCH 1/3] Remove SurfaceAllocator because it is unused. --- services/sky/compositor/BUILD.gn | 2 -- services/sky/compositor/surface_allocator.cc | 26 ------------------ services/sky/compositor/surface_allocator.h | 29 -------------------- services/sky/compositor/surface_holder.cc | 1 - services/sky/compositor/surface_holder.h | 2 -- 5 files changed, 60 deletions(-) delete mode 100644 services/sky/compositor/surface_allocator.cc delete mode 100644 services/sky/compositor/surface_allocator.h diff --git a/services/sky/compositor/BUILD.gn b/services/sky/compositor/BUILD.gn index ead14b1f255..9824792c15c 100644 --- a/services/sky/compositor/BUILD.gn +++ b/services/sky/compositor/BUILD.gn @@ -18,8 +18,6 @@ source_set("compositor") { "rasterizer_ganesh.h", "resource_manager.cc", "resource_manager.h", - "surface_allocator.cc", - "surface_allocator.h", "surface_holder.cc", "surface_holder.h", "texture_cache.cc", diff --git a/services/sky/compositor/surface_allocator.cc b/services/sky/compositor/surface_allocator.cc deleted file mode 100644 index d5176b8c185..00000000000 --- a/services/sky/compositor/surface_allocator.cc +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "services/sky/compositor/surface_allocator.h" - -#include "base/logging.h" - -namespace sky { - -SurfaceAllocator::SurfaceAllocator(uint32_t id_namespace) - : id_namespace_(id_namespace), next_id_(1) { - DCHECK(id_namespace); -} - -SurfaceAllocator::~SurfaceAllocator() { -} - -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/services/sky/compositor/surface_allocator.h b/services/sky/compositor/surface_allocator.h deleted file mode 100644 index 86c350e92dd..00000000000 --- a/services/sky/compositor/surface_allocator.h +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef SKY_VIEWER_COMPOSITOR_SURFACE_ALLOCATOR_H_ -#define SKY_VIEWER_COMPOSITOR_SURFACE_ALLOCATOR_H_ - -#include "base/basictypes.h" -#include "mojo/services/surfaces/public/interfaces/surface_id.mojom.h" - -namespace sky { - -class SurfaceAllocator { - public: - SurfaceAllocator(uint32_t id_namespace); - ~SurfaceAllocator(); - - mojo::SurfaceIdPtr CreateSurfaceId(); - - private: - uint32_t id_namespace_; - uint32_t next_id_; - - DISALLOW_COPY_AND_ASSIGN(SurfaceAllocator); -}; - -} // namespace sky - -#endif // SKY_VIEWER_COMPOSITOR_SURFACE_ALLOCATOR_H_ diff --git a/services/sky/compositor/surface_holder.cc b/services/sky/compositor/surface_holder.cc index 87f06afdbd4..2bf727cbd66 100644 --- a/services/sky/compositor/surface_holder.cc +++ b/services/sky/compositor/surface_holder.cc @@ -9,7 +9,6 @@ #include "mojo/converters/geometry/geometry_type_converters.h" #include "mojo/public/cpp/application/connect.h" #include "mojo/public/interfaces/application/shell.mojom.h" -#include "services/sky/compositor/surface_allocator.h" namespace sky { diff --git a/services/sky/compositor/surface_holder.h b/services/sky/compositor/surface_holder.h index cd34c90e8aa..c8f82d8ccc7 100644 --- a/services/sky/compositor/surface_holder.h +++ b/services/sky/compositor/surface_holder.h @@ -18,8 +18,6 @@ class Shell; } namespace sky { -class SurfaceAllocator; - class SurfaceHolder : public mojo::ResourceReturner { public: class Client { From 4d8e765423c13dee08d5e63063df274585383f46 Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Fri, 18 Sep 2015 17:23:28 -0700 Subject: [PATCH 2/3] Run sky_viewer directly on top of native_viewport / surfaces. Remove view manager from consideration for now. Although we could also remove the dependency on surfaces, it makes sense to keep it for a little while longer since the replacement for view manager will likely depend on it or a similar compositor. --- services/sky/document_view.cc | 188 +++++++++++++++++++--------------- services/sky/document_view.h | 57 ++++------- 2 files changed, 127 insertions(+), 118 deletions(-) diff --git a/services/sky/document_view.cc b/services/sky/document_view.cc index 80336f36e24..3247deeae26 100644 --- a/services/sky/document_view.cc +++ b/services/sky/document_view.cc @@ -15,9 +15,8 @@ #include "mojo/public/cpp/application/connect.h" #include "mojo/public/cpp/system/data_pipe.h" #include "mojo/public/interfaces/application/shell.mojom.h" -#include "mojo/services/view_manager/public/cpp/view.h" -#include "mojo/services/view_manager/public/cpp/view_manager.h" -#include "mojo/services/view_manager/public/interfaces/view_manager.mojom.h" +#include "mojo/services/surfaces/public/cpp/surfaces_utils.h" +#include "mojo/services/surfaces/public/interfaces/quads.mojom.h" #include "services/asset_bundle/asset_unpacker_job.h" #include "services/sky/compositor/layer_host.h" #include "services/sky/compositor/rasterizer_bitmap.h" @@ -73,25 +72,22 @@ scoped_ptr ConvertToUITouchEvent( } // namespace DocumentView::DocumentView( - mojo::InterfaceRequest services, - mojo::ServiceProviderPtr exported_services, + mojo::InterfaceRequest exported_services, + mojo::ServiceProviderPtr imported_services, mojo::URLResponsePtr response, mojo::Shell* shell) : response_(response.Pass()), - exported_services_(services.Pass()), - imported_services_(exported_services.Pass()), + exported_services_(exported_services.Pass()), + imported_services_(imported_services.Pass()), shell_(shell), - root_(nullptr), - view_manager_client_factory_(shell_, this), bitmap_rasterizer_(nullptr), + event_dispatcher_binding_(this), weak_factory_(this) { - exported_services_.AddService(&view_manager_client_factory_); InitServiceRegistry(); + InitViewport(); } DocumentView::~DocumentView() { - if (root_) - root_->RemoveObserver(this); ui::GestureRecognizer::Get()->CleanupStateForConsumer(this); } @@ -99,32 +95,67 @@ base::WeakPtr DocumentView::GetWeakPtr() { return weak_factory_.GetWeakPtr(); } -void DocumentView::OnEmbed( - mojo::View* root, - mojo::InterfaceRequest services_provided_to_embedder, - mojo::ServiceProviderPtr services_provided_by_embedder) { - root_ = root; +void DocumentView::InitViewport() { + mojo::ServiceProviderPtr viewport_service_provider; + shell_->ConnectToApplication("mojo:native_viewport_service", + mojo::GetProxy(&viewport_service_provider), + nullptr); + mojo::ConnectToService(viewport_service_provider.get(), &viewport_service_); + viewport_service_.set_connection_error_handler( + base::Bind(&DocumentView::OnViewportConnectionError, + base::Unretained(this))); - if (services_provided_by_embedder.get()) { - mojo::ConnectToService(services_provided_by_embedder.get(), - &navigator_host_); - } + mojo::NativeViewportEventDispatcherPtr dispatcher; + event_dispatcher_binding_.Bind(GetProxy(&dispatcher)); + viewport_service_->SetEventDispatcher(dispatcher.Pass()); - services_provided_to_embedder_ = services_provided_to_embedder.Pass(); - services_provided_by_embedder_ = services_provided_by_embedder.Pass(); + // Match the Nexus 5 aspect ratio initially. + auto size = mojo::Size::New(); + size->width = 320; + size->height = 640; - Load(response_.Pass()); + auto requested_configuration = mojo::SurfaceConfiguration::New(); - UpdateRootSizeAndViewportMetrics(root_->bounds()); - root_->AddObserver(this); + viewport_service_->Create(size.Clone(), + requested_configuration.Pass(), + base::Bind(&DocumentView::OnViewportCreated, + base::Unretained(this))); } -void DocumentView::OnViewManagerDisconnected(mojo::ViewManager* view_manager) { - // TODO(ksimbili): Need to figure out how to shutdown when view manager - // doesn't connect. +void DocumentView::OnViewportConnectionError() { delete this; } +void DocumentView::OnViewportCreated(mojo::ViewportMetricsPtr metrics) { + viewport_service_->Show(); + mojo::ContextProviderPtr onscreen_context_provider; + viewport_service_->GetContextProvider(GetProxy(&onscreen_context_provider)); + + mojo::ServiceProviderPtr surfaces_service_provider; + shell_->ConnectToApplication("mojo:surfaces_service", + mojo::GetProxy(&surfaces_service_provider), + nullptr); + mojo::DisplayFactoryPtr display_factory; + mojo::ConnectToService(surfaces_service_provider.get(), &display_factory); + display_factory->Create(onscreen_context_provider.Pass(), + nullptr, GetProxy(&display_)); + + Load(response_.Pass()); + UpdateViewportMetrics(metrics.Pass()); + RequestUpdatedViewportMetrics(); +} + +void DocumentView::OnViewportMetricsChanged(mojo::ViewportMetricsPtr metrics) { + UpdateViewportMetrics(metrics.Pass()); + RequestUpdatedViewportMetrics(); +} + +void DocumentView::RequestUpdatedViewportMetrics() { + viewport_service_->RequestMetrics( + base::Bind(&DocumentView::OnViewportMetricsChanged, + base::Unretained(this))); +} + void DocumentView::LoadFromSnapshotStream( String name, mojo::ScopedDataPipeConsumerHandle snapshot) { if (sky_view_) { @@ -171,11 +202,15 @@ mojo::ScopedMessagePipeHandle DocumentView::TakeRootBundleHandle() { } mojo::ScopedMessagePipeHandle DocumentView::TakeServicesProvidedToEmbedder() { - return services_provided_to_embedder_.PassMessagePipe(); + // TODO(jeffbrown): Stubbed out until we migrate from native viewport + // to a new view system that supports embedding again. + return mojo::ScopedMessagePipeHandle(); } mojo::ScopedMessagePipeHandle DocumentView::TakeServicesProvidedByEmbedder() { - return services_provided_by_embedder_.PassInterface().PassHandle(); + // TODO(jeffbrown): Stubbed out until we migrate from native viewport + // to a new view system that supports embedding again. + return mojo::ScopedMessagePipeHandle(); } mojo::ScopedMessagePipeHandle DocumentView::TakeServiceRegistry() { @@ -196,8 +231,29 @@ void DocumentView::BeginFrame(base::TimeTicks frame_time) { } void DocumentView::OnSurfaceIdAvailable(mojo::SurfaceIdPtr surface_id) { - if (root_) - root_->SetSurfaceId(surface_id.Pass()); + mojo::FramePtr frame = mojo::Frame::New(); + frame->resources.resize(0u); + + mojo::Rect bounds; + bounds.width = viewport_metrics_->size->width; + bounds.height = viewport_metrics_->size->height; + mojo::PassPtr pass = mojo::CreateDefaultPass(1, bounds); + pass->shared_quad_states.push_back(mojo::CreateDefaultSQS( + *viewport_metrics_->size)); + + mojo::QuadPtr quad = mojo::Quad::New(); + quad->material = mojo::MATERIAL_SURFACE_CONTENT; + quad->rect = bounds.Clone(); + quad->opaque_rect = bounds.Clone(); + quad->visible_rect = bounds.Clone(); + quad->shared_quad_state_index = 0u; + quad->surface_quad_state = mojo::SurfaceQuadState::New(); + quad->surface_quad_state->surface = surface_id.Pass(); + + pass->quads.push_back(quad.Pass()); + frame->passes.push_back(pass.Pass()); + + display_->SubmitFrame(frame.Pass(), base::Bind(&base::DoNothing)); } void DocumentView::PaintContents(SkCanvas* canvas, const gfx::Rect& clip) { @@ -208,12 +264,6 @@ void DocumentView::PaintContents(SkCanvas* canvas, const gfx::Rect& clip) { } } -float DocumentView::GetDevicePixelRatio() const { - if (root_) - return root_->viewport_metrics().device_pixel_ratio; - return 1.f; -} - void DocumentView::DidCreateIsolate(Dart_Isolate isolate) { Internals::Create(isolate, this); } @@ -222,49 +272,30 @@ mojo::NavigatorHost* DocumentView::NavigatorHost() { return navigator_host_.get(); } -void DocumentView::OnViewBoundsChanged(mojo::View* view, - const mojo::Rect& old_bounds, - const mojo::Rect& new_bounds) { - DCHECK_EQ(view, root_); - UpdateRootSizeAndViewportMetrics(new_bounds); -} - -void DocumentView::OnViewViewportMetricsChanged( - mojo::View* view, - const mojo::ViewportMetrics& old_metrics, - const mojo::ViewportMetrics& new_metrics) { - DCHECK_EQ(view, root_); - - UpdateRootSizeAndViewportMetrics(root_->bounds()); -} - -void DocumentView::UpdateRootSizeAndViewportMetrics( - const mojo::Rect& new_bounds) { - float device_pixel_ratio = GetDevicePixelRatio(); +void DocumentView::UpdateViewportMetrics( + mojo::ViewportMetricsPtr viewport_metrics) { + viewport_metrics_ = viewport_metrics.Pass(); if (sky_view_) { blink::SkyDisplayMetrics metrics; - mojo::Rect bounds = root_->bounds(); - metrics.physical_size = blink::WebSize(bounds.width, bounds.height); - metrics.device_pixel_ratio = device_pixel_ratio; + metrics.physical_size = blink::WebSize( + viewport_metrics_->size->width, + viewport_metrics_->size->height); + metrics.device_pixel_ratio = viewport_metrics_->device_pixel_ratio; sky_view_->SetDisplayMetrics(metrics); - return; } } -void DocumentView::OnViewFocusChanged(mojo::View* gained_focus, - mojo::View* lost_focus) { +void DocumentView::OnEvent(mojo::EventPtr event, + const mojo::Callback& callback) { + HandleInputEvent(event.Pass()); + callback.Run(); } -void DocumentView::OnViewDestroyed(mojo::View* view) { - DCHECK_EQ(view, root_); - - root_ = nullptr; -} - -void DocumentView::OnViewInputEvent( - mojo::View* view, const mojo::EventPtr& event) { - float device_pixel_ratio = GetDevicePixelRatio(); +void DocumentView::HandleInputEvent(mojo::EventPtr event) { + if (!viewport_metrics_) + return; + float device_pixel_ratio = viewport_metrics_->device_pixel_ratio; scoped_ptr web_event = ConvertEvent(event, device_pixel_ratio); if (!web_event) @@ -302,15 +333,8 @@ void DocumentView::StartDebuggerInspectorBackend() { } void DocumentView::InitServiceRegistry() { - mojo::ConnectToService(imported_services_.get(), &service_registry_); - mojo::Array interface_names(1); - interface_names[0] = mojo::ViewManagerClient::Name_; - mojo::ServiceProviderImpl* sp_impl(new mojo::ServiceProviderImpl()); - sp_impl->AddService(&view_manager_client_factory_); - mojo::ServiceProviderPtr sp; - service_registry_service_provider_binding_.reset( - new mojo::StrongBinding(sp_impl, &sp)); - service_registry_->AddServices(interface_names.Pass(), sp.Pass()); + if (imported_services_) + mojo::ConnectToService(imported_services_.get(), &service_registry_); } void DocumentView::ScheduleFrame() { diff --git a/services/sky/document_view.h b/services/sky/document_view.h index 36907cf1d1a..50e27d1ef88 100644 --- a/services/sky/document_view.h +++ b/services/sky/document_view.h @@ -13,12 +13,11 @@ #include "mojo/public/interfaces/application/application.mojom.h" #include "mojo/services/asset_bundle/public/interfaces/asset_bundle.mojom.h" #include "mojo/services/content_handler/public/interfaces/content_handler.mojom.h" +#include "mojo/services/native_viewport/public/interfaces/native_viewport.mojom.h" #include "mojo/services/navigation/public/interfaces/navigation.mojom.h" #include "mojo/services/network/public/interfaces/url_loader.mojom.h" #include "mojo/services/service_registry/public/interfaces/service_registry.mojom.h" -#include "mojo/services/view_manager/public/cpp/view_manager_client_factory.h" -#include "mojo/services/view_manager/public/cpp/view_manager_delegate.h" -#include "mojo/services/view_manager/public/cpp/view_observer.h" +#include "mojo/services/surfaces/public/interfaces/display.mojom.h" #include "services/sky/compositor/layer_client.h" #include "services/sky/compositor/layer_host_client.h" #include "sky/compositor/layer_tree.h" @@ -27,11 +26,6 @@ #include "sky/engine/public/sky/sky_view_client.h" #include "ui/events/gestures/gesture_types.h" -namespace mojo { -class ViewManager; -class View; -} - namespace sky { class DartLibraryProviderImpl; class LayerHost; @@ -41,14 +35,13 @@ class TextureLayer; class DocumentView : public blink::ServiceProvider, public blink::SkyViewClient, - public mojo::ViewManagerDelegate, - public mojo::ViewObserver, + public mojo::NativeViewportEventDispatcher, public sky::LayerClient, public sky::LayerHostClient, public ui::GestureConsumer { public: - DocumentView(mojo::InterfaceRequest services, - mojo::ServiceProviderPtr exported_services, + DocumentView(mojo::InterfaceRequest exported_services, + mojo::ServiceProviderPtr imported_services, mojo::URLResponsePtr response, mojo::Shell* shell); ~DocumentView() override; @@ -83,24 +76,14 @@ class DocumentView : public blink::ServiceProvider, // Services methods: mojo::NavigatorHost* NavigatorHost() override; - // ViewManagerDelegate methods: - void OnEmbed(mojo::View* root, - mojo::InterfaceRequest services, - mojo::ServiceProviderPtr exposed_services) override; - void OnViewManagerDisconnected(mojo::ViewManager* view_manager) override; + // NativeViewportEventDispatcher methods: + void OnEvent(mojo::EventPtr event, + const mojo::Callback& callback) override; - // ViewObserver methods: - void OnViewBoundsChanged(mojo::View* view, - const mojo::Rect& old_bounds, - const mojo::Rect& new_bounds) override; - void OnViewViewportMetricsChanged( - mojo::View* view, - const mojo::ViewportMetrics& old_metrics, - const mojo::ViewportMetrics& new_metrics) override; - void OnViewFocusChanged(mojo::View* gained_focus, - mojo::View* lost_focus) override; - void OnViewDestroyed(mojo::View* view) override; - void OnViewInputEvent(mojo::View* view, const mojo::EventPtr& event) override; + void OnViewportConnectionError(); + void OnViewportCreated(mojo::ViewportMetricsPtr metrics); + void OnViewportMetricsChanged(mojo::ViewportMetricsPtr metrics); + void RequestUpdatedViewportMetrics(); void Load(mojo::URLResponsePtr response); float GetDevicePixelRatio() const; @@ -109,29 +92,31 @@ class DocumentView : public blink::ServiceProvider, void LoadFromSnapshotStream(String name, mojo::ScopedDataPipeConsumerHandle snapshot); - void UpdateRootSizeAndViewportMetrics(const mojo::Rect& new_bounds); + void UpdateViewportMetrics(mojo::ViewportMetricsPtr viewport_metrics); + + void HandleInputEvent(mojo::EventPtr event); void InitServiceRegistry(); + void InitViewport(); mojo::URLResponsePtr response_; mojo::ServiceProviderImpl exported_services_; mojo::ServiceProviderPtr imported_services_; - mojo::InterfaceRequest services_provided_to_embedder_; - mojo::ServiceProviderPtr services_provided_by_embedder_; + mojo::NativeViewportPtr viewport_service_; mojo::Shell* shell_; mojo::asset_bundle::AssetBundlePtr root_bundle_; mojo::NavigatorHostPtr navigator_host_; std::unique_ptr sky_view_; - mojo::View* root_; - mojo::ViewManagerClientFactory view_manager_client_factory_; + scoped_ptr library_provider_; scoped_ptr layer_host_; scoped_refptr root_layer_; std::unique_ptr current_layer_tree_; // TODO(abarth): Integrate //sky/compositor and //services/sky/compositor. compositor::PaintContext paint_context_; RasterizerBitmap* bitmap_rasterizer_; // Used for pixel tests. mojo::ServiceRegistryPtr service_registry_; - scoped_ptr> - service_registry_service_provider_binding_; + mojo::Binding event_dispatcher_binding_; + mojo::ViewportMetricsPtr viewport_metrics_; + mojo::DisplayPtr display_; base::WeakPtrFactory weak_factory_; From c483b2e2e65f393f594667d8a977622afbba26fc Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Mon, 21 Sep 2015 20:09:29 -0700 Subject: [PATCH 3/3] Only pointers which are down should be tracked. This change ensures that we only store the results of a hit test on the initial pointer down event. Moreover, we perform new hit tests each time a hovering pointer moves. This is important to ensure correct behavior of input devices which can hover, such as mice. Previously the first hover movement after releasing a mouse button would cause a new pointer state to be recorded along with hit test results for wherever the pointer happened to be which caused the following pointer down event to be delivered to the wrong place. Fixes issue #1189. --- .../sky/lib/src/rendering/sky_binding.dart | 57 +++++++++++-------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/sky/packages/sky/lib/src/rendering/sky_binding.dart b/sky/packages/sky/lib/src/rendering/sky_binding.dart index 971de744284..02a89bd6c5f 100644 --- a/sky/packages/sky/lib/src/rendering/sky_binding.dart +++ b/sky/packages/sky/lib/src/rendering/sky_binding.dart @@ -105,37 +105,44 @@ class SkyBinding extends HitTestTarget { /// A router that routes all pointer events received from the engine final PointerRouter pointerRouter = new PointerRouter(); + /// State for all pointers which are currently down. + /// We do not track the state of hovering pointers because we need + /// to hit-test them on each movement. Map _stateForPointer = new Map(); - _PointerState _createStateForPointer(sky.PointerEvent event, Point position) { - HitTestResult result = hitTest(position); - _PointerState state = new _PointerState(result: result, lastPosition: position); - _stateForPointer[event.pointer] = state; - return state; - } - - _PointerState _getOrCreateStateForPointer(event, position) { - _PointerState state = _stateForPointer[event.pointer]; - if (state == null) - state = _createStateForPointer(event, position); - return state; - } - void _handlePointerEvent(sky.PointerEvent event) { Point position = new Point(event.x, event.y); - _PointerState state = _getOrCreateStateForPointer(event, position); - - if (event.type == 'pointerup' || event.type == 'pointercancel') { - if (_hammingWeight(event.buttons) <= 1) - _stateForPointer.remove(event.pointer); + _PointerState state = _stateForPointer[event.pointer]; + switch (event.type) { + case 'pointerdown': + if (state == null) { + state = new _PointerState(result: hitTest(position), lastPosition: position); + _stateForPointer[event.pointer] = state; + } + break; + case 'pointermove': + if (state == null) { + // The pointer is hovering, ignore it for now since we don't + // know what to do with it yet. + return; + } + event.dx = position.x - state.lastPosition.x; + event.dy = position.y - state.lastPosition.y; + state.lastPosition = position; + break; + case 'pointerup': + case 'pointercancel': + if (state == null) { + // This seems to be a spurious event. Ignore it. + return; + } + // Only remove the pointer state when the last button has been released. + if (_hammingWeight(event.buttons) <= 1) + _stateForPointer.remove(event.pointer); + break; } - - event.dx = position.x - state.lastPosition.x; - event.dy = position.y - state.lastPosition.y; - state.lastPosition = position; - - return dispatchEvent(event, state.result); + dispatchEvent(event, state.result); } /// Determine which [HitTestTarget] objects are located at a given position