Commit 6f042bc6 authored by Tom Anderson's avatar Tom Anderson Committed by Commit Bot

Improve image sizing for status icons

* Use 128x128 logo for the "background mode" status icon on Linux.
  * The 22x22 logo is now unused, so remove it.  [1] removes internal
    icons.
* Always use the highest resolution image representation that we're
  given, since all status icon implementations want to draw in pixel
  coordinates and we don't know in general how large the containing
  window will be.
* Change StatusIconLinuxX11 drawing logic to handle icon resizing.

[1] https://chrome-internal-review.googlesource.com/c/chrome/theme/google_chrome/+/1410578/

BUG=419673
R=thestig

Change-Id: I2cad9414ba24022b9305d808c8b30228ae5d5c17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1671427Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671514}
parent ec99dbf1
...@@ -37,9 +37,6 @@ ...@@ -37,9 +37,6 @@
<if expr="is_macosx"> <if expr="is_macosx">
<include name="IDR_STATUS_TRAY_ICON" file="google_chrome/product_logo_22_mono.png" type="BINDATA" /> <include name="IDR_STATUS_TRAY_ICON" file="google_chrome/product_logo_22_mono.png" type="BINDATA" />
</if> </if>
<if expr="not is_macosx and not is_win">
<include name="IDR_STATUS_TRAY_ICON" file="google_chrome/product_logo_22.png" type="BINDATA" />
</if>
<if expr="chromeos"> <if expr="chromeos">
<include name="IDR_CHROME_APP_ICON_32" file="google_chrome/chromeos/chrome_app_icon_32.png" type="BINDATA" /> <include name="IDR_CHROME_APP_ICON_32" file="google_chrome/chromeos/chrome_app_icon_32.png" type="BINDATA" />
<include name="IDR_CHROME_APP_ICON_192" file="google_chrome/chromeos/chrome_app_icon_192.png" type="BINDATA" /> <include name="IDR_CHROME_APP_ICON_192" file="google_chrome/chromeos/chrome_app_icon_192.png" type="BINDATA" />
...@@ -72,9 +69,6 @@ ...@@ -72,9 +69,6 @@
<if expr="is_macosx"> <if expr="is_macosx">
<include name="IDR_STATUS_TRAY_ICON" file="chromium/product_logo_22_mono.png" type="BINDATA" /> <include name="IDR_STATUS_TRAY_ICON" file="chromium/product_logo_22_mono.png" type="BINDATA" />
</if> </if>
<if expr="not is_macosx and not is_win">
<include name="IDR_STATUS_TRAY_ICON" file="chromium/product_logo_22.png" type="BINDATA" />
</if>
<if expr="chromeos"> <if expr="chromeos">
<include name="IDR_CHROME_APP_ICON_32" file="chromium/chromeos/chrome_app_icon_32.png" type="BINDATA" /> <include name="IDR_CHROME_APP_ICON_32" file="chromium/chromeos/chrome_app_icon_32.png" type="BINDATA" />
<include name="IDR_CHROME_APP_ICON_192" file="chromium/chromeos/chrome_app_icon_192.png" type="BINDATA" /> <include name="IDR_CHROME_APP_ICON_192" file="chromium/chromeos/chrome_app_icon_192.png" type="BINDATA" />
......
...@@ -769,10 +769,15 @@ gfx::ImageSkia GetStatusTrayIcon() { ...@@ -769,10 +769,15 @@ gfx::ImageSkia GetStatusTrayIcon() {
return gfx::ImageSkia(); return gfx::ImageSkia();
return family->CreateExact(size).AsImageSkia(); return family->CreateExact(size).AsImageSkia();
#else #elif defined(OS_LINUX)
// On other platforms, just get a static resource image. return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
IDR_PRODUCT_LOGO_128);
#elif defined(OS_MACOSX)
return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed( return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
IDR_STATUS_TRAY_ICON); IDR_STATUS_TRAY_ICON);
#else
NOTREACHED();
return gfx::ImageSkia();
#endif #endif
} }
......
...@@ -27,6 +27,20 @@ constexpr base::Feature kEnableDbusAndX11StatusIcons{ ...@@ -27,6 +27,20 @@ constexpr base::Feature kEnableDbusAndX11StatusIcons{
// Prefix for app indicator ids // Prefix for app indicator ids
const char kAppIndicatorIdPrefix[] = "chrome_app_indicator_"; const char kAppIndicatorIdPrefix[] = "chrome_app_indicator_";
gfx::ImageSkia GetBestImageRep(const gfx::ImageSkia& image) {
float best_scale = 0.0f;
SkBitmap best_rep;
for (const auto& rep : image.image_reps()) {
if (rep.scale() > best_scale) {
best_scale = rep.scale();
best_rep = rep.GetBitmap();
}
}
// All status icon implementations want the image in pixel coordinates, so use
// a scale factor of 1.
return gfx::ImageSkia(gfx::ImageSkiaRep(best_rep, 1.0f));
}
} // namespace } // namespace
StatusIconLinuxWrapper::StatusIconLinuxWrapper( StatusIconLinuxWrapper::StatusIconLinuxWrapper(
...@@ -36,7 +50,7 @@ StatusIconLinuxWrapper::StatusIconLinuxWrapper( ...@@ -36,7 +50,7 @@ StatusIconLinuxWrapper::StatusIconLinuxWrapper(
const base::string16& tool_tip) const base::string16& tool_tip)
: status_icon_(std::move(status_icon)), : status_icon_(std::move(status_icon)),
status_icon_type_(status_icon_type), status_icon_type_(status_icon_type),
image_(image), image_(GetBestImageRep(image)),
tool_tip_(tool_tip), tool_tip_(tool_tip),
menu_model_(nullptr) { menu_model_(nullptr) {
status_icon_->SetDelegate(this); status_icon_->SetDelegate(this);
...@@ -48,9 +62,9 @@ StatusIconLinuxWrapper::~StatusIconLinuxWrapper() { ...@@ -48,9 +62,9 @@ StatusIconLinuxWrapper::~StatusIconLinuxWrapper() {
} }
void StatusIconLinuxWrapper::SetImage(const gfx::ImageSkia& image) { void StatusIconLinuxWrapper::SetImage(const gfx::ImageSkia& image) {
image_ = image; image_ = GetBestImageRep(image);
if (status_icon_) if (status_icon_)
status_icon_->SetIcon(image); status_icon_->SetIcon(image_);
} }
void StatusIconLinuxWrapper::SetToolTip(const base::string16& tool_tip) { void StatusIconLinuxWrapper::SetToolTip(const base::string16& tool_tip) {
......
...@@ -17,7 +17,10 @@ ...@@ -17,7 +17,10 @@
#include "ui/aura/window_tree_host.h" #include "ui/aura/window_tree_host.h"
#include "ui/base/x/x11_util.h" #include "ui/base/x/x11_util.h"
#include "ui/events/platform/x11/x11_event_source.h" #include "ui/events/platform/x11/x11_event_source.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#include "ui/gfx/scoped_canvas.h"
#include "ui/gfx/transform.h"
#include "ui/gfx/x/x11.h" #include "ui/gfx/x/x11.h"
#include "ui/gfx/x/x11_atom_cache.h" #include "ui/gfx/x/x11_atom_cache.h"
#include "ui/gfx/x/x11_error_tracker.h" #include "ui/gfx/x/x11_error_tracker.h"
...@@ -37,12 +40,12 @@ const int16_t kInitialWindowPos = std::numeric_limits<int16_t>::min(); ...@@ -37,12 +40,12 @@ const int16_t kInitialWindowPos = std::numeric_limits<int16_t>::min();
} // namespace } // namespace
StatusIconLinuxX11::StatusIconLinuxX11() : ImageButton(this) {} StatusIconLinuxX11::StatusIconLinuxX11() : Button(this) {}
StatusIconLinuxX11::~StatusIconLinuxX11() = default; StatusIconLinuxX11::~StatusIconLinuxX11() = default;
void StatusIconLinuxX11::SetIcon(const gfx::ImageSkia& image) { void StatusIconLinuxX11::SetIcon(const gfx::ImageSkia& image) {
SetImage(views::Button::STATE_NORMAL, image); // Nothing to do.
} }
void StatusIconLinuxX11::SetToolTip(const base::string16& tool_tip) { void StatusIconLinuxX11::SetToolTip(const base::string16& tool_tip) {
...@@ -71,7 +74,7 @@ void StatusIconLinuxX11::OnSetDelegate() { ...@@ -71,7 +74,7 @@ void StatusIconLinuxX11::OnSetDelegate() {
auto host = std::make_unique<views::DesktopWindowTreeHostX11>( auto host = std::make_unique<views::DesktopWindowTreeHostX11>(
widget_.get(), native_widget.get()); widget_.get(), native_widget.get());
aura::WindowTreeHost* window_tree_host = host.get(); host_ = host.get();
int visual_id; int visual_id;
if (ui::GetIntProperty(manager, "_NET_SYSTEM_TRAY_VISUAL", &visual_id)) if (ui::GetIntProperty(manager, "_NET_SYSTEM_TRAY_VISUAL", &visual_id))
...@@ -98,15 +101,14 @@ void StatusIconLinuxX11::OnSetDelegate() { ...@@ -98,15 +101,14 @@ void StatusIconLinuxX11::OnSetDelegate() {
widget_->Init(params); widget_->Init(params);
Window window = window_tree_host->GetAcceleratedWidget(); Window window = host_->GetAcceleratedWidget();
DCHECK(window); DCHECK(window);
widget_->SetContentsView(this); widget_->SetContentsView(this);
set_owned_by_client(); set_owned_by_client();
SetBorder(nullptr);
SetIcon(delegate_->GetImage()); SetIcon(delegate_->GetImage());
SetImageHorizontalAlignment(ALIGN_CENTER);
SetImageVerticalAlignment(ALIGN_MIDDLE);
SetTooltipText(delegate_->GetToolTip()); SetTooltipText(delegate_->GetToolTip());
set_context_menu_controller(this); set_context_menu_controller(this);
...@@ -154,3 +156,29 @@ void StatusIconLinuxX11::ShowContextMenuForViewImpl( ...@@ -154,3 +156,29 @@ void StatusIconLinuxX11::ShowContextMenuForViewImpl(
void StatusIconLinuxX11::ButtonPressed(Button* sender, const ui::Event& event) { void StatusIconLinuxX11::ButtonPressed(Button* sender, const ui::Event& event) {
delegate_->OnClick(); delegate_->OnClick();
} }
void StatusIconLinuxX11::PaintButtonContents(gfx::Canvas* canvas) {
gfx::ScopedCanvas scoped_canvas(canvas);
canvas->UndoDeviceScaleFactor();
gfx::Rect bounds = host_->GetBoundsInPixels();
const gfx::ImageSkia& image = delegate_->GetImage();
// If the image fits in the window, center it. But if it won't fit, downscale
// it preserving aspect ratio.
float scale =
std::min({1.0f, static_cast<float>(bounds.width()) / image.width(),
static_cast<float>(bounds.height()) / image.height()});
float x_offset = (bounds.width() - scale * image.width()) / 2.0f;
float y_offset = (bounds.height() - scale * image.height()) / 2.0f;
gfx::Transform transform;
transform.Translate(x_offset, y_offset);
transform.Scale(scale, scale);
canvas->Transform(transform);
cc::PaintFlags flags;
flags.setFilterQuality(kHigh_SkFilterQuality);
canvas->DrawImageInt(image, 0, 0, image.width(), image.height(), 0, 0,
image.width(), image.height(), true, flags);
}
...@@ -11,15 +11,18 @@ ...@@ -11,15 +11,18 @@
#include "ui/gfx/x/x11_types.h" #include "ui/gfx/x/x11_types.h"
#include "ui/views/context_menu_controller.h" #include "ui/views/context_menu_controller.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/controls/menu/menu_runner.h" #include "ui/views/controls/menu/menu_runner.h"
#include "ui/views/linux_ui/status_icon_linux.h" #include "ui/views/linux_ui/status_icon_linux.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
namespace aura {
class WindowTreeHost;
}
// A status icon that uses the XEmbed protocol. // A status icon that uses the XEmbed protocol.
// https://standards.freedesktop.org/xembed-spec/xembed-spec-latest.html // https://standards.freedesktop.org/xembed-spec/xembed-spec-latest.html
class StatusIconLinuxX11 : public views::StatusIconLinux, class StatusIconLinuxX11 : public views::StatusIconLinux,
public views::ImageButton, public views::Button,
public views::ContextMenuController, public views::ContextMenuController,
public views::ButtonListener { public views::ButtonListener {
public: public:
...@@ -40,9 +43,14 @@ class StatusIconLinuxX11 : public views::StatusIconLinux, ...@@ -40,9 +43,14 @@ class StatusIconLinuxX11 : public views::StatusIconLinux,
// views::ButtonListener: // views::ButtonListener:
void ButtonPressed(Button* sender, const ui::Event& event) override; void ButtonPressed(Button* sender, const ui::Event& event) override;
// views::Button:
void PaintButtonContents(gfx::Canvas* canvas) override;
private: private:
std::unique_ptr<views::Widget> widget_; std::unique_ptr<views::Widget> widget_;
aura::WindowTreeHost* host_ = nullptr;
std::unique_ptr<views::MenuRunner> menu_runner_; std::unique_ptr<views::MenuRunner> menu_runner_;
DISALLOW_COPY_AND_ASSIGN(StatusIconLinuxX11); DISALLOW_COPY_AND_ASSIGN(StatusIconLinuxX11);
......
...@@ -278,7 +278,6 @@ copy("theme_files") { ...@@ -278,7 +278,6 @@ copy("theme_files") {
"$branding_dir/BRANDING", "$branding_dir/BRANDING",
"$branding_dir/linux/product_logo_32.xpm", "$branding_dir/linux/product_logo_32.xpm",
"$branding_dir/product_logo_128.png", "$branding_dir/product_logo_128.png",
"$branding_dir/product_logo_22.png",
"$branding_dir/product_logo_24.png", "$branding_dir/product_logo_24.png",
"$branding_dir/product_logo_256.png", "$branding_dir/product_logo_256.png",
"$branding_dir/product_logo_48.png", "$branding_dir/product_logo_48.png",
...@@ -292,8 +291,6 @@ copy("theme_files") { ...@@ -292,8 +291,6 @@ copy("theme_files") {
"$branding_dir/linux/product_logo_32_dev.xpm", "$branding_dir/linux/product_logo_32_dev.xpm",
"$branding_dir/product_logo_128_beta.png", "$branding_dir/product_logo_128_beta.png",
"$branding_dir/product_logo_128_dev.png", "$branding_dir/product_logo_128_dev.png",
"$branding_dir/product_logo_22_beta.png",
"$branding_dir/product_logo_22_dev.png",
"$branding_dir/product_logo_24_beta.png", "$branding_dir/product_logo_24_beta.png",
"$branding_dir/product_logo_24_dev.png", "$branding_dir/product_logo_24_dev.png",
"$branding_dir/product_logo_256_beta.png", "$branding_dir/product_logo_256_beta.png",
......
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