Commit 01c91250 authored by wutao's avatar wutao Committed by Commit Bot

ambient: Fix scaling photos

For paired portrait photos, server returns image with half screen width,
and screen height. We used to assume the returned image is portrait as
well in order to tile them. However, for ultra wide screen, the image
size could be landscape.
In this patch, we remove the portrait size limitation to tile two
images.

This patch also mitigates the scaling issue when device is in portrait
orientation. Previously we request width of portrait screen, and server
returns the half width, which is too small and causes the displayed
image blurry. To reduce the effect, we always send the landscape screen
size to the server.

Bug: b/171992309
Test: manual
Change-Id: I09c065032d7ec92549ea0eb441319cb98f97c84e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2509008
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822816}
parent d9065f9b
...@@ -465,14 +465,21 @@ void AmbientBackendControllerImpl::FetchScreenUpdateInfoInternal( ...@@ -465,14 +465,21 @@ void AmbientBackendControllerImpl::FetchScreenUpdateInfoInternal(
num_topics, gaia_id, access_token, client_id); num_topics, gaia_id, access_token, client_id);
auto resource_request = CreateResourceRequest(request); auto resource_request = CreateResourceRequest(request);
// Request photo with display size in pixel. // For portrait photos, the server returns image of half requested width.
// When the device is in portrait mode, where only shows one portrait photo,
// it will cause unnecessary scaling. To reduce this effect, always requesting
// the landscape display size.
// TODO(b/172075868): Support tiling in portrait mode.
gfx::Size display_size_px = GetDisplaySizeInPixel(); gfx::Size display_size_px = GetDisplaySizeInPixel();
const int width = std::max(display_size_px.width(), display_size_px.height());
const int height =
std::min(display_size_px.width(), display_size_px.height());
resource_request->url = resource_request->url =
net::AppendQueryParameter(resource_request->url, "device-screen-width", net::AppendQueryParameter(resource_request->url, "device-screen-width",
base::NumberToString(display_size_px.width())); base::NumberToString(width));
resource_request->url = resource_request->url =
net::AppendQueryParameter(resource_request->url, "device-screen-height", net::AppendQueryParameter(resource_request->url, "device-screen-height",
base::NumberToString(display_size_px.height())); base::NumberToString(height));
auto backdrop_url_loader = std::make_unique<BackdropURLLoader>(); auto backdrop_url_loader = std::make_unique<BackdropURLLoader>();
auto* loader_ptr = backdrop_url_loader.get(); auto* loader_ptr = backdrop_url_loader.get();
......
...@@ -280,7 +280,7 @@ void AmbientBackgroundImageView::UpdateGlanceableInfoPosition() { ...@@ -280,7 +280,7 @@ void AmbientBackgroundImageView::UpdateGlanceableInfoPosition() {
bool AmbientBackgroundImageView::UpdateRelatedImageViewVisibility() { bool AmbientBackgroundImageView::UpdateRelatedImageViewVisibility() {
const bool did_show_pair = related_image_view_->GetVisible(); const bool did_show_pair = related_image_view_->GetVisible();
const bool show_pair = IsLandscapeOrientation() && HasPairedPortraitImages(); const bool show_pair = IsLandscapeOrientation() && HasPairedImages();
related_image_view_->SetVisible(show_pair); related_image_view_->SetVisible(show_pair);
return did_show_pair != show_pair; return did_show_pair != show_pair;
} }
...@@ -307,10 +307,8 @@ bool AmbientBackgroundImageView::IsLandscapeOrientation() const { ...@@ -307,10 +307,8 @@ bool AmbientBackgroundImageView::IsLandscapeOrientation() const {
return width() > height(); return width() > height();
} }
bool AmbientBackgroundImageView::HasPairedPortraitImages() const { bool AmbientBackgroundImageView::HasPairedImages() const {
const auto& primary_image = image_unscaled_; return !image_unscaled_.isNull() && !related_image_unscaled_.isNull();
return !primary_image.isNull() && !related_image_unscaled_.isNull() &&
primary_image.height() > primary_image.width();
} }
BEGIN_METADATA(AmbientBackgroundImageView, views::View) BEGIN_METADATA(AmbientBackgroundImageView, views::View)
......
...@@ -69,7 +69,7 @@ class ASH_EXPORT AmbientBackgroundImageView : public views::View, ...@@ -69,7 +69,7 @@ class ASH_EXPORT AmbientBackgroundImageView : public views::View,
// Whether the device is in landscape orientation. // Whether the device is in landscape orientation.
bool IsLandscapeOrientation() const; bool IsLandscapeOrientation() const;
bool HasPairedPortraitImages() const; bool HasPairedImages() const;
// Owned by |AmbientController| and should always outlive |this|. // Owned by |AmbientController| and should always outlive |this|.
AmbientViewDelegate* delegate_ = nullptr; AmbientViewDelegate* delegate_ = nullptr;
......
...@@ -114,7 +114,8 @@ TEST_F(AmbientPhotoViewTest, ...@@ -114,7 +114,8 @@ TEST_F(AmbientPhotoViewTest,
gfx::Rect(/*x=*/0, /*y=*/-100, /*width=*/400, /*height=*/800)); gfx::Rect(/*x=*/0, /*y=*/-100, /*width=*/400, /*height=*/800));
} }
// Test that landscape images will not be tiled when screen is landscape. // Test that landscape images can be tiled when screen is landscape as long as
// they are related.
TEST_F(AmbientPhotoViewTest, TEST_F(AmbientPhotoViewTest,
ShouldNotTileTwoLandscapeImagesForLandscapeScreen) { ShouldNotTileTwoLandscapeImagesForLandscapeScreen) {
SetPhotoViewImageSize(/*width=*/20, /*height=*/10); SetPhotoViewImageSize(/*width=*/20, /*height=*/10);
...@@ -127,12 +128,13 @@ TEST_F(AmbientPhotoViewTest, ...@@ -127,12 +128,13 @@ TEST_F(AmbientPhotoViewTest,
auto* image_view = GetAmbientBackgroundImageView(); auto* image_view = GetAmbientBackgroundImageView();
// Only show one landscape image. // Show two landscape image.
// Image should be full height. Image width should extend equally to the left // Image should be full height. Image width should extend equally to the left
// and right of the visible part of the screen. // and right of the visible part of the screen.
ASSERT_EQ(image_view->GetImageBoundsForTesting(), ASSERT_EQ(image_view->GetImageBoundsForTesting(),
gfx::Rect(/*x=*/-196, /*y=*/0, /*width=*/1200, /*height=*/600)); gfx::Rect(/*x=*/0, /*y=*/200, /*width=*/400, /*height=*/200));
ASSERT_EQ(image_view->GetRelatedImageBoundsForTesting(), gfx::Rect()); ASSERT_EQ(image_view->GetRelatedImageBoundsForTesting(),
gfx::Rect(/*x=*/0, /*y=*/200, /*width=*/400, /*height=*/200));
} }
// Test that only have one available image will not be tiled when screen is // Test that only have one available image will not be tiled when screen is
...@@ -164,9 +166,9 @@ TEST_F(AmbientPhotoViewTest, ...@@ -164,9 +166,9 @@ TEST_F(AmbientPhotoViewTest,
} }
// Test that image is scaled to fill screen height when the image is landscape // Test that image is scaled to fill screen height when the image is landscape
// and the screen is landscape. The image will be zoomed in and the left and // and no related image when the screen is landscape.
// right will be cut off, as the width of the image is greater than the width of // The image will be zoomed in and the left and right will be cut off, as the
// the screen. // width of the image is greater than the width of the screen.
TEST_F(AmbientPhotoViewTest, ShouldResizeLandscapeImageForLandscapeScreen) { TEST_F(AmbientPhotoViewTest, ShouldResizeLandscapeImageForLandscapeScreen) {
SetPhotoViewImageSize(/*width=*/30, /*height=*/20); SetPhotoViewImageSize(/*width=*/30, /*height=*/20);
...@@ -178,10 +180,16 @@ TEST_F(AmbientPhotoViewTest, ShouldResizeLandscapeImageForLandscapeScreen) { ...@@ -178,10 +180,16 @@ TEST_F(AmbientPhotoViewTest, ShouldResizeLandscapeImageForLandscapeScreen) {
auto* image_view = GetAmbientBackgroundImageView(); auto* image_view = GetAmbientBackgroundImageView();
// Remove the related image.
image_view->ResetRelatedImageForTesting();
// Trigger layout.
UpdateDisplay("808x600");
// Image should be full height. Image width should extend equally to the left // Image should be full height. Image width should extend equally to the left
// and right of the visible part of the screen. // and right of the visible part of the screen.
ASSERT_EQ(image_view->GetImageBoundsForTesting(), ASSERT_EQ(image_view->GetImageBoundsForTesting(),
gfx::Rect(/*x=*/-50, /*y=*/0, /*width=*/900, /*height=*/600)); gfx::Rect(/*x=*/-46, /*y=*/0, /*width=*/900, /*height=*/600));
ASSERT_EQ(image_view->GetRelatedImageBoundsForTesting(), gfx::Rect()); ASSERT_EQ(image_view->GetRelatedImageBoundsForTesting(), gfx::Rect());
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment