Commit 61956122 authored by nancylingwang's avatar nancylingwang Committed by Commit Bot

Resolve the Arc icons pixelated issue.

Some Arc app icons are pixelated, e.g. spotify. The reasons is the
background/foreground icon image fetched from the Arc side is pixelated.
The Arc side cl:12384305 modifies the Arc side implementation to fetch
the high quality raw icon foreground/background image:
https://googleplex-android-review.git.corp.google.com/c/platform/vendor/google_arc/+/12384305

This CL implements the chromium side change to generate the adaptive
icon in Chrome OS.
1. The raw foreground/background icon size is not the requested icon
size. So modify ArcAppIcon, adding a flag, extract_subset_allowed_, to
allow other size image to be forwarded to AppService for the
foreground/background image.

(Note: Arc default app icons may have different size
background/foreground as well, but the case should be rare, because we
just enable the flag for 1 week. Also most time the default app icons
should match the size. If there are cases that the current Canary
release is updated, and the default add icons are wrong due to the
migration from last 1 week version, we can suggest disable/enable
the Arc play store to refresh to the new image size.)

2. Modify AppService Arc icon handling to generate the adaptive icon
using the raw background/foreground icon images. The Arc raw
background/foreground image has padding for all 4 sides. So if the
foreground/background icon image doesn't match the requested size, chop
paddings and resize image_rep to the appropriate size. Then combine the
foreground/background images, and apply the mask.

This CL and the Arc side cl:12384305 work together to resolve the Arc
app icons pixelated issue. This CL should be submitted first, and this
CL is backward compatible.

BUG=1114771

