From 74e80ae59473d03bd5a5c610a9c5c4416e944012 Mon Sep 17 00:00:00 2001 From: "erg@chromium.org" <erg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> Date: Thu, 8 Jul 2010 00:10:41 +0000 Subject: [PATCH] GTK: Fix highlight and image colors in the new wrench menu. - The states weren't being set properly to replicate the menu item highlight effect. The button background needs to be STATE_SELECTED while the actual widget needs PRELIGHT. This removes a bunch of crud. - The fullscreen button needs to be tinted multiple times to the different label states. Now the fullscreen icon is the correct label color. Required adding infrastructure for GtkIconSets. BUG=45757 TEST=All labels and images in buttons should match the normal and prelight states of the labels in normal menu items. Make sure to try themes that have different normal and prelight states (DarkLooks) and themes that have the same color (Ambiance). Review URL: http://codereview.chromium.org/2864044 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51805 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/gtk/browser_toolbar_gtk.cc | 4 + chrome/browser/gtk/browser_toolbar_gtk.h | 1 + chrome/browser/gtk/gtk_custom_menu_item.cc | 39 +++------- chrome/browser/gtk/gtk_theme_provider.cc | 86 ++++++++++++++++++++-- chrome/browser/gtk/gtk_theme_provider.h | 25 +++++++ chrome/browser/gtk/menu_gtk.cc | 42 ++++++++--- chrome/browser/gtk/menu_gtk.h | 3 + 7 files changed, 155 insertions(+), 45 deletions(-) diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc index e195ee1d2c009..a7ac3237e40e2 100644 --- a/chrome/browser/gtk/browser_toolbar_gtk.cc +++ b/chrome/browser/gtk/browser_toolbar_gtk.cc @@ -351,6 +351,10 @@ void BrowserToolbarGtk::StoppedShowing() { GTK_CHROME_BUTTON(app_menu_button_.get())); } +GtkIconSet* BrowserToolbarGtk::GetIconSetForId(int idr) { + return theme_provider_->GetIconSetForId(idr); +} + // menus::SimpleMenuModel::Delegate bool BrowserToolbarGtk::IsCommandIdEnabled(int id) const { diff --git a/chrome/browser/gtk/browser_toolbar_gtk.h b/chrome/browser/gtk/browser_toolbar_gtk.h index 48d984ad44262..f58b582e1ac69 100644 --- a/chrome/browser/gtk/browser_toolbar_gtk.h +++ b/chrome/browser/gtk/browser_toolbar_gtk.h @@ -93,6 +93,7 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver, // Overridden from MenuGtk::Delegate: virtual void StoppedShowing(); + virtual GtkIconSet* GetIconSetForId(int idr); // Overridden from menus::SimpleMenuModel::Delegate: virtual bool IsCommandIdEnabled(int id) const; diff --git a/chrome/browser/gtk/gtk_custom_menu_item.cc b/chrome/browser/gtk/gtk_custom_menu_item.cc index 8c9c923b63117..febf8bc5b592a 100644 --- a/chrome/browser/gtk/gtk_custom_menu_item.cc +++ b/chrome/browser/gtk/gtk_custom_menu_item.cc @@ -19,12 +19,20 @@ G_DEFINE_TYPE(GtkCustomMenuItem, gtk_custom_menu_item, GTK_TYPE_MENU_ITEM) static void set_selected(GtkCustomMenuItem* item, GtkWidget* selected) { if (selected != item->currently_selected_button) { - if (item->currently_selected_button) + if (item->currently_selected_button) { gtk_widget_set_state(item->currently_selected_button, GTK_STATE_NORMAL); + gtk_widget_set_state( + gtk_bin_get_child(GTK_BIN(item->currently_selected_button)), + GTK_STATE_NORMAL); + } item->currently_selected_button = selected; - if (item->currently_selected_button) + if (item->currently_selected_button) { gtk_widget_set_state(item->currently_selected_button, GTK_STATE_SELECTED); + gtk_widget_set_state( + gtk_bin_get_child(GTK_BIN(item->currently_selected_button)), + GTK_STATE_PRELIGHT); + } } } @@ -49,27 +57,6 @@ static void gtk_custom_menu_item_select(GtkItem *item); static void gtk_custom_menu_item_deselect(GtkItem *item); static void gtk_custom_menu_item_activate(GtkMenuItem* menu_item); -static void gtk_custom_menu_item_style_set(GtkCustomMenuItem* item, - GtkStyle* old_style) { - // Because several popular themes have no idea about styling buttons in menus - // (it's sort of a weird concept) and look like crap, we manually apply the - // menu item's prelight information to the button. - GtkStyle* style = gtk_widget_get_style(GTK_WIDGET(item)); - - for (GList* i = item->button_widgets; i; i = g_list_next(i)) { - // Set the button prelight colors. - GtkWidget* button = GTK_WIDGET(i->data); - gtk_widget_modify_fg(button, GTK_STATE_PRELIGHT, - &style->fg[GTK_STATE_PRELIGHT]); - gtk_widget_modify_bg(button, GTK_STATE_PRELIGHT, - &style->bg[GTK_STATE_PRELIGHT]); - gtk_widget_modify_text(button, GTK_STATE_PRELIGHT, - &style->text[GTK_STATE_PRELIGHT]); - gtk_widget_modify_base(button, GTK_STATE_PRELIGHT, - &style->base[GTK_STATE_PRELIGHT]); - } -} - static void gtk_custom_menu_item_init(GtkCustomMenuItem* item) { item->all_widgets = NULL; item->button_widgets = NULL; @@ -86,9 +73,6 @@ static void gtk_custom_menu_item_init(GtkCustomMenuItem* item) { item->hbox = gtk_hbox_new(FALSE, 0); gtk_box_pack_end(GTK_BOX(menu_hbox), item->hbox, FALSE, FALSE, 0); - g_signal_connect(item, "style-set", - G_CALLBACK(gtk_custom_menu_item_style_set), NULL); - g_signal_connect(item->hbox, "expose-event", G_CALLBACK(gtk_custom_menu_item_hbox_expose), item); @@ -309,7 +293,8 @@ GtkWidget* gtk_custom_menu_item_add_button_label(GtkCustomMenuItem* menu_item, g_object_set_data(G_OBJECT(button), "command-id", GINT_TO_POINTER(command_id)); gtk_box_pack_start(GTK_BOX(menu_item->hbox), button, FALSE, FALSE, 0); - g_signal_connect(button, "notify", G_CALLBACK(on_button_label_set), NULL); + g_signal_connect(button, "notify::label", + G_CALLBACK(on_button_label_set), NULL); gtk_widget_show(button); menu_item->all_widgets = g_list_append(menu_item->all_widgets, button); diff --git a/chrome/browser/gtk/gtk_theme_provider.cc b/chrome/browser/gtk/gtk_theme_provider.cc index 1aefdab1c2b2d..79d45cdb84b5c 100644 --- a/chrome/browser/gtk/gtk_theme_provider.cc +++ b/chrome/browser/gtk/gtk_theme_provider.cc @@ -201,9 +201,11 @@ GtkThemeProvider* GtkThemeProvider::GetFrom(Profile* profile) { GtkThemeProvider::GtkThemeProvider() : BrowserThemeProvider(), fake_window_(gtk_window_new(GTK_WINDOW_TOPLEVEL)), - fake_frame_(meta_frames_new()) { + fake_frame_(meta_frames_new()), + fullscreen_icon_set_(NULL) { fake_label_.Own(gtk_label_new("")); fake_entry_.Own(gtk_entry_new()); + fake_menu_item_.Own(gtk_menu_item_new()); // Only realized widgets receive style-set notifications, which we need to // broadcast new theme images and colors. Only realized widgets have style @@ -220,6 +222,9 @@ GtkThemeProvider::~GtkThemeProvider() { gtk_widget_destroy(fake_frame_); fake_label_.Destroy(); fake_entry_.Destroy(); + fake_menu_item_.Destroy(); + + FreeIconSets(); // We have to call this because FreePlatformCached() in ~BrowserThemeProvider // doesn't call the right virutal FreePlatformCaches. @@ -234,12 +239,12 @@ void GtkThemeProvider::Init(Profile* profile) { } SkBitmap* GtkThemeProvider::GetBitmapNamed(int id) const { - if (use_gtk_ && IsOverridableImage(id)) { - // Try to get our cached version: - ImageCache::const_iterator it = gtk_images_.find(id); - if (it != gtk_images_.end()) - return it->second; + // Try to get our cached version: + ImageCache::const_iterator it = gtk_images_.find(id); + if (it != gtk_images_.end()) + return it->second; + if (use_gtk_ && IsOverridableImage(id)) { // We haven't built this image yet: SkBitmap* bitmap = GenerateGtkThemeBitmap(id); gtk_images_[id] = bitmap; @@ -357,6 +362,13 @@ GdkColor GtkThemeProvider::GetBorderColor() const { return color; } +GtkIconSet* GtkThemeProvider::GetIconSetForId(int id) const { + if (id == IDR_FULLSCREEN_MENU_BUTTON) + return fullscreen_icon_set_; + + return NULL; +} + void GtkThemeProvider::GetScrollbarColors(GdkColor* thumb_active_color, GdkColor* thumb_inactive_color, GdkColor* track_color) { @@ -535,6 +547,8 @@ void GtkThemeProvider::LoadThemePrefs() { LoadDefaultValues(); BrowserThemeProvider::LoadThemePrefs(); } + + RebuildMenuIconSets(); } void GtkThemeProvider::NotifyThemeChanged(Extension* extension) { @@ -568,6 +582,8 @@ void GtkThemeProvider::OnStyleSet(GtkWidget* widget, NotifyThemeChanged(NULL); } + RebuildMenuIconSets(); + // Free the old icons only after the theme change notification has gone // through. if (default_folder_icon) @@ -761,6 +777,57 @@ void GtkThemeProvider::LoadDefaultValues() { inactive_selection_fg_color_ = SkColorSetRGB(50, 50, 50); } +void GtkThemeProvider::RebuildMenuIconSets() { + FreeIconSets(); + + GtkStyle* style = gtk_rc_get_style(fake_menu_item_.get()); + + fullscreen_icon_set_ = gtk_icon_set_new(); + BuildIconFromIDRWithColor(IDR_FULLSCREEN_MENU_BUTTON, + style, + GTK_STATE_PRELIGHT, + fullscreen_icon_set_); + BuildIconFromIDRWithColor(IDR_FULLSCREEN_MENU_BUTTON, + style, + GTK_STATE_NORMAL, + fullscreen_icon_set_); +} + +void GtkThemeProvider::BuildIconFromIDRWithColor(int id, + GtkStyle* style, + GtkStateType state, + GtkIconSet* icon_set) { + SkColor color = GdkToSkColor(&style->fg[state]); + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); + SkBitmap original = *rb.GetBitmapNamed(id); + + SkBitmap fill_color; + fill_color.setConfig(SkBitmap::kARGB_8888_Config, + original.width(), original.height(), 0); + fill_color.allocPixels(); + fill_color.eraseColor(color); + SkBitmap masked = SkBitmapOperations::CreateMaskedBitmap( + fill_color, original); + + GtkIconSource* icon = gtk_icon_source_new(); + GdkPixbuf* pixbuf = gfx::GdkPixbufFromSkBitmap(&masked); + gtk_icon_source_set_pixbuf(icon, pixbuf); + g_object_unref(pixbuf); + + gtk_icon_source_set_direction_wildcarded(icon, TRUE); + gtk_icon_source_set_size_wildcarded(icon, TRUE); + + gtk_icon_source_set_state(icon, state); + // All fields default to wildcarding being on and setting a property doesn't + // turn off wildcarding. You need to do this yourself. This is stated once in + // the documentation in the gtk_icon_source_new() function, and no where else. + gtk_icon_source_set_state_wildcarded( + icon, state == GTK_STATE_NORMAL); + + gtk_icon_set_add_source(icon_set, icon); + gtk_icon_source_free(icon); +} + void GtkThemeProvider::SetThemeColorFromGtk(int id, const GdkColor* color) { colors_[id] = GdkToSkColor(color); } @@ -802,6 +869,13 @@ void GtkThemeProvider::FreePerDisplaySurfaces( per_display_map->clear(); } +void GtkThemeProvider::FreeIconSets() { + if (fullscreen_icon_set_) { + gtk_icon_set_unref(fullscreen_icon_set_); + fullscreen_icon_set_ = NULL; + } +} + SkBitmap* GtkThemeProvider::GenerateGtkThemeBitmap(int id) const { switch (id) { case IDR_THEME_TOOLBAR: { diff --git a/chrome/browser/gtk/gtk_theme_provider.h b/chrome/browser/gtk/gtk_theme_provider.h index e6321e9656613..edc5ce7b42987 100644 --- a/chrome/browser/gtk/gtk_theme_provider.h +++ b/chrome/browser/gtk/gtk_theme_provider.h @@ -70,6 +70,10 @@ class GtkThemeProvider : public BrowserThemeProvider, // label. Used for borders between GTK stuff and the webcontent. GdkColor GetBorderColor() const; + // Returns a set of icons tinted for different GtkStateTypes based on the + // label colors for the IDR resource |id|. + GtkIconSet* GetIconSetForId(int id) const; + // This method returns averages of the thumb part and of the track colors. // Used when rendering scrollbars. static void GetScrollbarColors(GdkColor* thumb_active_color, @@ -144,6 +148,18 @@ class GtkThemeProvider : public BrowserThemeProvider, // Sets the values that we send to webkit to safe defaults. void LoadDefaultValues(); + // Builds all of the tinted menus images needed for custom buttons. This is + // always called on style-set even if we aren't using the gtk-theme because + // the menus are always rendered with gtk colors. + void RebuildMenuIconSets(); + + // Builds and tints the image with |id| to the GtkStateType |state| and + // places the result in |icon_set|. + void BuildIconFromIDRWithColor(int id, + GtkStyle* style, + GtkStateType state, + GtkIconSet* icon_set); + // Sets the underlying theme colors/tints from a GTK color. void SetThemeColorFromGtk(int id, const GdkColor* color); void SetThemeTintFromGtk(int id, const GdkColor* color); @@ -156,6 +172,9 @@ class GtkThemeProvider : public BrowserThemeProvider, // points to GtkThemeProvider's version. void FreePerDisplaySurfaces(PerDisplaySurfaceMap* per_display_map); + // Frees all the created GtkIconSets we use for the chrome menu. + void FreeIconSets(); + // Lazily generates each bitmap used in the gtk theme. SkBitmap* GenerateGtkThemeBitmap(int id) const; @@ -199,6 +218,7 @@ class GtkThemeProvider : public BrowserThemeProvider, GtkWidget* fake_frame_; OwnedWidgetGtk fake_label_; OwnedWidgetGtk fake_entry_; + OwnedWidgetGtk fake_menu_item_; // A list of all GtkChromeButton instances. We hold on to these to notify // them of theme changes. @@ -228,6 +248,11 @@ class GtkThemeProvider : public BrowserThemeProvider, SkColor inactive_selection_bg_color_; SkColor inactive_selection_fg_color_; + // A GtkIconSet that has the tinted icons for the NORMAL and PRELIGHT states + // of the IDR_FULLSCREEN_MENU_BUTTON tinted to the respective menu item label + // colors. + GtkIconSet* fullscreen_icon_set_; + // Image cache of lazily created images, created when requested by // GetBitmapNamed(). mutable ImageCache gtk_images_; diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc index e8c856b358375..179b8d6375f4f 100644 --- a/chrome/browser/gtk/menu_gtk.cc +++ b/chrome/browser/gtk/menu_gtk.cc @@ -92,7 +92,7 @@ void SetupDynamicLabelMenuButton(GtkWidget* button, int index) { if (model->IsLabelDynamicAt(index)) { g_object_set_data(G_OBJECT(button), "button-model", - reinterpret_cast<void*>(model)); + model); g_object_set_data(G_OBJECT(button), "button-model-id", GINT_TO_POINTER(index)); g_signal_connect(menu, "show", G_CALLBACK(OnSubmenuShowButtonMenuItem), @@ -100,6 +100,32 @@ void SetupDynamicLabelMenuButton(GtkWidget* button, } } +void OnSubmenuShowButtonImage(GtkWidget* widget, GtkButton* button) { + MenuGtk::Delegate* delegate = reinterpret_cast<MenuGtk::Delegate*>( + g_object_get_data(G_OBJECT(button), "menu-gtk-delegate")); + int icon_idr = GPOINTER_TO_INT(g_object_get_data( + G_OBJECT(button), "button-image-idr")); + + GtkIconSet* icon_set = delegate->GetIconSetForId(icon_idr); + if (icon_set) { + gtk_button_set_image( + button, gtk_image_new_from_icon_set(icon_set, + GTK_ICON_SIZE_MENU)); + } +} + +void SetupImageIcon(GtkWidget* button, + GtkWidget* menu, + int icon_idr, + MenuGtk::Delegate* menu_gtk_delegate) { + g_object_set_data(G_OBJECT(button), "button-image-idr", + GINT_TO_POINTER(icon_idr)); + g_object_set_data(G_OBJECT(button), "menu-gtk-delegate", + menu_gtk_delegate); + + g_signal_connect(menu, "show", G_CALLBACK(OnSubmenuShowButtonImage), button); +} + // Popup menus may get squished if they open up too close to the bottom of the // screen. This function takes the size of the screen, the size of the menu, // an optional widget, the Y position of the mouse click, and adjusts the popup @@ -329,8 +355,7 @@ void MenuGtk::BuildSubmenuFromModel(menus::MenuModel* model, GtkWidget* menu) { GTK_ACCEL_VISIBLE); } - g_object_set_data(G_OBJECT(menu_item), "model", - reinterpret_cast<void*>(model)); + g_object_set_data(G_OBJECT(menu_item), "model", model); AppendMenuItemToMenu(i, menu_item, menu, connect_to_activate); if (model->IsLabelDynamicAt(i)) { @@ -348,8 +373,7 @@ GtkWidget* MenuGtk::BuildButtomMenuItem(menus::ButtonMenuItemModel* model, RemoveWindowsStyleAccelerators(UTF16ToUTF8(model->label())).c_str()); // Set up the callback to the model for when it is clicked. - g_object_set_data(G_OBJECT(menu_item), "button-model", - reinterpret_cast<void*>(model)); + g_object_set_data(G_OBJECT(menu_item), "button-model", model); g_signal_connect(menu_item, "button-pushed", G_CALLBACK(OnMenuButtonPressed), this); @@ -366,13 +390,7 @@ GtkWidget* MenuGtk::BuildButtomMenuItem(menus::ButtonMenuItemModel* model, int icon_idr; if (model->GetIconAt(i, &icon_idr)) { - // TODO(erg): This should go through the GtkThemeProvider so we can - // get a version tinted to label color. - gtk_button_set_image( - GTK_BUTTON(button), - gtk_image_new_from_pixbuf( - ResourceBundle::GetSharedInstance(). - GetPixbufNamed(icon_idr))); + SetupImageIcon(button, menu, icon_idr, delegate_); } else { gtk_button_set_label( GTK_BUTTON(button), diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h index 721b736da8563..55767dae88f6f 100644 --- a/chrome/browser/gtk/menu_gtk.h +++ b/chrome/browser/gtk/menu_gtk.h @@ -42,6 +42,9 @@ class MenuGtk { // Return true if we should override the "gtk-menu-images" system setting // when showing image menu items for this menu. virtual bool AlwaysShowImages() const { return false; } + + // Returns a tinted image used in button in a menu. + virtual GtkIconSet* GetIconSetForId(int idr) { return NULL; } }; MenuGtk(MenuGtk::Delegate* delegate, menus::MenuModel* model); -- GitLab