Skip to content

Commit

Permalink
Omnibox/Views: Make Omnibox drop down icons 20x20.
Browse files Browse the repository at this point in the history
Update the icon sizes used in the omnibox popup/dropdown to change dynamically
depending on whether touch mode is on or off.

For favicons and extension icons which have fixed sized 16x16 icons supplied by
the developer, add extra padding around it to align them properly with the
correctly sized vector icons.

Bug: 801583
Change-Id: I23a9a2c5b5e3ae35b5c975ef1e3b7b6f5c7a9282
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/945294
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547630}
  • Loading branch information
plorcupine authored and Commit Bot committed Apr 3, 2018
1 parent 027f319 commit 077b476
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 48 deletions.
20 changes: 19 additions & 1 deletion chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "build/build_config.h"
#include "chrome/browser/search/search.h"
#include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/layout_constants.h"
#include "chrome/browser/ui/omnibox/omnibox_theme.h"
#include "chrome/browser/ui/views/location_bar/background_with_1_px_border.h"
#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
Expand All @@ -33,6 +34,7 @@
#include "ui/gfx/animation/slide_animation.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/geometry/safe_integer_conversions.h"
#include "ui/gfx/image/canvas_image_source.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia_operations.h"
#include "ui/gfx/path.h"
Expand Down Expand Up @@ -248,7 +250,23 @@ void OmniboxPopupContentsView::OpenMatch(size_t index,
gfx::Image OmniboxPopupContentsView::GetMatchIcon(
const AutocompleteMatch& match,
SkColor vector_icon_color) const {
return model_->GetMatchIcon(match, vector_icon_color);
gfx::Image icon = model_->GetMatchIcon(match, vector_icon_color);
if (icon.IsEmpty())
return icon;

const int icon_size = GetLayoutConstant(LOCATION_BAR_ICON_SIZE);
// In touch mode, icons are 20x20. FaviconCache and ExtensionIconManager both
// guarantee favicons and extension icons will be 16x16, so add extra padding
// around them to align them vertically with the other vector icons.
DCHECK_GE(icon_size, icon.Height());
DCHECK_GE(icon_size, icon.Width());
gfx::Insets padding_border((icon_size - icon.Height()) / 2,
(icon_size - icon.Width()) / 2);
if (!padding_border.IsEmpty()) {
return gfx::Image(gfx::CanvasImageSource::CreatePadded(*icon.ToImageSkia(),
padding_border));
}
return icon;
}

OmniboxTint OmniboxPopupContentsView::GetTint() const {
Expand Down
8 changes: 5 additions & 3 deletions chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ OmniboxTabSwitchButton::OmniboxTabSwitchButton(OmniboxResultView* result_view,
: MdTextButton(this, views::style::CONTEXT_BUTTON_MD),
text_height_(text_height),
result_view_(result_view) {
// TODO: SetTooltipText(text);
// SetImageAlignment(ALIGN_CENTER, ALIGN_MIDDLE);
// TODO(krb): SetTooltipText(text);
// SetImageAlignment(ALIGN_CENTER, ALIGN_MIDDLE);
SetBgColorOverride(GetBackgroundColor());
SetImage(STATE_NORMAL,
gfx::CreateVectorIcon(omnibox::kSwitchIcon, 16, SK_ColorBLACK));
gfx::CreateVectorIcon(omnibox::kSwitchIcon,
GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
SK_ColorBLACK));
SetText(base::ASCIIToUTF16("Switch to open tab"));
set_corner_radius(CalculatePreferredSize().height() / 2.f);
}
Expand Down
28 changes: 4 additions & 24 deletions chrome/browser/ui/views/profiles/dice_accounts_menu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,37 +24,17 @@ constexpr int kUseAnotherAccountCmdId = std::numeric_limits<int>::max();
// Anchor inset used to position the accounts menu.
constexpr int kAnchorInset = 8;

// TODO(tangltom): Calculate these values considering the existing menu config
// TODO(tangltom): Calculate these values considering the existing menu config.
constexpr int kVerticalImagePadding = 9;
constexpr int kHorizontalImagePadding = 6;

class PaddedImageSource : public gfx::CanvasImageSource {
public:
explicit PaddedImageSource(const gfx::Image& image)
: CanvasImageSource(gfx::Size(image.Width() + 2 * kHorizontalImagePadding,
image.Height() + 2 * kVerticalImagePadding),
false),
image_(image) {}

// gfx::CanvasImageSource:
void Draw(gfx::Canvas* canvas) override {
canvas->DrawImageInt(*image_.ToImageSkia(), kHorizontalImagePadding,
kVerticalImagePadding);
}

private:
gfx::Image image_;

DISALLOW_COPY_AND_ASSIGN(PaddedImageSource);
};

gfx::Image SizeAndCircleIcon(const gfx::Image& icon) {
gfx::Image circled_icon = profiles::GetSizedAvatarIcon(
icon, true, kAvatarIconSize, kAvatarIconSize, profiles::SHAPE_CIRCLE);

gfx::Image padded_icon = gfx::Image(gfx::ImageSkia(
std::make_unique<PaddedImageSource>(circled_icon), 1 /* scale */));
return padded_icon;
return gfx::Image(gfx::CanvasImageSource::CreatePadded(
*circled_icon.ToImageSkia(),
gfx::Insets(kVerticalImagePadding, kHorizontalImagePadding)));
}

} // namespace
Expand Down
42 changes: 36 additions & 6 deletions ui/gfx/image/canvas_image_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,52 @@

#include "base/logging.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/geometry/insets.h"
#include "ui/gfx/image/image_skia.h"

namespace gfx {

namespace {

class PaddedImageSource : public CanvasImageSource {
public:
PaddedImageSource(const ImageSkia& image, const Insets& insets)
: CanvasImageSource(Size(image.width() + insets.width(),
image.height() + insets.height()),
false),
image_(image),
insets_(insets) {}

// CanvasImageSource:
void Draw(Canvas* canvas) override {
canvas->DrawImageInt(image_, insets_.left(), insets_.top());
}

private:
const ImageSkia image_;
const Insets insets_;

DISALLOW_COPY_AND_ASSIGN(PaddedImageSource);
};

} // namespace

////////////////////////////////////////////////////////////////////////////////
// CanvasImageSource

CanvasImageSource::CanvasImageSource(const gfx::Size& size, bool is_opaque)
: size_(size),
is_opaque_(is_opaque) {
// static
ImageSkia CanvasImageSource::CreatePadded(const ImageSkia& image,
const Insets& insets) {
return MakeImageSkia<PaddedImageSource>(image, insets);
}

gfx::ImageSkiaRep CanvasImageSource::GetImageForScale(float scale) {
gfx::Canvas canvas(size_, scale, is_opaque_);
CanvasImageSource::CanvasImageSource(const Size& size, bool is_opaque)
: size_(size), is_opaque_(is_opaque) {}

ImageSkiaRep CanvasImageSource::GetImageForScale(float scale) {
Canvas canvas(size_, scale, is_opaque_);
Draw(&canvas);
return gfx::ImageSkiaRep(canvas.GetBitmap(), scale);
return ImageSkiaRep(canvas.GetBitmap(), scale);
}

} // namespace gfx
32 changes: 18 additions & 14 deletions ui/gfx/image/canvas_image_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,37 +18,41 @@
namespace gfx {
class Canvas;
class ImageSkiaRep;
class Insets;

// CanvasImageSource is useful if you need to generate an image for
// a scale factor using gfx::Canvas. It creates a new Canvas
// with target scale factor and generates ImageSkiaRep when drawing is
// completed.
class GFX_EXPORT CanvasImageSource : public gfx::ImageSkiaSource {
// CanvasImageSource is useful if you need to generate an image for a scale
// factor using Canvas. It creates a new Canvas with target scale factor and
// generates ImageSkiaRep when drawing is completed.
class GFX_EXPORT CanvasImageSource : public ImageSkiaSource {
public:
// Factory function to create an ImageSkia from a CanvasImageSource. Example:
// gfx::ImageSkia my_image =
// ImageSkia my_image =
// CanvasImageSource::MakeImageSkia<MySource>(param1, param2);
template <typename T, typename... Args>
static ImageSkia MakeImageSkia(Args&&... args) {
auto source = std::make_unique<T>(std::forward<Args>(args)...);
gfx::Size size = source->size();
return gfx::ImageSkia(std::move(source), size);
Size size = source->size();
return ImageSkia(std::move(source), size);
}

CanvasImageSource(const gfx::Size& size, bool is_opaque);
// Creates a Image containing |image| with transparent padding around the
// edges as specified by |insets|.
static ImageSkia CreatePadded(const ImageSkia& image, const Insets& insets);

CanvasImageSource(const Size& size, bool is_opaque);
~CanvasImageSource() override {}

// Called when a new image needs to be drawn for a scale factor.
virtual void Draw(gfx::Canvas* canvas) = 0;
virtual void Draw(Canvas* canvas) = 0;

// Returns the size of images in DIP that this source will generate.
const gfx::Size& size() const { return size_; };
const Size& size() const { return size_; };

// Overridden from gfx::ImageSkiaSource.
gfx::ImageSkiaRep GetImageForScale(float scale) override;
// Overridden from ImageSkiaSource.
ImageSkiaRep GetImageForScale(float scale) override;

protected:
const gfx::Size size_;
const Size size_;
const bool is_opaque_;
DISALLOW_COPY_AND_ASSIGN(CanvasImageSource);
};
Expand Down

0 comments on commit 077b476

Please sign in to comment.