From 5644d4c0ecc75e3740d61d854880e3f3963016e1 Mon Sep 17 00:00:00 2001 From: Michael Spang Date: Fri, 16 Jan 2015 18:29:41 -0500 Subject: [PATCH] [Ozone-DRI] Add display observer Introduce a display change observer and make the window delegates update their cached HardwareDisplayController pointer as a result of changes in ScreenManager. BUG=446184 NOTRY=true Review URL: https://codereview.chromium.org/844343002 Cr-Commit-Position: refs/heads/master@{#311112} Review URL: https://codereview.chromium.org/855153002 Cr-Commit-Position: refs/branch-heads/2272@{#40} Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958} --- ui/ozone/platform/dri/BUILD.gn | 1 + .../platform/dri/display_change_observer.h | 26 +++++++++ ui/ozone/platform/dri/dri.gypi | 1 + .../platform/dri/dri_window_delegate_impl.cc | 30 +++++++++- .../platform/dri/dri_window_delegate_impl.h | 21 ++++--- ui/ozone/platform/dri/screen_manager.cc | 28 +++++++-- ui/ozone/platform/dri/screen_manager.h | 18 +++--- .../platform/dri/screen_manager_unittest.cc | 58 +++++++++++++++++-- 8 files changed, 156 insertions(+), 27 deletions(-) create mode 100644 ui/ozone/platform/dri/display_change_observer.h diff --git a/ui/ozone/platform/dri/BUILD.gn b/ui/ozone/platform/dri/BUILD.gn index 8cf51ce95b0e3..1e255500a0c12 100644 --- a/ui/ozone/platform/dri/BUILD.gn +++ b/ui/ozone/platform/dri/BUILD.gn @@ -14,6 +14,7 @@ source_set("dri_common") { "channel_observer.h", "crtc_controller.cc", "crtc_controller.h", + "display_change_observer.h", "display_manager.cc", "display_manager.h", "display_mode_dri.cc", diff --git a/ui/ozone/platform/dri/display_change_observer.h b/ui/ozone/platform/dri/display_change_observer.h new file mode 100644 index 0000000000000..a174cf639fedb --- /dev/null +++ b/ui/ozone/platform/dri/display_change_observer.h @@ -0,0 +1,26 @@ +// Copyright 2015 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 UI_OZONE_PLATFORM_DRI_DISPLAY_CHANGE_OBSERVER_H_ +#define UI_OZONE_PLATFORM_DRI_DISPLAY_CHANGE_OBSERVER_H_ + +namespace ui { + +class HardwareDisplayController; + +class DisplayChangeObserver { + public: + virtual ~DisplayChangeObserver() {} + + // Called when |controller| was changed/added. + virtual void OnDisplayChanged(HardwareDisplayController* controller) = 0; + + // Called just before |controller| is removed. Observers that cached + // |controller| must invalidate it at this point. + virtual void OnDisplayRemoved(HardwareDisplayController* controller) = 0; +}; + +} // namespace ui + +#endif // UI_OZONE_PLATFORM_DRI_DISPLAY_CHANGE_OBSERVER_H_ diff --git a/ui/ozone/platform/dri/dri.gypi b/ui/ozone/platform/dri/dri.gypi index 89cf21ebed492..963959f96fa1d 100644 --- a/ui/ozone/platform/dri/dri.gypi +++ b/ui/ozone/platform/dri/dri.gypi @@ -38,6 +38,7 @@ 'channel_observer.h', 'crtc_controller.cc', 'crtc_controller.h', + 'display_change_observer.h', 'display_manager.cc', 'display_manager.h', 'display_mode_dri.cc', diff --git a/ui/ozone/platform/dri/dri_window_delegate_impl.cc b/ui/ozone/platform/dri/dri_window_delegate_impl.cc index fd39e9154f98f..467578396379c 100644 --- a/ui/ozone/platform/dri/dri_window_delegate_impl.cc +++ b/ui/ozone/platform/dri/dri_window_delegate_impl.cc @@ -61,6 +61,8 @@ DriWindowDelegateImpl::~DriWindowDelegateImpl() { void DriWindowDelegateImpl::Initialize() { TRACE_EVENT1("dri", "DriWindowDelegateImpl::Initialize", "widget", widget_); + screen_manager_->AddObserver(this); + uint64_t cursor_width = 64; uint64_t cursor_height = 64; drm_->GetCapability(DRM_CAP_CURSOR_WIDTH, &cursor_width); @@ -78,6 +80,7 @@ void DriWindowDelegateImpl::Initialize() { void DriWindowDelegateImpl::Shutdown() { TRACE_EVENT1("dri", "DriWindowDelegateImpl::Shutdown", "widget", widget_); + screen_manager_->RemoveObserver(this); } gfx::AcceleratedWidget DriWindowDelegateImpl::GetAcceleratedWidget() { @@ -85,12 +88,13 @@ gfx::AcceleratedWidget DriWindowDelegateImpl::GetAcceleratedWidget() { } HardwareDisplayController* DriWindowDelegateImpl::GetController() { - return controller_.get(); + return controller_; } void DriWindowDelegateImpl::OnBoundsChanged(const gfx::Rect& bounds) { TRACE_EVENT2("dri", "DriWindowDelegateImpl::OnBoundsChanged", "widget", widget_, "bounds", bounds.ToString()); + bounds_ = bounds; controller_ = screen_manager_->GetDisplayController(bounds); } @@ -128,6 +132,30 @@ void DriWindowDelegateImpl::MoveCursor(const gfx::Point& location) { controller_->MoveCursor(location); } +void DriWindowDelegateImpl::OnDisplayChanged( + HardwareDisplayController* controller) { + DCHECK(controller); + + gfx::Rect controller_bounds = + gfx::Rect(controller->origin(), controller->GetModeSize()); + if (controller_) { + if (controller_ != controller) + return; + + if (controller->IsDisabled() || bounds_ != controller_bounds) + controller_ = nullptr; + } else { + if (bounds_ == controller_bounds && !controller->IsDisabled()) + controller_ = controller; + } +} + +void DriWindowDelegateImpl::OnDisplayRemoved( + HardwareDisplayController* controller) { + if (controller_ == controller) + controller_ = nullptr; +} + void DriWindowDelegateImpl::ResetCursor(bool bitmap_only) { if (cursor_bitmaps_.size()) { // Draw new cursor into backbuffer. diff --git a/ui/ozone/platform/dri/dri_window_delegate_impl.h b/ui/ozone/platform/dri/dri_window_delegate_impl.h index ca3b08fe640c5..445cc4c3fb5e2 100644 --- a/ui/ozone/platform/dri/dri_window_delegate_impl.h +++ b/ui/ozone/platform/dri/dri_window_delegate_impl.h @@ -5,16 +5,13 @@ #ifndef UI_OZONE_PLATFORM_DRI_DRI_WINDOW_DELEGATE_IMPL_H_ #define UI_OZONE_PLATFORM_DRI_DRI_WINDOW_DELEGATE_IMPL_H_ -#include "base/memory/weak_ptr.h" #include "base/timer/timer.h" #include "ui/gfx/geometry/point.h" +#include "ui/gfx/geometry/rect.h" #include "ui/gfx/native_widget_types.h" +#include "ui/ozone/platform/dri/display_change_observer.h" #include "ui/ozone/platform/dri/dri_window_delegate.h" -namespace gfx { -class Rect; -} // namespace gfx - namespace ui { class DriBuffer; @@ -23,7 +20,8 @@ class DriWrapper; class HardwareDisplayController; class ScreenManager; -class DriWindowDelegateImpl : public DriWindowDelegate { +class DriWindowDelegateImpl : public DriWindowDelegate, + public DisplayChangeObserver { public: DriWindowDelegateImpl(gfx::AcceleratedWidget widget, DriWrapper* drm, @@ -44,6 +42,10 @@ class DriWindowDelegateImpl : public DriWindowDelegate { const gfx::Point& location) override; void MoveCursor(const gfx::Point& location) override; + // DisplayChangeObserver: + void OnDisplayChanged(HardwareDisplayController* controller) override; + void OnDisplayRemoved(HardwareDisplayController* controller) override; + private: // Draw the last set cursor & update the cursor plane. void ResetCursor(bool bitmap_only); @@ -57,7 +59,12 @@ class DriWindowDelegateImpl : public DriWindowDelegate { DriWindowDelegateManager* window_manager_; // Not owned. ScreenManager* screen_manager_; // Not owned. - base::WeakPtr controller_; + // The current bounds of the window. + gfx::Rect bounds_; + + // The controller associated with the current window. This may be nullptr if + // the window isn't over an active display. + HardwareDisplayController* controller_; base::RepeatingTimer cursor_timer_; diff --git a/ui/ozone/platform/dri/screen_manager.cc b/ui/ozone/platform/dri/screen_manager.cc index 6305830688da5..e273bbdd24abc 100644 --- a/ui/ozone/platform/dri/screen_manager.cc +++ b/ui/ozone/platform/dri/screen_manager.cc @@ -89,8 +89,11 @@ void ScreenManager::RemoveDisplayController(uint32_t crtc) { if (it != controllers_.end()) { bool is_mirrored = (*it)->IsMirrored(); (*it)->RemoveCrtc(crtc); - if (!is_mirrored) + if (!is_mirrored) { + FOR_EACH_OBSERVER(DisplayChangeObserver, observers_, + OnDisplayRemoved(*it)); controllers_.erase(it); + } } } @@ -105,7 +108,6 @@ bool ScreenManager::ConfigureDisplayController(uint32_t crtc, << ") doesn't exist."; HardwareDisplayController* controller = *it; - controller = *it; // If nothing changed just enable the controller. Note, we perform an exact // comparison on the mode since the refresh rate may have changed. if (SameMode(mode, controller->get_mode()) && @@ -119,6 +121,8 @@ bool ScreenManager::ConfigureDisplayController(uint32_t crtc, return HandleMirrorMode(it, mirror, crtc, connector); } + FOR_EACH_OBSERVER(DisplayChangeObserver, observers_, + OnDisplayChanged(controller)); // Just re-enable the controller to re-use the current state. return controller->Enable(); } @@ -159,7 +163,7 @@ bool ScreenManager::DisableDisplayController(uint32_t crtc) { return false; } -base::WeakPtr ScreenManager::GetDisplayController( +HardwareDisplayController* ScreenManager::GetDisplayController( const gfx::Rect& bounds) { // TODO(dnicoara): Remove hack once TestScreen uses a simple Ozone display // configuration reader and ScreenManager is called from there to create the @@ -170,9 +174,17 @@ base::WeakPtr ScreenManager::GetDisplayController( HardwareDisplayControllers::iterator it = FindActiveDisplayControllerByLocation(bounds); if (it != controllers_.end()) - return (*it)->AsWeakPtr(); + return *it; + + return nullptr; +} + +void ScreenManager::AddObserver(DisplayChangeObserver* observer) { + observers_.AddObserver(observer); +} - return base::WeakPtr(); +void ScreenManager::RemoveObserver(DisplayChangeObserver* observer) { + observers_.RemoveObserver(observer); } ScreenManager::HardwareDisplayControllers::iterator @@ -244,6 +256,8 @@ bool ScreenManager::ModesetDisplayController( return false; } + FOR_EACH_OBSERVER(DisplayChangeObserver, observers_, + OnDisplayChanged(controller)); return true; } @@ -254,6 +268,10 @@ bool ScreenManager::HandleMirrorMode( uint32_t connector) { (*mirror)->AddCrtc((*original)->RemoveCrtc(crtc)); if ((*mirror)->Enable()) { + FOR_EACH_OBSERVER(DisplayChangeObserver, observers_, + OnDisplayRemoved(*original)); + FOR_EACH_OBSERVER(DisplayChangeObserver, observers_, + OnDisplayChanged(*mirror)); controllers_.erase(original); return true; } diff --git a/ui/ozone/platform/dri/screen_manager.h b/ui/ozone/platform/dri/screen_manager.h index 6b955165713e4..5eaae1aafa6a5 100644 --- a/ui/ozone/platform/dri/screen_manager.h +++ b/ui/ozone/platform/dri/screen_manager.h @@ -7,7 +7,8 @@ #include "base/macros.h" #include "base/memory/scoped_vector.h" -#include "base/memory/weak_ptr.h" +#include "base/observer_list.h" +#include "ui/ozone/platform/dri/display_change_observer.h" #include "ui/ozone/platform/dri/hardware_display_controller.h" typedef struct _drmModeModeInfo drmModeModeInfo; @@ -49,13 +50,12 @@ class ScreenManager { bool DisableDisplayController(uint32_t crtc); // Returns a reference to the display controller configured to display within - // |bounds|. - // This returns a weak reference since the display controller may be destroyed - // at any point in time, but the changes are propagated to the compositor much - // later (Compositor owns SurfaceOzone*, which is responsible for updating the - // display surface). - base::WeakPtr GetDisplayController( - const gfx::Rect& bounds); + // |bounds|. If the caller caches the controller it must also register as an + // observer to be notified when the controller goes out of scope. + HardwareDisplayController* GetDisplayController(const gfx::Rect& bounds); + + void AddObserver(DisplayChangeObserver* observer); + void RemoveObserver(DisplayChangeObserver* observer); // On non CrOS builds there is no display configurator to look-up available // displays and initialize the HDCs. In such cases this is called internally @@ -92,6 +92,8 @@ class ScreenManager { // List of display controllers (active and disabled). HardwareDisplayControllers controllers_; + ObserverList observers_; + DISALLOW_COPY_AND_ASSIGN(ScreenManager); }; diff --git a/ui/ozone/platform/dri/screen_manager_unittest.cc b/ui/ozone/platform/dri/screen_manager_unittest.cc index e27a64b77aa6a..be51557dc2911 100644 --- a/ui/ozone/platform/dri/screen_manager_unittest.cc +++ b/ui/ozone/platform/dri/screen_manager_unittest.cc @@ -33,6 +33,32 @@ class MockScreenManager : public ui::ScreenManager { DISALLOW_COPY_AND_ASSIGN(MockScreenManager); }; +class TestDisplayChangeObserver : public ui::DisplayChangeObserver { + public: + TestDisplayChangeObserver() + : num_displays_changed_(0), num_displays_removed_(0) {} + + ~TestDisplayChangeObserver() override {} + + int num_displays_changed() const { return num_displays_changed_; } + int num_displays_removed() const { return num_displays_removed_; } + + // TestDisplayChangeObserver: + void OnDisplayChanged(ui::HardwareDisplayController* controller) override { + num_displays_changed_++; + } + + void OnDisplayRemoved(ui::HardwareDisplayController* controller) override { + num_displays_removed_++; + } + + private: + int num_displays_changed_; + int num_displays_removed_; + + DISALLOW_COPY_AND_ASSIGN(TestDisplayChangeObserver); +}; + } // namespace class ScreenManagerTest : public testing::Test { @@ -55,8 +81,10 @@ class ScreenManagerTest : public testing::Test { buffer_generator_.reset(new ui::DriBufferGenerator(dri_.get())); screen_manager_.reset(new MockScreenManager( dri_.get(), buffer_generator_.get())); + screen_manager_->AddObserver(&observer_); } void TearDown() override { + screen_manager_->RemoveObserver(&observer_); screen_manager_.reset(); dri_.reset(); } @@ -66,6 +94,8 @@ class ScreenManagerTest : public testing::Test { scoped_ptr buffer_generator_; scoped_ptr screen_manager_; + TestDisplayChangeObserver observer_; + private: DISALLOW_COPY_AND_ASSIGN(ScreenManagerTest); }; @@ -81,7 +111,7 @@ TEST_F(ScreenManagerTest, CheckWithValidController) { kPrimaryConnector, GetPrimaryBounds().origin(), kDefaultMode); - base::WeakPtr controller = + ui::HardwareDisplayController* controller = screen_manager_->GetDisplayController(GetPrimaryBounds()); EXPECT_TRUE(controller); @@ -124,12 +154,14 @@ TEST_F(ScreenManagerTest, CheckControllerAfterItIsRemoved) { kPrimaryConnector, GetPrimaryBounds().origin(), kDefaultMode); - base::WeakPtr controller = - screen_manager_->GetDisplayController(GetPrimaryBounds()); + EXPECT_EQ(1, observer_.num_displays_changed()); + EXPECT_EQ(0, observer_.num_displays_removed()); + EXPECT_TRUE(screen_manager_->GetDisplayController(GetPrimaryBounds())); - EXPECT_TRUE(controller); screen_manager_->RemoveDisplayController(kPrimaryCrtc); - EXPECT_FALSE(controller); + EXPECT_EQ(1, observer_.num_displays_changed()); + EXPECT_EQ(1, observer_.num_displays_removed()); + EXPECT_FALSE(screen_manager_->GetDisplayController(GetPrimaryBounds())); } TEST_F(ScreenManagerTest, CheckDuplicateConfiguration) { @@ -149,6 +181,9 @@ TEST_F(ScreenManagerTest, CheckDuplicateConfiguration) { // Should reuse existing framebuffer. EXPECT_EQ(framebuffer, dri_->current_framebuffer()); + EXPECT_EQ(2, observer_.num_displays_changed()); + EXPECT_EQ(0, observer_.num_displays_removed()); + EXPECT_TRUE(screen_manager_->GetDisplayController(GetPrimaryBounds())); EXPECT_FALSE(screen_manager_->GetDisplayController(GetSecondaryBounds())); } @@ -165,6 +200,9 @@ TEST_F(ScreenManagerTest, CheckChangingMode) { screen_manager_->ConfigureDisplayController( kPrimaryCrtc, kPrimaryConnector, GetPrimaryBounds().origin(), new_mode); + EXPECT_EQ(2, observer_.num_displays_changed()); + EXPECT_EQ(0, observer_.num_displays_removed()); + gfx::Rect new_bounds(0, 0, new_mode.hdisplay, new_mode.vdisplay); EXPECT_TRUE(screen_manager_->GetDisplayController(new_bounds)); EXPECT_FALSE(screen_manager_->GetDisplayController(GetSecondaryBounds())); @@ -188,6 +226,8 @@ TEST_F(ScreenManagerTest, CheckForControllersInMirroredMode) { GetPrimaryBounds().origin(), kDefaultMode); + EXPECT_EQ(2, observer_.num_displays_changed()); + EXPECT_EQ(1, observer_.num_displays_removed()); EXPECT_TRUE(screen_manager_->GetDisplayController(GetPrimaryBounds())); EXPECT_FALSE(screen_manager_->GetDisplayController(GetSecondaryBounds())); } @@ -246,12 +286,18 @@ TEST_F(ScreenManagerTest, MonitorGoneInMirrorMode) { GetPrimaryBounds().origin(), kDefaultMode); + EXPECT_EQ(2, observer_.num_displays_changed()); + EXPECT_EQ(1, observer_.num_displays_removed()); + screen_manager_->RemoveDisplayController(kSecondaryCrtc); EXPECT_TRUE( screen_manager_->ConfigureDisplayController(kPrimaryCrtc, kPrimaryConnector, GetPrimaryBounds().origin(), kDefaultMode)); + EXPECT_EQ(3, observer_.num_displays_changed()); + EXPECT_EQ(1, observer_.num_displays_removed()); + EXPECT_TRUE(screen_manager_->GetDisplayController(GetPrimaryBounds())); EXPECT_FALSE(screen_manager_->GetDisplayController(GetSecondaryBounds())); } @@ -314,7 +360,7 @@ TEST_F(ScreenManagerTest, CheckMirrorModeAfterBeginReEnabled) { kSecondaryCrtc, kSecondaryConnector, GetPrimaryBounds().origin(), kDefaultMode); - base::WeakPtr controller = + ui::HardwareDisplayController* controller = screen_manager_->GetDisplayController(GetPrimaryBounds()); EXPECT_TRUE(controller); EXPECT_FALSE(controller->IsMirrored());