Skip to content

Commit

Permalink
ash: Remove off-by-one pixel translation.
Browse files Browse the repository at this point in the history
This removes the off-by-one pixel translation currently used
when creating rotation matrix for root window.

This was causing the contents of fullscreen windows to be
located one pixel off screeen display is rotated and as a
result also prevent HW overlays from working.

Bug: 757884
Change-Id: I2854c9cb187685ec2790a10cc29426e59c373418
Reviewed-on: https://chromium-review.googlesource.com/626737
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496777}
  • Loading branch information
reveman-chromium authored and Commit Bot committed Aug 23, 2017
1 parent 63b5efd commit 291ba8b
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 46 deletions.
4 changes: 2 additions & 2 deletions ash/display/display_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2073,13 +2073,13 @@ TEST_F(DisplayManagerTest, UpdateMouseCursorAfterRotateZoom) {
generator1.MoveMouseToInHost(150, 50);
EXPECT_EQ("150,50", env->last_mouse_location().ToString());
UpdateDisplay("300x200/r,200x150");
EXPECT_EQ("50,149", env->last_mouse_location().ToString());
EXPECT_EQ("50,150", env->last_mouse_location().ToString());

// Test on 2nd display.
generator2.MoveMouseToInHost(50, 100);
EXPECT_EQ("250,100", env->last_mouse_location().ToString());
UpdateDisplay("300x200/r,200x150/l");
EXPECT_EQ("249,50", env->last_mouse_location().ToString());
EXPECT_EQ("250,50", env->last_mouse_location().ToString());

// The native location is now outside, so move to the center
// of closest display.
Expand Down
8 changes: 4 additions & 4 deletions ash/display/display_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ TEST_F(DisplayUtilTest, RotatedDisplay) {
gfx::Rect rect0 = GetNativeEdgeBounds(host0, gfx::Rect(499, 10, 1, 300));
gfx::Rect rect1 = GetNativeEdgeBounds(host1, gfx::Rect(500, 10, 1, 300));
EXPECT_EQ("509,20 1x300", rect0.ToString());
EXPECT_EQ("1289,10 300x1", rect1.ToString());
EXPECT_EQ("1290,10 300x1", rect1.ToString());
}
{
UpdateDisplay("10+10-500x400,600+10-1000x600/l");
Expand All @@ -47,7 +47,7 @@ TEST_F(DisplayUtilTest, RotatedDisplay) {
gfx::Rect rect0 = GetNativeEdgeBounds(host0, gfx::Rect(499, 10, 1, 300));
gfx::Rect rect1 = GetNativeEdgeBounds(host1, gfx::Rect(500, 10, 1, 300));
EXPECT_EQ("509,20 1x300", rect0.ToString());
EXPECT_EQ("1599,299 1x300", rect1.ToString());
EXPECT_EQ("1599,300 1x300", rect1.ToString());
}

{
Expand All @@ -59,7 +59,7 @@ TEST_F(DisplayUtilTest, RotatedDisplay) {
RootWindowController::ForWindow(root_windows[1])->ash_host();
gfx::Rect rect0 = GetNativeEdgeBounds(host0, gfx::Rect(399, 10, 1, 300));
gfx::Rect rect1 = GetNativeEdgeBounds(host1, gfx::Rect(400, 10, 1, 300));
EXPECT_EQ("199,409 300x1", rect0.ToString());
EXPECT_EQ("200,409 300x1", rect0.ToString());
EXPECT_EQ("600,20 1x300", rect1.ToString());
}
{
Expand All @@ -83,7 +83,7 @@ TEST_F(DisplayUtilTest, RotatedDisplay) {
RootWindowController::ForWindow(root_windows[1])->ash_host();
gfx::Rect rect0 = GetNativeEdgeBounds(host0, gfx::Rect(499, 10, 1, 300));
gfx::Rect rect1 = GetNativeEdgeBounds(host1, gfx::Rect(500, 10, 1, 300));
EXPECT_EQ("10,99 1x300", rect0.ToString());
EXPECT_EQ("10,100 1x300", rect0.ToString());
EXPECT_EQ("600,20 1x300", rect1.ToString());
}
}
Expand Down
2 changes: 1 addition & 1 deletion ash/display/mirror_window_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ TEST_F(MirrorWindowControllerTest, MirrorCursorMoveOnEnter) {
EXPECT_EQ("18,7", test_api.GetCursorHotPoint().ToString());
// New coordinates are not (200,200) because (200,200) is not the center of
// the display.
EXPECT_EQ("199,200",
EXPECT_EQ("200,200",
test_api.GetCursorHotPointLocationInRootWindow().ToString());
}

Expand Down
26 changes: 13 additions & 13 deletions ash/display/root_window_transformers_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -335,17 +335,17 @@ TEST_F(RootWindowTransformersTest, ConvertHostToRootCoords) {
ui::test::EventGenerator generator(root_windows[0]);
generator.MoveMouseToInHost(300, 200);
magnifier->SetEnabled(true);
EXPECT_EQ("150,224", event_handler.GetLocationAndReset());
EXPECT_EQ("150,225", event_handler.GetLocationAndReset());
EXPECT_FLOAT_EQ(2.0f, magnifier->GetScale());

generator.MoveMouseToInHost(300, 200);
EXPECT_EQ("150,224", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(200, 300);
EXPECT_EQ("187,261", event_handler.GetLocationAndReset());
EXPECT_EQ("187,262", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(100, 400);
EXPECT_EQ("237,299", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(0, 0);
EXPECT_EQ("137,348", event_handler.GetLocationAndReset());
EXPECT_EQ("137,349", event_handler.GetLocationAndReset());

magnifier->SetEnabled(false);
EXPECT_FLOAT_EQ(1.0f, magnifier->GetScale());
Expand All @@ -360,17 +360,17 @@ TEST_F(RootWindowTransformersTest, ConvertHostToRootCoords) {

generator.MoveMouseToInHost(300, 200);
magnifier->SetEnabled(true);
EXPECT_EQ("224,149", event_handler.GetLocationAndReset());
EXPECT_EQ("225,150", event_handler.GetLocationAndReset());
EXPECT_FLOAT_EQ(2.0f, magnifier->GetScale());

generator.MoveMouseToInHost(300, 200);
EXPECT_EQ("224,148", event_handler.GetLocationAndReset());
EXPECT_EQ("224,150", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(200, 300);
EXPECT_EQ("261,111", event_handler.GetLocationAndReset());
EXPECT_EQ("262,112", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(100, 400);
EXPECT_EQ("299,60", event_handler.GetLocationAndReset());
EXPECT_EQ("299,62", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(0, 0);
EXPECT_EQ("348,159", event_handler.GetLocationAndReset());
EXPECT_EQ("349,162", event_handler.GetLocationAndReset());

magnifier->SetEnabled(false);
EXPECT_FLOAT_EQ(1.0f, magnifier->GetScale());
Expand All @@ -385,17 +385,17 @@ TEST_F(RootWindowTransformersTest, ConvertHostToRootCoords) {

generator.MoveMouseToInHost(300, 200);
magnifier->SetEnabled(true);
EXPECT_EQ("149,225", event_handler.GetLocationAndReset());
EXPECT_EQ("150,225", event_handler.GetLocationAndReset());
EXPECT_FLOAT_EQ(2.0f, magnifier->GetScale());

generator.MoveMouseToInHost(300, 200);
EXPECT_EQ("148,224", event_handler.GetLocationAndReset());
EXPECT_EQ("150,224", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(200, 300);
EXPECT_EQ("111,187", event_handler.GetLocationAndReset());
EXPECT_EQ("112,187", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(100, 400);
EXPECT_EQ("60,149", event_handler.GetLocationAndReset());
EXPECT_EQ("61,149", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(0, 0);
EXPECT_EQ("159,99", event_handler.GetLocationAndReset());
EXPECT_EQ("161,99", event_handler.GetLocationAndReset());

magnifier->SetEnabled(false);
EXPECT_FLOAT_EQ(1.0f, magnifier->GetScale());
Expand Down
12 changes: 6 additions & 6 deletions ash/display/screen_position_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,12 @@ TEST_F(ScreenPositionControllerTest, ConvertHostPointToScreenRotate) {
display::Screen::GetScreen()->GetPrimaryDisplay());

// The point is on the 1st host.
EXPECT_EQ("70,149", ConvertHostPointToScreen(50, 70));
EXPECT_EQ("70,150", ConvertHostPointToScreen(50, 70));
// The point is out of the host windows.
EXPECT_EQ("250,-51", ConvertHostPointToScreen(250, 250));
EXPECT_EQ("250,-50", ConvertHostPointToScreen(250, 250));
// The point is on the 2nd host. Point on 2nd host (30,150) -
// rotate 270 clockwise -> (149, 30) - layout [+(200,0)] -> (349,30).
EXPECT_EQ("349,30", ConvertHostPointToScreen(30, 450));
EXPECT_EQ("350,30", ConvertHostPointToScreen(30, 450));

// Move |window_| to the 2nd.
window_->SetBoundsInScreen(gfx::Rect(300, 20, 50, 50),
Expand All @@ -223,12 +223,12 @@ TEST_F(ScreenPositionControllerTest, ConvertHostPointToScreenRotate) {

// The point is on the 2nd host. (50,70) on 2n host -
// roatate 270 clockwise -> (129,50) -layout [+(200,0)] -> (329,50)
EXPECT_EQ("329,50", ConvertHostPointToScreen(50, 70));
EXPECT_EQ("330,50", ConvertHostPointToScreen(50, 70));
// The point is out of the host windows.
EXPECT_EQ("449,50", ConvertHostPointToScreen(50, -50));
EXPECT_EQ("450,50", ConvertHostPointToScreen(50, -50));
// The point is on the 2nd host. Point on 2nd host (50,50) -
// rotate 90 clockwise -> (50, 149)
EXPECT_EQ("50,149", ConvertHostPointToScreen(50, -350));
EXPECT_EQ("50,150", ConvertHostPointToScreen(50, -350));
}

TEST_F(ScreenPositionControllerTest, ConvertHostPointToScreenUIScale) {
Expand Down
20 changes: 10 additions & 10 deletions ash/display/window_tree_host_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,7 @@ TEST_F(WindowTreeHostManagerTest, Rotate) {
EXPECT_EQ("200,0 150x200",
display_manager()->GetSecondaryDisplay().bounds().ToString());
generator1.MoveMouseToInHost(50, 40);
EXPECT_EQ("40,69", event_handler.GetLocationAndReset());
EXPECT_EQ("40,70", event_handler.GetLocationAndReset());
EXPECT_EQ(display::Display::ROTATE_90,
GetActiveDisplayRotation(display1.id()));
EXPECT_EQ(display::Display::ROTATE_0, GetActiveDisplayRotation(display2_id));
Expand All @@ -1265,7 +1265,7 @@ TEST_F(WindowTreeHostManagerTest, Rotate) {

ui::test::EventGenerator generator2(root_windows[1]);
generator2.MoveMouseToInHost(50, 40);
EXPECT_EQ("179,25", event_handler.GetLocationAndReset());
EXPECT_EQ("180,25", event_handler.GetLocationAndReset());
display_manager()->SetDisplayRotation(
display1.id(), display::Display::ROTATE_180,
display::Display::ROTATION_SOURCE_ACTIVE);
Expand All @@ -1282,7 +1282,7 @@ TEST_F(WindowTreeHostManagerTest, Rotate) {
EXPECT_EQ(1, observer.GetRotationChangedCountAndReset());

generator1.MoveMouseToInHost(50, 40);
EXPECT_EQ("69,159", event_handler.GetLocationAndReset());
EXPECT_EQ("70,160", event_handler.GetLocationAndReset());

Shell::Get()->RemovePreTargetHandler(&event_handler);
}
Expand Down Expand Up @@ -1365,13 +1365,13 @@ TEST_F(WindowTreeHostManagerTest, ConvertHostToRootCoords) {

ui::test::EventGenerator generator(root_windows[0]);
generator.MoveMouseToInHost(0, 0);
EXPECT_EQ("0,449", event_handler.GetLocationAndReset());
EXPECT_EQ("0,450", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(599, 0);
EXPECT_EQ("0,0", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(599, 399);
EXPECT_EQ("299,0", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(0, 399);
EXPECT_EQ("299,449", event_handler.GetLocationAndReset());
EXPECT_EQ("299,450", event_handler.GetLocationAndReset());

UpdateDisplay("600x400*2/u@1.5");
display1 = display::Screen::GetScreen()->GetPrimaryDisplay();
Expand All @@ -1381,13 +1381,13 @@ TEST_F(WindowTreeHostManagerTest, ConvertHostToRootCoords) {
EXPECT_EQ(1.5f, GetStoredUIScale(display1.id()));

generator.MoveMouseToInHost(0, 0);
EXPECT_EQ("449,299", event_handler.GetLocationAndReset());
EXPECT_EQ("450,300", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(599, 0);
EXPECT_EQ("0,299", event_handler.GetLocationAndReset());
EXPECT_EQ("0,300", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(599, 399);
EXPECT_EQ("0,0", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(0, 399);
EXPECT_EQ("449,0", event_handler.GetLocationAndReset());
EXPECT_EQ("450,0", event_handler.GetLocationAndReset());

UpdateDisplay("600x400*2/l@1.5");
display1 = display::Screen::GetScreen()->GetPrimaryDisplay();
Expand All @@ -1397,9 +1397,9 @@ TEST_F(WindowTreeHostManagerTest, ConvertHostToRootCoords) {
EXPECT_EQ(1.5f, GetStoredUIScale(display1.id()));

generator.MoveMouseToInHost(0, 0);
EXPECT_EQ("299,0", event_handler.GetLocationAndReset());
EXPECT_EQ("300,0", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(599, 0);
EXPECT_EQ("299,449", event_handler.GetLocationAndReset());
EXPECT_EQ("300,449", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(599, 399);
EXPECT_EQ("0,449", event_handler.GetLocationAndReset());
generator.MoveMouseToInHost(0, 399);
Expand Down
6 changes: 3 additions & 3 deletions ash/highlighter/highlighter_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,21 +270,21 @@ TEST_F(HighlighterControllerTest, HighlighterGesturesRotated) {
controller_test_api_->ResetSelection();
TraceRect(trace);
EXPECT_TRUE(controller_test_api_->handle_selection_called());
EXPECT_EQ("100,899 300x400", controller_test_api_->selection().ToString());
EXPECT_EQ("100,900 300x400", controller_test_api_->selection().ToString());

// Rotate to 180 degrees
UpdateDisplay("1500x1000/u");
controller_test_api_->ResetSelection();
TraceRect(trace);
EXPECT_TRUE(controller_test_api_->handle_selection_called());
EXPECT_EQ("899,599 400x300", controller_test_api_->selection().ToString());
EXPECT_EQ("900,600 400x300", controller_test_api_->selection().ToString());

// Rotate to 270 degrees
UpdateDisplay("1500x1000/l");
controller_test_api_->ResetSelection();
TraceRect(trace);
EXPECT_TRUE(controller_test_api_->handle_selection_called());
EXPECT_EQ("599,200 300x400", controller_test_api_->selection().ToString());
EXPECT_EQ("600,200 300x400", controller_test_api_->selection().ToString());
}

} // namespace ash
10 changes: 3 additions & 7 deletions ash/utility/transformer_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,19 @@ gfx::Transform CreateRotationTransform(display::Display::Rotation old_rotation,
const display::Display& display) {
const int rotation_angle = 90 * (((new_rotation - old_rotation) + 4) % 4);
gfx::Transform rotate;
// The origin is (0, 0), so the translate width/height must be reduced by
// 1 pixel.
float one_pixel = 1.0f / display.device_scale_factor();
switch (rotation_angle) {
case 0:
break;
case 90:
rotate.Translate(display.bounds().height() - one_pixel, 0);
rotate.Translate(display.bounds().height(), 0);
rotate.Rotate(90);
break;
case 180:
rotate.Translate(display.bounds().width() - one_pixel,
display.bounds().height() - one_pixel);
rotate.Translate(display.bounds().width(), display.bounds().height());
rotate.Rotate(180);
break;
case 270:
rotate.Translate(0, display.bounds().width() - one_pixel);
rotate.Translate(0, display.bounds().width());
rotate.Rotate(270);
break;
}
Expand Down

0 comments on commit 291ba8b

Please sign in to comment.