Skip to content

Commit

Permalink
Use registered info to initialize primary display instead of
Browse files Browse the repository at this point in the history
 swapping primary display in DisplayController::InitSecondaryDisplays.

cleanups:
* move GetCurrentDisplayIdPair/GetCurrentDisplayLayout to display manager as the do not
 depends on primary display/root windows
* Remove unnecessary UpdateDisplayBoundsForLayout call in DisplayController::InitSecondaryDisplays.

BUG=270117
TEST=no functional change. all tests should pass.

Review URL: https://chromiumcodereview.appspot.com/23058006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@217523 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
oshima@chromium.org committed Aug 14, 2013
1 parent fe37ec4 commit 4bfb372
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 130 deletions.
61 changes: 10 additions & 51 deletions ash/display/display_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -288,29 +288,21 @@ int DisplayController::GetNumDisplays() {
}

void DisplayController::InitPrimaryDisplay() {
const gfx::Display* primary_candidate =
const gfx::Display& primary_candidate =
GetDisplayManager()->GetPrimaryDisplayCandidate();
primary_display_id = primary_candidate->id();
AddRootWindowForDisplay(*primary_candidate);
primary_display_id = primary_candidate.id();
AddRootWindowForDisplay(primary_candidate);
}

void DisplayController::InitSecondaryDisplays() {
internal::DisplayManager* display_manager = GetDisplayManager();
UpdateDisplayBoundsForLayout();
for (size_t i = 0; i < display_manager->GetNumDisplays(); ++i) {
const gfx::Display& display = display_manager->GetDisplayAt(i);
if (primary_display_id != display.id()) {
aura::RootWindow* root = AddRootWindowForDisplay(display);
Shell::GetInstance()->InitRootWindowForSecondaryDisplay(root);
}
}
if (display_manager->GetNumDisplays() > 1) {
DisplayIdPair pair = GetCurrentDisplayIdPair();
DisplayLayout layout = GetCurrentDisplayLayout();
SetPrimaryDisplayId(
layout.primary_id == gfx::Display::kInvalidDisplayID ?
pair.first : layout.primary_id);
}
UpdateHostWindowNames();
}

Expand Down Expand Up @@ -387,7 +379,7 @@ void DisplayController::SetLayoutForCurrentDisplays(
if (GetDisplayManager()->GetNumDisplays() < 2)
return;
const gfx::Display& primary = GetPrimaryDisplay();
const DisplayIdPair pair = GetCurrentDisplayIdPair();
const DisplayIdPair pair = GetDisplayManager()->GetCurrentDisplayIdPair();
// Invert if the primary was swapped.
DisplayLayout to_set = pair.first == primary.id() ?
layout_relative_to_primary : layout_relative_to_primary.Invert();
Expand All @@ -412,40 +404,6 @@ void DisplayController::SetLayoutForCurrentDisplays(
}
}

DisplayLayout DisplayController::GetCurrentDisplayLayout() {
DCHECK_EQ(2U, GetDisplayManager()->num_connected_displays());
// Invert if the primary was swapped.
if (GetDisplayManager()->num_connected_displays() > 1) {
DisplayIdPair pair = GetCurrentDisplayIdPair();
return GetDisplayManager()->layout_store()->
ComputeDisplayLayoutForDisplayIdPair(pair);
}
NOTREACHED() << "DisplayLayout is requested for single display";
// On release build, just fallback to default instead of blowing up.
DisplayLayout layout =
GetDisplayManager()->layout_store()->default_display_layout();
layout.primary_id = primary_display_id;
return layout;
}

DisplayIdPair DisplayController::GetCurrentDisplayIdPair() const {
internal::DisplayManager* display_manager = GetDisplayManager();
const gfx::Display& primary = GetPrimaryDisplay();
if (display_manager->IsMirrored()) {
return std::make_pair(primary.id(),
display_manager->mirrored_display().id());
}

const gfx::Display& secondary = ScreenAsh::GetSecondaryDisplay();
if (primary.IsInternal() ||
GetDisplayManager()->first_display_id() == primary.id()) {
return std::make_pair(primary.id(), secondary.id());
} else {
// Display has been Swapped.
return std::make_pair(secondary.id(), primary.id());
}
}

