Wrong colors with dark variant of gtk3 themes

[Chrysostomus]

Enabling dark theme (gtk-application-prefer-dark-theme=1 in ~/.config/gtk-3.0/settings.ini) causes text to illegible atleast with adwaita theme. The top panels and menus become dark like they should and text becomes light grey to accomodate this. However, since background of working area stays bright white, text cannot be properly seen unless icon is selected. I think spacefm should use dark background if dark theme is selected, or alternatively use different font color for working area.

[Chrysostomus]

Detailed view seems to use right background color, while compact and icons view don't.

[IgnorantGuru]

Confirmed - this seems to have been introduced by GTK3 dev sometime after GTK v3.12.

[OmegaPhil]

Interesting. When I comment out 1609-1612 (exo-icon-view.c:exo_icon_view_realize), the black background properly applies, and it doesn't appear to break normal Adwaita or XFCE themes. According to the documentation, gtk_widget_set_style is not appropriate as it overrules the correct style that should be applied - which is what is happening here.

gdk_window_set_background is completely wrong now - gtk_style_set_background should be used according to the docs, but it looks to be superfluous here. Should I just kill these lines off?

[IgnorantGuru]

Yeah i would say remove them for GTK >= 3.12, or just for all of GTK 3.

Sometimes applications will override styles, but it's frowned upon since the user wants to control it via theme. This is a custom widget though so apparently they tried to adjust it for whatever reason.

[OmegaPhil]

Oddly in GTK2 without this code the widget background is black (and the GTK3 settings file is irrelevant here)... so I have put it in a GTK2-only block, fixed in next.

[IgnorantGuru]

Thanks for looking at this.

[IgnorantGuru]

Related discussion

[OmegaPhil]

J__: Can you remove the #if and #endif lines from src/exo/exo-icon-view.c, around line 1609 and retest? That essentially reverts 37698e2354e24e6634e694a82215698e848d4b08

[ikn]

Yes, that fixes the issue (I used the spacefm-git package in AUR).

[OmegaPhil]

