Commit 5dd93023 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

ui: use vector icon for fullscreen icon in app menu

In order to fully fix the linked bug, I need to change the color of
this icon; doing that on a PNG is considerably painful. As such, this
change first switches from using the PNG to using a vector icon. That
allows cleaning up some bespoke image rendering code used with the
old PNG icon.

A followup CL will perform the actual color adjustment; this CL uses
the exact colors the old PNG used to avoid unnecessary visual changes.
The new Material icon does look slightly different from the old PNG,
but that delta (the icon being different) has been approved by UX.

TBR=oshima@chromium.org

Bug: 957391
Change-Id: I3787b561dbe15bf00977ad54dd57ce183115bbec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815733
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698792}
parent f2393ed0
......@@ -128,7 +128,6 @@
<structure type="chrome_scaled_image" name="IDR_FORWARD_H" file="common/browser_forward_hover.png" />
<structure type="chrome_scaled_image" name="IDR_FORWARD_P" file="common/browser_forward_pressed.png" />
</if>
<structure type="chrome_scaled_image" name="IDR_FULLSCREEN_MENU_BUTTON" file="common/fullscreen_menu_button.png" />
<if expr="_google_chrome">
<structure type="chrome_scaled_image" name="IDR_GOOGLE_ICON" file="google_chrome/google_icon.png" />
</if>
......
......@@ -52,6 +52,7 @@ aggregate_vector_icons("chrome_vector_icons") {
"file_download_shelf.icon",
"fingerprint.icon",
"forward_arrow_touch.icon",
"fullscreen.icon",
"generic_stop.icon",
"globe.icon",
"hardware_computer.icon",
......
// Copyright 2019 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.
// From https://material.io/resources/icons/?icon=fullscreen&style=baseline,
// converted via skiaify:
CANVAS_DIMENSIONS, 24,
MOVE_TO, 7, 14,
H_LINE_TO, 5,
R_V_LINE_TO, 5,
R_H_LINE_TO, 5,
R_V_LINE_TO, -2,
H_LINE_TO, 7,
R_V_LINE_TO, -3,
CLOSE,
R_MOVE_TO, -2, -4,
R_H_LINE_TO, 2,
V_LINE_TO, 7,
R_H_LINE_TO, 3,
V_LINE_TO, 5,
H_LINE_TO, 5,
R_V_LINE_TO, 5,
CLOSE,
R_MOVE_TO, 12, 7,
R_H_LINE_TO, -3,
R_V_LINE_TO, 2,
R_H_LINE_TO, 5,
R_V_LINE_TO, -5,
R_H_LINE_TO, -2,
R_V_LINE_TO, 3,
CLOSE,
MOVE_TO, 14, 5,
R_V_LINE_TO, 2,
R_H_LINE_TO, 3,
R_V_LINE_TO, 3,
R_H_LINE_TO, 2,
V_LINE_TO, 5,
R_H_LINE_TO, -5,
CLOSE
......@@ -891,8 +891,9 @@ void AppMenuModel::CreateZoomMenu() {
IDS_ZOOM_MINUS2);
zoom_menu_item_model_->AddGroupItemWithStringId(IDC_ZOOM_PLUS,
IDS_ZOOM_PLUS2);
zoom_menu_item_model_->AddItemWithImage(IDC_FULLSCREEN,
IDR_FULLSCREEN_MENU_BUTTON);
// TODO(https://crbug.com/957391): Remove the former IDR_ parameter here once
// the change to remove it from the model (crrev.com/1816118) is in.
zoom_menu_item_model_->AddItemWithImage(IDC_FULLSCREEN, -1);
AddButtonItem(IDC_ZOOM_MENU, zoom_menu_item_model_.get());
}
......
......@@ -19,6 +19,7 @@
#include "build/branding_buildflags.h"
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/search.h"
......@@ -52,7 +53,7 @@
#include "ui/gfx/canvas.h"
#include "ui/gfx/font_list.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia_source.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/gfx/scoped_canvas.h"
#include "ui/gfx/skia_util.h"
#include "ui/gfx/text_utils.h"
......@@ -360,39 +361,6 @@ class AppMenuView : public views::View, public views::ButtonListener {
DISALLOW_COPY_AND_ASSIGN(AppMenuView);
};
// Generate the button image for hover state.
class HoveredImageSource : public gfx::ImageSkiaSource {
public:
HoveredImageSource(const gfx::ImageSkia& image, SkColor color)
: image_(image),
color_(color) {
}
~HoveredImageSource() override {}
gfx::ImageSkiaRep GetImageForScale(float scale) override {
const gfx::ImageSkiaRep& rep = image_.GetRepresentation(scale);
SkBitmap bitmap = rep.GetBitmap();
SkBitmap white;
white.allocN32Pixels(bitmap.width(), bitmap.height());
white.eraseARGB(0, 0, 0, 0);
for (int y = 0; y < bitmap.height(); ++y) {
uint32_t* image_row = bitmap.getAddr32(0, y);
uint32_t* dst_row = white.getAddr32(0, y);
for (int x = 0; x < bitmap.width(); ++x) {
uint32_t image_pixel = image_row[x];
// Fill the non transparent pixels with |color_|.
dst_row[x] = (image_pixel & 0xFF000000) == 0x0 ? 0x0 : color_;
}
}
return gfx::ImageSkiaRep(white, scale);
}
private:
const gfx::ImageSkia image_;
const SkColor color_;
DISALLOW_COPY_AND_ASSIGN(HoveredImageSource);
};
} // namespace
// CutCopyPasteView ------------------------------------------------------------
......@@ -502,10 +470,14 @@ class AppMenu::ZoomView : public AppMenuView {
// all buttons on menu should must be a custom button in order for
// the keyboard navigation to work.
DCHECK(Button::AsButton(fullscreen_button_));
gfx::ImageSkia* full_screen_image =
ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
IDR_FULLSCREEN_MENU_BUTTON);
fullscreen_button_->SetImage(ImageButton::STATE_NORMAL, full_screen_image);
// TODO(https://crbug.com/957391): Do away with this bespoke color. Ideally
// this should be kColorId_EnabledMenuItemForegroundColor. The color here is
// ripped directly from the old PNG asset for this image.
fullscreen_button_->SetImage(
ImageButton::STATE_NORMAL,
gfx::CreateVectorIcon(kFullscreenIcon,
SkColorSetRGB(0x98, 0x98, 0x98)));
// Since |fullscreen_button_| will reside in a menu, make it ALWAYS
// focusable regardless of the platform.
......@@ -579,18 +551,14 @@ class AppMenu::ZoomView : public AppMenuView {
if (theme) {
zoom_label_->SetEnabledColor(theme->GetSystemColor(
ui::NativeTheme::kColorId_EnabledMenuItemForegroundColor));
gfx::ImageSkia* full_screen_image =
ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
IDR_FULLSCREEN_MENU_BUTTON);
SkColor fg_color = theme->GetSystemColor(
ui::NativeTheme::kColorId_SelectedMenuItemForegroundColor);
gfx::ImageSkia hovered_fullscreen_image(
std::make_unique<HoveredImageSource>(*full_screen_image, fg_color),
full_screen_image->size());
fullscreen_button_->SetImage(
ImageButton::STATE_HOVERED, &hovered_fullscreen_image);
fullscreen_button_->SetImage(
ImageButton::STATE_PRESSED, &hovered_fullscreen_image);
gfx::ImageSkia hovered_fullscreen_image = gfx::CreateVectorIcon(
kFullscreenIcon,
theme->GetSystemColor(
ui::NativeTheme::kColorId_SelectedMenuItemForegroundColor));
fullscreen_button_->SetImage(ImageButton::STATE_HOVERED,
hovered_fullscreen_image);
fullscreen_button_->SetImage(ImageButton::STATE_PRESSED,
hovered_fullscreen_image);
}
}
......
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