Change-Id: I70af928f5b6c81c28f71c54efa102b7b1c617a0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359311
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: default avatarLong Cheng <lgcheng@google.com>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798806}
parent dbb290b6
......@@ -60,6 +60,7 @@
#include "chrome/browser/ui/app_list/md_icon_normalizer.h"
#include "chrome/grit/chrome_unscaled_resources.h"
#include "ui/chromeos/resources/grit/ui_chromeos_resources.h"
#include "ui/gfx/skia_util.h"
#endif
namespace {
......@@ -67,6 +68,11 @@ namespace {
static const int kInvalidIconResource = 0;
#if defined(OS_CHROMEOS)
// Copy from Android code, all four sides of the ARC foreground and background
// images are padded 25% of it's width and height.
float kAndroidAdaptiveIconPaddingPercentage = 1.0f / 8.0f;
using SizeToImageSkiaRep = std::map<int, gfx::ImageSkiaRep>;
using ScaleToImageSkiaReps = std::map<float, SizeToImageSkiaRep>;
using MaskImageSkiaReps = std::pair<SkBitmap, ScaleToImageSkiaReps>;
......@@ -137,6 +143,61 @@ gfx::ImageSkia LoadMaskImage(const ScaleToSize& scale_to_size) {
return mask_image;
}
bool IsConsistentPixelSize(const gfx::ImageSkiaRep& rep,
const gfx::ImageSkia& image_skia) {
// The pixel size calculation method must be consistent with
// ArcAppIconDescriptor::GetSizeInPixels.
return rep.pixel_width() == roundf(image_skia.width() * rep.scale()) &&
rep.pixel_height() == roundf(image_skia.height() * rep.scale());
}
// Return whether the image_reps in |image_skia| should be chopped for
// paddings. If the image_rep's pixel size is inconsistent with the scaled width
// and height, that means the image_rep has paddings and should be chopped to
// remove paddings. Otherwise, the image_rep doesn't have padding, and should
// not be chopped.
bool ShouldExtractSubset(const gfx::ImageSkia& image_skia) {
DCHECK(!image_skia.image_reps().empty());
for (const auto& rep : image_skia.image_reps()) {
if (!IsConsistentPixelSize(rep, image_skia)) {
return true;
}
}
return false;
}
// Chop paddings for all four sides of |image_skia|, and resize image_rep to the
// appropriate size.
gfx::ImageSkia ExtractSubsetForArcImage(const gfx::ImageSkia& image_skia) {
gfx::ImageSkia subset_image;
for (const auto& rep : image_skia.image_reps()) {
if (IsConsistentPixelSize(rep, image_skia)) {
continue;
}
int padding_width =
rep.pixel_width() * kAndroidAdaptiveIconPaddingPercentage;
int padding_height =
rep.pixel_height() * kAndroidAdaptiveIconPaddingPercentage;
// Chop paddings for all four sides of |image_skia|.
SkBitmap dst;
bool success = rep.GetBitmap().extractSubset(
&dst,
RectToSkIRect(gfx::Rect(padding_width, padding_height,
rep.pixel_width() - 2 * padding_width,
rep.pixel_height() - 2 * padding_height)));
DCHECK(success);
// Resize |rep| to size_hint_in_dip * rep.scale().
const SkBitmap resized = skia::ImageOperations::Resize(
dst, skia::ImageOperations::RESIZE_LANCZOS3,
image_skia.width() * rep.scale(), image_skia.height() * rep.scale());
subset_image.AddRepresentation(gfx::ImageSkiaRep(resized, rep.scale()));
}
return subset_image;
}
#endif
std::map<std::pair<int, int>, gfx::ImageSkia>& GetResourceIconCache() {
......@@ -988,10 +1049,28 @@ gfx::ImageSkia ApplyBackgroundAndMask(const gfx::ImageSkia& image) {
gfx::ImageSkia CompositeImagesAndApplyMask(
const gfx::ImageSkia& foreground_image,
const gfx::ImageSkia& background_image) {
bool should_extract_subset_foreground = ShouldExtractSubset(foreground_image);
bool should_extract_subset_background = ShouldExtractSubset(background_image);
if (!should_extract_subset_foreground && !should_extract_subset_background) {
return gfx::ImageSkiaOperations::CreateMaskedImage(
gfx::ImageSkiaOperations::CreateSuperimposedImage(background_image,
foreground_image),
LoadMaskImage(GetScaleToSize(foreground_image)));
}
// If the foreground or background image has padding, chop the padding of the
// four sides, and resize the image_reps for different scales.
gfx::ImageSkia foreground = should_extract_subset_foreground
? ExtractSubsetForArcImage(foreground_image)
: foreground_image;
gfx::ImageSkia background = should_extract_subset_background
? ExtractSubsetForArcImage(background_image)
: background_image;
return gfx::ImageSkiaOperations::CreateMaskedImage(
gfx::ImageSkiaOperations::CreateSuperimposedImage(background_image,
foreground_image),
LoadMaskImage(GetScaleToSize(foreground_image)));
gfx::ImageSkiaOperations::CreateSuperimposedImage(background, foreground),
LoadMaskImage(GetScaleToSize(foreground)));
}
void ArcRawIconPngDataToImageSkia(
......
......@@ -146,6 +146,7 @@ class ArcAppIcon::DecodeRequest : public ImageDecoder::ImageRequest {
ArcAppIcon& host,
const ArcAppIconDescriptor& descriptor,
bool resize_allowed,
bool retain_padding,
gfx::ImageSkia& image_skia,
std::map<ui::ScaleFactor, base::Time>& incomplete_scale_factors);
~DecodeRequest() override;
......@@ -158,6 +159,7 @@ class ArcAppIcon::DecodeRequest : public ImageDecoder::ImageRequest {
ArcAppIcon& host_;
const ArcAppIconDescriptor descriptor_;
const bool resize_allowed_;
const bool retain_padding_;
gfx::ImageSkia& image_skia_;
std::map<ui::ScaleFactor, base::Time>& incomplete_scale_factors_;
DISALLOW_COPY_AND_ASSIGN(DecodeRequest);
......@@ -170,11 +172,13 @@ ArcAppIcon::DecodeRequest::DecodeRequest(
ArcAppIcon& host,
const ArcAppIconDescriptor& descriptor,
bool resize_allowed,
bool retain_padding,
gfx::ImageSkia& image_skia,
std::map<ui::ScaleFactor, base::Time>& incomplete_scale_factors)
: host_(host),
descriptor_(descriptor),
resize_allowed_(resize_allowed),
retain_padding_(retain_padding),
image_skia_(image_skia),
incomplete_scale_factors_(incomplete_scale_factors) {}
......@@ -186,7 +190,12 @@ void ArcAppIcon::DecodeRequest::OnImageDecoded(const SkBitmap& bitmap) {
DCHECK(!bitmap.isNull() && !bitmap.empty());
const int expected_dim = descriptor_.GetSizeInPixels();
if (bitmap.width() != expected_dim || bitmap.height() != expected_dim) {
// If retain_padding is true, that means the foreground or the
// background image has paddings which should be chopped by AppService, so the
// raw image is forwarded without resize.
if (!retain_padding_ &&
(bitmap.width() != expected_dim || bitmap.height() != expected_dim)) {
if (!resize_allowed_) {
VLOG(2) << "Decoded ARC icon has unexpected dimension " << bitmap.width()
<< "x" << bitmap.height() << ". Expected " << expected_dim << ".";
......@@ -612,8 +621,8 @@ void ArcAppIcon::OnIconRead(
DecodeImage(read_result->unsafe_icon_data[0],
ArcAppIconDescriptor(resource_size_in_dip_,
read_result->scale_factor),
read_result->resize_allowed, image_skia_,
incomplete_scale_factors_);
read_result->resize_allowed, false /* retain_padding */,
image_skia_, incomplete_scale_factors_);
return;
}
case IconType::kCompressed: {
......@@ -631,7 +640,8 @@ void ArcAppIcon::OnIconRead(
DecodeImage(read_result->unsafe_icon_data[0],
ArcAppIconDescriptor(resource_size_in_dip_,
read_result->scale_factor),
read_result->resize_allowed, foreground_image_skia_,
read_result->resize_allowed, false /* retain_padding */,
foreground_image_skia_,
foreground_incomplete_scale_factors_);
background_incomplete_scale_factors_.clear();
return;
......@@ -642,13 +652,13 @@ void ArcAppIcon::OnIconRead(
DecodeImage(read_result->unsafe_icon_data[0],
ArcAppIconDescriptor(resource_size_in_dip_,
read_result->scale_factor),
read_result->resize_allowed, foreground_image_skia_,
foreground_incomplete_scale_factors_);
read_result->resize_allowed, true /* retain_padding */,
foreground_image_skia_, foreground_incomplete_scale_factors_);
DecodeImage(read_result->unsafe_icon_data[1],
ArcAppIconDescriptor(resource_size_in_dip_,
read_result->scale_factor),
read_result->resize_allowed, background_image_skia_,
background_incomplete_scale_factors_);
read_result->resize_allowed, true /* retain_padding */,
background_image_skia_, background_incomplete_scale_factors_);
return;
}
}
......@@ -658,10 +668,12 @@ void ArcAppIcon::DecodeImage(
const std::string& unsafe_icon_data,
const ArcAppIconDescriptor& descriptor,
bool resize_allowed,
bool retain_padding,
gfx::ImageSkia& image_skia,
std::map<ui::ScaleFactor, base::Time>& incomplete_scale_factors) {
decode_requests_.emplace_back(std::make_unique<DecodeRequest>(
*this, descriptor, resize_allowed, image_skia, incomplete_scale_factors));
*this, descriptor, resize_allowed, retain_padding, image_skia,
incomplete_scale_factors));
if (disable_safe_decoding_for_testing) {
SkBitmap bitmap;
if (!unsafe_icon_data.empty() &&
......
......@@ -204,6 +204,7 @@ class ArcAppIcon {
const std::string& unsafe_icon_data,
const ArcAppIconDescriptor& descriptor,
bool resize_allowed,
bool retain_padding,
gfx::ImageSkia& image_skia,
std::map<ui::ScaleFactor, base::Time>& incomplete_scale_factors);
void UpdateImageSkia(
......
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