void DisplayController::ToggleMirrorMode() {
internal::DisplayManager* display_manager = GetDisplayManager();
if (display_manager->num_connected_displays() <= 1)
Expand Down Expand Up @@ -542,7 +500,7 @@ void DisplayController::SetPrimaryDisplay(

primary_display_id = new_primary_display.id();
GetDisplayManager()->layout_store()->UpdatePrimaryDisplayId(
GetCurrentDisplayIdPair(), primary_display_id);
display_manager->GetCurrentDisplayIdPair(), primary_display_id);

UpdateWorkAreaOfDisplayNearestWindow(
primary_root, old_primary_display.GetWorkAreaInsets());
Expand Down Expand Up @@ -783,7 +741,7 @@ void DisplayController::PostDisplayConfigurationChange() {
internal::DisplayManager* display_manager = GetDisplayManager();
internal::DisplayLayoutStore* layout_store = display_manager->layout_store();
if (display_manager->num_connected_displays() > 1) {
DisplayIdPair pair = GetCurrentDisplayIdPair();
DisplayIdPair pair = display_manager->GetCurrentDisplayIdPair();
layout_store->UpdateMirrorStatus(pair, display_manager->IsMirrored());
DisplayLayout layout = layout_store->GetRegisteredDisplayLayout(pair);

Expand Down Expand Up @@ -837,14 +795,15 @@ aura::RootWindow* DisplayController::AddRootWindowForDisplay(
}

void DisplayController::UpdateDisplayBoundsForLayout() {
internal::DisplayManager* display_manager = GetDisplayManager();
if (Shell::GetScreen()->GetNumDisplays() < 2 ||
GetDisplayManager()->num_connected_displays() < 2) {
display_manager->num_connected_displays() < 2) {
return;
}
DCHECK_EQ(2, Shell::GetScreen()->GetNumDisplays());

const DisplayLayout layout = GetCurrentDisplayLayout();
Shell::GetInstance()->display_manager()->UpdateDisplayBoundsForLayoutById(
const DisplayLayout layout = display_manager->GetCurrentDisplayLayout();
display_manager->UpdateDisplayBoundsForLayoutById(
layout, GetPrimaryDisplay(),
ScreenAsh::GetSecondaryDisplay().id());
}
Expand Down
6 changes: 0 additions & 6 deletions ash/display/display_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,6 @@ class ASH_EXPORT DisplayController : public gfx::DisplayObserver,
// the locaion of the secondary display relative to the primary.
void SetLayoutForCurrentDisplays(const DisplayLayout& layout);

// Returns the display layout used for current displays.
DisplayLayout GetCurrentDisplayLayout();

// Returns the current display pair.
DisplayIdPair GetCurrentDisplayIdPair() const;

// Checks if the mouse pointer is on one of displays, and moves to
// the center of the nearest display if it's outside of all displays.
void EnsurePointerInDisplays();
Expand Down
37 changes: 7 additions & 30 deletions ash/display/display_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -416,31 +416,6 @@ TEST_F(DisplayControllerTest, BoundsUpdated) {
EXPECT_EQ(0, observer.CountAndReset());
}

TEST_F(DisplayControllerTest, MirroredLayout) {
if (!SupportsMultipleDisplays())
return;

DisplayController* display_controller =
Shell::GetInstance()->display_controller();
UpdateDisplay("500x500,400x400");
EXPECT_FALSE(display_controller->GetCurrentDisplayLayout().mirrored);
EXPECT_EQ(2, Shell::GetScreen()->GetNumDisplays());
EXPECT_EQ(
2U, Shell::GetInstance()->display_manager()->num_connected_displays());

UpdateDisplay("1+0-500x500,1+0-500x500");
EXPECT_TRUE(display_controller->GetCurrentDisplayLayout().mirrored);
EXPECT_EQ(1, Shell::GetScreen()->GetNumDisplays());
EXPECT_EQ(
2U, Shell::GetInstance()->display_manager()->num_connected_displays());

UpdateDisplay("500x500,500x500");
EXPECT_FALSE(display_controller->GetCurrentDisplayLayout().mirrored);
EXPECT_EQ(2, Shell::GetScreen()->GetNumDisplays());
EXPECT_EQ(
2U, Shell::GetInstance()->display_manager()->num_connected_displays());
}

TEST_F(DisplayControllerTest, InvertLayout) {
EXPECT_EQ("left, 0",
DisplayLayout(DisplayLayout::RIGHT, 0).Invert().ToString());
Expand Down Expand Up @@ -477,6 +452,8 @@ TEST_F(DisplayControllerTest, SwapPrimary) {

DisplayController* display_controller =
Shell::GetInstance()->display_controller();
internal::DisplayManager* display_manager =
Shell::GetInstance()->display_manager();

UpdateDisplay("200x200,300x300");
gfx::Display primary_display = Shell::GetScreen()->GetPrimaryDisplay();
Expand Down Expand Up @@ -506,12 +483,12 @@ TEST_F(DisplayControllerTest, SwapPrimary) {
EXPECT_EQ("200,0 300x300", secondary_display.bounds().ToString());
EXPECT_EQ("200,0 300x252", secondary_display.work_area().ToString());
EXPECT_EQ("right, 50",
display_controller->GetCurrentDisplayLayout().ToString());
display_manager->GetCurrentDisplayLayout().ToString());

// Switch primary and secondary
display_controller->SetPrimaryDisplay(secondary_display);
const DisplayLayout& inverted_layout =
display_controller->GetCurrentDisplayLayout();
display_manager->GetCurrentDisplayLayout();
EXPECT_EQ("left, -50", inverted_layout.ToString());

EXPECT_EQ(secondary_display.id(),
Expand Down Expand Up @@ -566,6 +543,8 @@ TEST_F(DisplayControllerTest, SwapPrimaryById) {

DisplayController* display_controller =
Shell::GetInstance()->display_controller();
internal::DisplayManager* display_manager =
Shell::GetInstance()->display_manager();

UpdateDisplay("200x200,300x300");
gfx::Display primary_display = Shell::GetScreen()->GetPrimaryDisplay();
Expand Down Expand Up @@ -608,7 +587,7 @@ TEST_F(DisplayControllerTest, SwapPrimaryById) {
EXPECT_FALSE(secondary_root->Contains(launcher_window));

const DisplayLayout& inverted_layout =
display_controller->GetCurrentDisplayLayout();
display_manager->GetCurrentDisplayLayout();

EXPECT_EQ("left, -50", inverted_layout.ToString());

Expand All @@ -634,8 +613,6 @@ TEST_F(DisplayControllerTest, SwapPrimaryById) {
EXPECT_FALSE(tracker.Contains(secondary_root));
EXPECT_TRUE(primary_root->Contains(launcher_window));

internal::DisplayManager* display_manager =
Shell::GetInstance()->display_manager();
// Adding 2nd display with the same ID. The 2nd display should become primary
// since secondary id is still stored as desirable_primary_id.
std::vector<internal::DisplayInfo> display_info_list;
Expand Down
59 changes: 38 additions & 21 deletions ash/display/display_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,38 @@ bool DisplayManager::IsInternalDisplayId(int64 id) const {
return gfx::Display::InternalDisplayId() == id;
}

DisplayLayout DisplayManager::GetCurrentDisplayLayout() {
DCHECK_EQ(2U, num_connected_displays());
// Invert if the primary was swapped.
if (num_connected_displays() > 1) {
DisplayIdPair pair = GetCurrentDisplayIdPair();
return layout_store_->ComputeDisplayLayoutForDisplayIdPair(pair);
}
NOTREACHED() << "DisplayLayout is requested for single display";
// On release build, just fallback to default instead of blowing up.
DisplayLayout layout =
layout_store_->default_display_layout();
layout.primary_id = displays_[0].id();
return layout;
}

DisplayIdPair DisplayManager::GetCurrentDisplayIdPair() const {
if (IsMirrored()) {
int64 mirrored_id = mirrored_display().id();
return displays_[0].id() == mirrored_id ?
std::make_pair(displays_[1].id(), mirrored_id) :
std::make_pair(displays_[0].id(), mirrored_id);
} else {
int64 id_at_zero = displays_[0].id();
if (id_at_zero == gfx::Display::InternalDisplayId() ||
id_at_zero == first_display_id()) {
return std::make_pair(id_at_zero, displays_[1].id());
} else {
return std::make_pair(displays_[1].id(), id_at_zero);
}
}
}

const gfx::Display& DisplayManager::GetDisplayForId(int64 id) const {
gfx::Display* display =
const_cast<DisplayManager*>(this)->FindDisplayForId(id);
Expand Down Expand Up @@ -672,28 +704,13 @@ const gfx::Display& DisplayManager::GetDisplayAt(size_t index) const {
return displays_[index];
}

const gfx::Display* DisplayManager::GetPrimaryDisplayCandidate() const {
const gfx::Display* primary_candidate = &displays_[0];
#if defined(OS_CHROMEOS)
if (base::chromeos::IsRunningOnChromeOS()) {
// On ChromeOS device, root windows are stacked vertically, and
// default primary is the one on top.
int count = GetNumDisplays();
int y = GetDisplayInfo(primary_candidate->id()).bounds_in_pixel().y();
for (int i = 1; i < count; ++i) {
const gfx::Display* display = &displays_[i];
const DisplayInfo& display_info = GetDisplayInfo(display->id());
if (display->IsInternal()) {
primary_candidate = display;
break;
} else if (display_info.bounds_in_pixel().y() < y) {
primary_candidate = display;
y = display_info.bounds_in_pixel().y();
}
}
const gfx::Display& DisplayManager::GetPrimaryDisplayCandidate() const {
if (GetNumDisplays() == 1) {
return displays_[0];
}
#endif
return primary_candidate;
DisplayLayout layout = layout_store_->GetRegisteredDisplayLayout(
GetCurrentDisplayIdPair());
return GetDisplayForId(layout.primary_id);
}

size_t DisplayManager::GetNumDisplays() const {
Expand Down
8 changes: 7 additions & 1 deletion ash/display/display_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ class ASH_EXPORT DisplayManager

bool IsInternalDisplayId(int64 id) const;

// Returns the display layout used for current displays.
DisplayLayout GetCurrentDisplayLayout();

// Returns the current display pair.
DisplayIdPair GetCurrentDisplayIdPair() const;

// Returns display for given |id|;
const gfx::Display& GetDisplayForId(int64 id) const;

Expand Down Expand Up @@ -170,7 +176,7 @@ class ASH_EXPORT DisplayManager
// no longer considered "primary".
const gfx::Display& GetDisplayAt(size_t index) const;

const gfx::Display* GetPrimaryDisplayCandidate() const;
const gfx::Display& GetPrimaryDisplayCandidate() const;

// Returns the logical number of displays. This returns 1
// when displays are mirrored.
Expand Down
21 changes: 21 additions & 0 deletions ash/display/display_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1027,5 +1027,26 @@ TEST_F(DisplayManagerTest, SoftwareMirroring) {
Shell::GetScreen()->RemoveObserver(&display_observer);
}

TEST_F(DisplayManagerTest, MirroredLayout) {
if (!SupportsMultipleDisplays())
return;

DisplayManager* display_manager = Shell::GetInstance()->display_manager();
UpdateDisplay("500x500,400x400");
EXPECT_FALSE(display_manager->GetCurrentDisplayLayout().mirrored);
EXPECT_EQ(2, Shell::GetScreen()->GetNumDisplays());
EXPECT_EQ(2U, display_manager->num_connected_displays());

UpdateDisplay("1+0-500x500,1+0-500x500");
EXPECT_TRUE(display_manager->GetCurrentDisplayLayout().mirrored);
EXPECT_EQ(1, Shell::GetScreen()->GetNumDisplays());
EXPECT_EQ(2U, display_manager->num_connected_displays());

UpdateDisplay("500x500,500x500");
EXPECT_FALSE(display_manager->GetCurrentDisplayLayout().mirrored);
EXPECT_EQ(2, Shell::GetScreen()->GetNumDisplays());
EXPECT_EQ(2U, display_manager->num_connected_displays());
}

} // namespace internal
} // namespace ash
7 changes: 4 additions & 3 deletions ash/display/mouse_cursor_event_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "ash/display/mouse_cursor_event_filter.h"

#include "ash/display/display_controller.h"
#include "ash/display/display_manager.h"
#include "ash/display/mirror_window_controller.h"
#include "ash/display/shared_display_edge_indicator.h"
#include "ash/screen_ash.h"
Expand Down Expand Up @@ -57,7 +58,7 @@ void MouseCursorEventFilter::ShowSharedEdgeIndicator(
drag_source_root_ = from;

DisplayLayout::Position position = Shell::GetInstance()->
display_controller()->GetCurrentDisplayLayout().position;
display_manager()->GetCurrentDisplayLayout().position;
if (position == DisplayLayout::TOP || position == DisplayLayout::BOTTOM)
UpdateHorizontalIndicatorWindowBounds();
else
Expand Down Expand Up @@ -158,7 +159,7 @@ void MouseCursorEventFilter::UpdateHorizontalIndicatorWindowBounds() {
Shell::GetScreen()->GetPrimaryDisplay().bounds();
const gfx::Rect secondary_bounds = ScreenAsh::GetSecondaryDisplay().bounds();
DisplayLayout::Position position = Shell::GetInstance()->
display_controller()->GetCurrentDisplayLayout().position;
display_manager()->GetCurrentDisplayLayout().position;

src_indicator_bounds_.set_x(
std::max(primary_bounds.x(), secondary_bounds.x()));
Expand Down Expand Up @@ -187,7 +188,7 @@ void MouseCursorEventFilter::UpdateVerticalIndicatorWindowBounds() {
Shell::GetScreen()->GetPrimaryDisplay().bounds();
const gfx::Rect secondary_bounds = ScreenAsh::GetSecondaryDisplay().bounds();
DisplayLayout::Position position = Shell::GetInstance()->
display_controller()->GetCurrentDisplayLayout().position;
display_manager()->GetCurrentDisplayLayout().position;

int upper_shared_y = std::max(primary_bounds.y(), secondary_bounds.y());
int lower_shared_y = std::min(primary_bounds.bottom(),
Expand Down
4 changes: 2 additions & 2 deletions ash/display/mouse_cursor_event_filter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ TEST_F(MouseCursorEventFilterTest, WarpMouseDifferentSizeDisplays) {
Shell::GetInstance()->mouse_cursor_filter();
ASSERT_EQ(
DisplayLayout::RIGHT,
Shell::GetInstance()->display_controller()->
Shell::GetInstance()->display_manager()->
GetCurrentDisplayLayout().position);

Shell::RootWindowList root_windows = Shell::GetAllRootWindows();
Expand Down Expand Up @@ -143,7 +143,7 @@ TEST_F(MouseCursorEventFilterTest, WarpMouseDifferentScaleDisplays) {
Shell::GetInstance()->mouse_cursor_filter();
ASSERT_EQ(
DisplayLayout::RIGHT,
Shell::GetInstance()->display_controller()->
Shell::GetInstance()->display_manager()->
GetCurrentDisplayLayout().position);

Shell::RootWindowList root_windows = Shell::GetAllRootWindows();
Expand Down
Loading

0 comments on commit 4bfb372

Please sign in to comment.