OK, I will queue up a task to fiddle around with this to find another way around the dark themes problem, however since only you can recreate the problem so far, you would need to test prospective code (just cloning git, changing to the branch and compiling, or using recent functionality of the net installer: https://ignorantguru.github.io/spacefm/#installer) - is this OK?

[ikn]

Yeah, I'm happy to test stuff.

[OmegaPhil]

@ikn: Quick test: if you keep the GTK_CHECK_VERSION block, but move the gtk_widget_set_style call above it, do you get correct background behaviour with Adwaita? Preferably also a theme that doesn't have a 'dark mode'.

[OmegaPhil]

Also if that works, does the background break when you go to Detailed view and then back to Compact?

[ikn]

It doesn't work (git diff below to be sure). I don't know how to tell if a theme has a dark mode, but with the HighContrast and Raleigh themes, the background seems to be the same colour as menu items (with Adwaita, it's not the same colour).

diff --git a/src/exo/exo-icon-view.c b/src/exo/exo-icon-view.c
index 793bcb6..55989d8 100644
--- a/src/exo/exo-icon-view.c
+++ b/src/exo/exo-icon-view.c
@@ -1606,10 +1606,10 @@ exo_icon_view_realize (GtkWidget *widget)
     priv->bin_window = gdk_window_new (gtk_widget_get_window(widget), &attributes, attributes_mask);
     gdk_window_set_user_data (priv->bin_window, widget);

+    gtk_widget_set_style (widget, gtk_style_attach (gtk_widget_get_style (widget), gtk_widget_get_window(widget)));
 #if !GTK_CHECK_VERSION (3, 0, 0)
     /* Attach style/background - this breaks 'dark theme version' styles in GTK3
      *  but appears to be needed for GTK2 - https://github.com/IgnorantGuru/spacefm/issues/578 */
-    gtk_widget_set_style (widget, gtk_style_attach (gtk_widget_get_style (widget), gtk_widget_get_window(widget)));
     gdk_window_set_background (priv->bin_window, &gtk_widget_get_style (widget)->base[gtk_widget_get_state (widget)]);
     gdk_window_set_background (gtk_widget_get_window (widget), &gtk_widget_get_style (widget)->base[gtk_widget_get_state (widget)]);
 #endif

[OmegaPhil]

OK, thats annoying. Yeah, sounds like Adwaita is the only 'dark theme capable' theme there. As a last-ditch test, can you confirm that both the remaining gdk_window_set_background calls are needed too?

[ikn]

It seems like at least one of the set_background lines is needed for the correct background colour, but the set_style line makes no difference. That is, it works with either of the set_background lines enabled, but not with neither of them.

[OmegaPhil]

@IgnorantGuru: I noticed that, unlike with Thunar, the view never restyles immediately when you change the XFCE4 'appearance' - exo-tree-view doesn't have a style_set function, but in exo_icon_view_style_set (which is called when this happens) the passed widget is always not realized - sounds like there is something fundamentally wrong with how the view is maintained, do you want to look at this or shall I try to fight it?

Asking as there have been a few realization problems now with this widget.

[IgnorantGuru]

@OmegaPhil I'm not familiar with the style_set, as I've never done much with custom widgets. So I can't tell you anything off-hand. I think the only realization problems I've seen were with that thread race, which doesn't necessarily indicate a problem in that component.

Where I've encountered similar is connecting to the changed signal of the icon theme, for example. I think the theme (style?) change is handled automatically by GTK. But since it's a custom widget you can provide your own handler, or perhaps must.

You could see if you can reach @BwackNinja, as he was knowledgeable with the custom GTK stuff, eg on_style_set in desktop-window.c. Or there may be some docs or examples.

The other question I would have is does that style_set concern only what happens when the theme is changed, or does it affect behavior on initial startup with the current theme? Iow is this really related to this issue? (rhetorical questions)

[IgnorantGuru]

Also, you might verify that get_realized is the appropriate test. Perhaps in GTK3 it sets the style before it's realized, or there's another appropriate test? Just a thought. Lots of changes in GTK3 theming. I would try removing that condition and see if it works.

Seems like one could at least set the style later statically, even if the automatic response to a theme change is a problem. Obviously better to have both though.

[OmegaPhil]

Well, theres still the keypress problem there, and unless your thread senses are tingling, this issue will be the first 'normal' one involving the improperly initialised widget - widgets are supposed to be created (results in calling _initialize), then be in a position that is visible, and then have show called on them (which then triggers _realize), its only when the latter happens that the object is actually usable (source), e.g. when I looked into the GtkTreeView realization problems originally, comparing gtk_tree_view_init with gtk_tree_view_realize.

Through debugging thunar, I can see that GTK picks up the theme change then eventually calls the right gtk_widget_set_style_internal, which then calls the registered style_set function for the GType - yeah this function is called both to style the widget initially and then whenever the style changes (testing now, the ExoTreeView is not realized in either instance).

This is all GTK2 stuff currently, I always start in this since its supposed to be sane.

I'm looking into all this since I have no idea what the ticket problem is or how to fix it, so I'm looking at what is different in something that actually works. Its more over the top, but in Thunar we have a great example - their version is built from:

exo-icon-view.c (libexo-1-0) -> thunar-abstract-icon-view.c -> thunar-icon-view.c

The current upstream exo_icon_view_style_set does this realized check, with the comment '/* apply the new style for the bin_window if we're realized */' - you can see the bin_window that it talks about is only created in gtk_tree_view_realize.

I'll see if I can get @BwackNinja 's attention before I continue working on this.

[IgnorantGuru]

Yeah I'm not familiar with much of that - you've looked into that more than me.

As far as the keypress unrealized event thing, I really doubt it's thread related. It occurs when SpaceFM is sitting idle, and the warning appears before the keypress handler even gets signaled. So there's simply nothing running but the glib main loop thread, and its idle event queue handling the keypress.

But what you're talking about could be the source of it somewhere. I know it occurs in Detailed view but not sure about Icon - I can't reproduce it at the moment

(For those who don't know what this is about, sometimes when SpaceFM first starts and you press a key before clicking anywhere in the window, you get a 'widget not realized for event' warning with each keypress. It vanishes as soon as you click on the list. Researching this it appears other apps have encountered it but no solution. May be a GTK bug. It's always just a stderr warning, never causes a crash and doesn't interfere with keypress processing. So just a nuisance.)

Overall with this bug, I would suggest seeing if you can manually set the style as desired. You could even create a gtktreeview, steal it's style, and destroy it. Once you get that manual style set working you could have it set it somewhere after it's realized (even on a timed delay). That would solve it for initial startup. Then the automatic style update on theme change may be another issue. But you know the details better so maybe that approach isn't good.

Maybe you can just copy Thunar's exo style set functions into our code. That should solve it for GTK2 at least as a starting point, since that's a working example.

And don't hesitate to write to the exo devs and ask them why our outdated hacked fork of exo isn't working in GTK3. :) But seriously they may answer some questions you have.

[BwackNinja]

Hi @OmegaPhil and @IgnorantGuru, you've got my attention. I'll read through this thread and try to see what I can do to make sense of things later today.

[OmegaPhil]

Well, its already being set manually - hence the dodgy calls and not knowing what is needed and why.

The code is copied from lib-exo, this hasn't changed (I didn't see any more magic than this simple set_style function which was annoying, so theres no easy get-out of this).

[OmegaPhil]

If I run out of leads I'll ask the XFCE mailing list - I follow it in general, I've got the impression they're moving away from custom widgets in their GTK3 port.

[OmegaPhil]

To be explicit, this 'style not being set on theme change' issue is in both GTK2 and 3 - I'm picking on it since it should be an example of something that should work in GTK2 if the widget is done right.

[BwackNinja]

The fundamental problem with this widget and it's theming is that it's a custom widget. That means that it either has to get it's colors from being set explicitly from a gtkrc or .css file, or (more usefully) use styles from another widget or inherit them from another class of widget.

What the current code is doing is taking the style from the window, setting that for this widget, and grabbing the base color (which is used for textboxes, views, etc.) and setting that as the background color for the widget.

The way to do this properly in GTK3 is: gtk_style_context_add_class (gtk_widget_get_style_context (GTK_WIDGET (widget)), GTK_STYLE_CLASS_VIEW);

The way to do that in GTK2 is: ...impossible

The only style that ExoIconView inherits is that of GtkContainer, and in turn GtkWidget, it's parent classes. You can add a ExoIconView match to your gtkrc file in your theme to fix this in GTK2, but that's about it - otherwise, what's here is about the best you can do in GTK2. https://wiki.gnome.org/Attic/GnomeArt/Tutorials/GtkThemes

I guess this is one of the examples of how GTK3 fixed themes and made them more sane rather than breaking them.

[OmegaPhil]

OK, but what do you think about the widget's style_set method always being called when the widget hasn't even been realized?

[BwackNinja]

It shouldn't be necessary for GTK3 at all, but for GTK2, the best reference would be GtkIconView itself. It doesn't look too different and has the same check:

https://github.com/GNOME/gtk/blob/gtk-2-24/gtk/gtkiconview.c#L1417

[OmegaPhil]

Thanks Bwack.

Right, I've managed to fix ExoIconView for GTK2 and responding properly to retheming - style_set wasn't actually being called for realized widgets at all, because GTK2 was treating the gtk_widget_set_style call in exo_icon_view_realize as hardcoding the style, and therefore outside of the rc theme system, therefore the widgets being explicitly ignored when firing off set_style calls during the theme change.

In GTK3's case I found that the gdk_window_set_background call in exo_icon_view_style_set was again a case of hardcoding, and GTK3 managed the correct colour by itself.

So now for GTK3 there is no style hacking at all going on. @ikn, I've pushed to next-omega - I bet this is broken for you still? If yes I will then do BwackNinja's change.

[ikn]

Yeah, still broken on that branch.

[OmegaPhil]

Sorry for the delay, just recovering from a large meal.

I have now pushed the Bwack change, at least doesn't break anything here - any good for you?

[ikn]

Unfortunately the background is still grey with 71f4d2.

[OmegaPhil]

Hmm, I'll do a bit more reading tomorrow on the GTK3 style stuff but at this rate I don't think I'll be able to change the code to both work in mine and your situation.

IG: The current changes still need to be included in next as they fix immediate restyling on changing themes.

[ikn]

Fair enough, sometimes there are library issues you just can't work around. Thanks for putting in the effort even if you don't find a solution.

[BwackNinja]

@ikn, are you sure? I reproduced your issue (I had to install and run gnome-settings-daemon and changed the theme back and forth with dconf-editor). With @OmegaPhil's branch, next-omega, I'm no longer able to reproduce it.

[ikn]

These are the steps I took:

git clone https://github.com/IgnorantGuru/spacefm.git
cd spacefm/
git checkout next-omega
./autogen.sh --prefix=/usr --with-gtk3
make
ps -ef | grep spacefm # to check for already-running spacefm, in case that could cause it to just open a new window in that process
./src/spacefm

I've set the widget theme using lxappearance (I dont have gnome-settings-daemon installed). Is there some reason I should be switching the theme when testing this, or are you just saying you switched from your normal theme to a problematic theme?

The usage of ./autogen.sh is taken from the Arch Linux package script; looking in the readme, it seems the recommendation is to use ./configure instead, and this gives me the same result.

If there's nothing wrong with this, what information can I give to help determine the difference between our setups?

[IgnorantGuru]

@OmegaPhil Thanks, you can push your changes to next whenever you're ready - that will give it more testing. Please just make sure you're still GTK 2.18 build compatible, or ifdef it (I didn't check so just a general reminder).

@BwackNinja Thanks for the input!

[BwackNinja]

I'm confused then. You're doing everything right. I could only see the wrong background with the latest-released SpaceFM when changing themes with gtk-application-prefer-dark-theme=1 in ~/.config/gtk-3.0/settings.ini. I needed gnome-settings-daemon so that the theme switches would happen while SpaceFM was open, other equivalents would certainly work. Then, I saw that the background color for the icon view was incorrect with the dark Adwaita. Otherwise, the colors were correct whenever just opening up SpaceFM and changing the view between compact and detailed. I was using lxappearance before to change themes.

Which theme are you using? Adwaita? And what version of GTK? I'm on 3.18.2. Finally, is there anything in your .config/gtk-3.0/gtk.css file that might be overriding parts of the theme?

[ikn]

$ pacman -Qi gtk3
Name           : gtk3
Version        : 3.18.4-1
Description    : GObject-based multi-platform GUI toolkit
Architecture   : x86_64
URL            : http://www.gtk.org/
Licences       : LGPL
Groups         : None
Provides       : None
Depends On     : atk  cairo  libcups  libxcursor  libxinerama  libxrandr  libxi  libepoxy  gdk-pixbuf2
                 libxcomposite  libxdamage  pango  shared-mime-info  colord  at-spi2-atk  wayland
                 libxkbcommon  adwaita-icon-theme  json-glib  rest  librsvg  gtk-update-icon-cache
Optional Deps  : libcanberra: gtk3-widget-factory demo [installed]
Required By    : gcr  girara  gnome-desktop  gnome-font-viewer  gsimplecal  gtk-engine-unico  gtkmm3
                 gtksourceview3  libunique3  libxfce4ui  seahorse  spacefm-git  system-config-printer
                 transmission-gtk  webkit2gtk  webkitgtk  zeitgeist
Optional For   : avahi  ghostscript  libreoffice-still
Conflicts With : None
Replaces       : None
Installed Size :  67.15 MiB
Packager       : Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>
Build Date     : Thu 12 Nov 2015 20:55:50 GMT
Install Date   : Fri 13 Nov 2015 18:09:58 GMT
Install Reason : Installed as a dependency for another package
Install Script : Yes
Validated By   : Signature

$ cat .config/gtk-3.0/settings.ini 
[Settings]
gtk-theme-name=Adwaita
gtk-icon-theme-name=elementary
gtk-font-name=Sans 10
gtk-cursor-theme-name=Vanilla-DMZ-AA
gtk-cursor-theme-size=24
gtk-toolbar-style=GTK_TOOLBAR_BOTH_HORIZ
gtk-toolbar-icon-size=GTK_ICON_SIZE_LARGE_TOOLBAR
gtk-button-images=1
gtk-menu-images=1
gtk-enable-event-sounds=0
gtk-enable-input-feedback-sounds=0
gtk-xft-antialias=1
gtk-xft-hinting=1
gtk-xft-hintstyle=hintfull
gtk-xft-rgba=rgb

I answered some other questions here: https://sourceforge.net/p/spacefm/discussion/general/thread/246e5984/

[OmegaPhil]

For reference since Bwack noticed this issue and then its fix with the current commits, I have merged into next and got rid of my branch. I'm currently unable to run a Debian Lenny VM (kernel stalls during the installation with Virtualbox), but I haven't added any interesting GTK calls with the changes so it should be fine.

[IgnorantGuru]

I'll leave this open to collect any more info on wrong colors appearing, such as ikn's unresolved problem. Overall it appears these issues are fixed for most GTK2/3 installs.

Note that OmegaPhil's first fix for this was released in 1.0.4 (37698e23), and his recent changes are in the next branch (888a7735 88d08d8e 71f4d21d), due for 1.0.5 release.

[ikn]

I just realised that I only see the incorrect behaviour when the tab bar isn't visible. When the tab bar is visible (either there's more than one tab, or the 'always show tab bar' option is enabled), the background is white as expected.

Took me a long time to notice...

[IgnorantGuru]

May have introduced a GTK2 bug #627 with latest changes here released in 1.0.5.