From dfa1cb7ca5aa62df1ab38e06d2130b07c9317a47 Mon Sep 17 00:00:00 2001 From: Ken Sanislo Date: Mon, 11 May 2026 00:48:22 -0700 Subject: [PATCH] PerKey icons: read theme fg from style-updated, not Settings notify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the editor and palette listened to Gtk.Settings notify::gtk-theme-name (and notify::gtk-application-prefer-dark-theme) to re-render their themed icons on theme switch. Two problems: 1. The initial icon load happened during widget construction, before the buttons were attached to the toolbar — so the style context resolved to a default (often white) foreground rather than the actual theme text color. Icons showed up white until the first theme-change event. 2. Settings notify fires *before* GTK's CSS engine re-resolves styles for the new theme. Reading the style context's foreground from that handler returned the previous theme's color, so toggling light <-> dark left both states settling on the same shade. Move both responsibilities into a new attach_themed_icon helper in _icons.py: it does the initial load, connects to the *button's own* style-updated signal, and rebuilds the icon on each emission. That signal fires *after* CSS resolution (both on first realize and on runtime theme switches), so the foreground we read is always the current one. A per-button color-key guard skips the rebuild when the resolved foreground hasn't changed, so unrelated style-updated emissions (hover, focus, active) don't trigger needless re-renders. The handler is connected to the button itself, so GTK cleans it up when the button is destroyed; both editor.py and palette.py drop their bespoke Gtk.Settings handler bookkeeping. --- lib/solaar/ui/perkey/_icons.py | 45 +++++++++++++++++++++++++++++++++ lib/solaar/ui/perkey/editor.py | 36 ++------------------------ lib/solaar/ui/perkey/palette.py | 45 +++++++-------------------------- 3 files changed, 56 insertions(+), 70 deletions(-) diff --git a/lib/solaar/ui/perkey/_icons.py b/lib/solaar/ui/perkey/_icons.py index d5bd3f47..550b15a4 100644 --- a/lib/solaar/ui/perkey/_icons.py +++ b/lib/solaar/ui/perkey/_icons.py @@ -86,3 +86,48 @@ def themed_icon_image(icon_name: str, style_widget: Gtk.Widget) -> Gtk.Image | N except Exception as e: logger.debug("recolor failed for %s: %s", icon_name, e) return None + + +def _fg_color_key(widget: Gtk.Widget) -> tuple[float, float, float]: + fg = widget.get_style_context().get_color(Gtk.StateFlags.NORMAL) + return (round(fg.red, 3), round(fg.green, 3), round(fg.blue, 3)) + + +def attach_themed_icon(button: Gtk.Container, icon_name: str) -> int | None: + """Add a themed icon to `button` and re-render it whenever the active + GTK theme changes the button's foreground color. Returns the + style-updated signal handler ID, or None if the icon couldn't be + loaded (in which case the button is left unchanged so the caller can + fall back to a text label). + + Listening to the button's own ``style-updated`` signal — instead of + ``Gtk.Settings notify::gtk-theme-name`` — means we read the + foreground color *after* GTK has re-resolved CSS for the new theme. + Subscribing to the Settings notify fires too early; it returns the + stale (pre-switch) color and produces icons that all settle on the + previous theme's tone. We guard the rebuild with a per-button color + key so unrelated style updates (hover, focus, active) don't trigger + needless re-renders. + """ + image = themed_icon_image(icon_name, button) + if image is None: + return None + button.add(image) + image.show() + state = {"color_key": _fg_color_key(button)} + + def _refresh(_widget) -> None: + new_key = _fg_color_key(button) + if new_key == state["color_key"]: + return + state["color_key"] = new_key + new_image = themed_icon_image(icon_name, button) + if new_image is None: + return + old = button.get_child() + if old is not None: + button.remove(old) + button.add(new_image) + new_image.show() + + return button.connect("style-updated", _refresh) diff --git a/lib/solaar/ui/perkey/editor.py b/lib/solaar/ui/perkey/editor.py index 7b6b1851..2de7276d 100644 --- a/lib/solaar/ui/perkey/editor.py +++ b/lib/solaar/ui/perkey/editor.py @@ -34,7 +34,7 @@ from gi.repository import Gtk # NOQA: E402 from solaar.i18n import _ # NOQA: E402 from . import binding # NOQA: E402 -from ._icons import themed_icon_image # NOQA: E402 +from ._icons import attach_themed_icon # NOQA: E402 from .canvas import KeyboardCanvas # NOQA: E402 from .layout import Layout # NOQA: E402 from .palette import GradientSwatch # NOQA: E402 @@ -75,9 +75,6 @@ class PerKeyEditor(Gtk.Box): # toolbar row toolbar = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=8) self._tool_buttons: dict[str, Gtk.RadioButton] = {} - # Track which buttons display a themed icon, so we can re-render them - # when the active GTK theme switches (light <-> dark, theme name). - self._themed_icon_buttons: dict[Gtk.RadioButton, str] = {} self._gradient_swatch: GradientSwatch | None = None first: Gtk.RadioButton | None = None supported = layout.supported_tools if layout else ("single", "rect", "bucket", "gradient") @@ -93,12 +90,9 @@ class PerKeyEditor(Gtk.Box): icon_name = _TOOL_ICON_NAMES.get(name) btn = Gtk.RadioButton.new_from_widget(first) btn.set_mode(False) # render as toggle button rather than radio - image = themed_icon_image(icon_name, btn) if icon_name else None - if image is not None: - btn.add(image) + if icon_name and attach_themed_icon(btn, icon_name) is not None: btn.set_tooltip_text(tip or label) btn.get_accessible().set_name(label) - self._themed_icon_buttons[btn] = icon_name else: btn.set_label(label) btn.set_tooltip_text(tip) @@ -108,14 +102,6 @@ class PerKeyEditor(Gtk.Box): toolbar.pack_start(btn, False, False, 0) self._tool_buttons[name] = btn - # Re-render themed icons when the GTK theme changes at runtime. - self._theme_signal_handlers: list[tuple[object, int]] = [] - if self._themed_icon_buttons: - settings = Gtk.Settings.get_default() - for prop in ("notify::gtk-theme-name", "notify::gtk-application-prefer-dark-theme"): - hid = settings.connect(prop, self._on_gtk_theme_changed) - self._theme_signal_handlers.append((settings, hid)) - initial_active, initial_previous = 0xFF0000, 0xFF0000 try: persisted = sink.palette_state() @@ -165,29 +151,11 @@ class PerKeyEditor(Gtk.Box): except Exception as e: logger.debug("perkey sink unsubscribe failed: %s", e) self._unsubscribe = None - for obj, hid in self._theme_signal_handlers: - try: - obj.disconnect(hid) - except Exception as e: - logger.debug("theme signal disconnect failed: %s", e) - self._theme_signal_handlers = [] try: self._palette.shutdown() except Exception as e: logger.debug("palette shutdown failed: %s", e) - def _on_gtk_theme_changed(self, _settings, _pspec) -> None: - """Rebuild themed tool icons so they match the new theme's foreground.""" - for btn, icon_name in self._themed_icon_buttons.items(): - old = btn.get_child() - new_image = themed_icon_image(icon_name, btn) - if new_image is None: - continue - if old is not None: - btn.remove(old) - btn.add(new_image) - new_image.show() - def canvas_size(self) -> tuple[int, int]: """Return the canvas's pixel size_request — what the dialog should size its content area to so the layout fits without scrollbars. diff --git a/lib/solaar/ui/perkey/palette.py b/lib/solaar/ui/perkey/palette.py index 3fc76f51..b9f6ee1e 100644 --- a/lib/solaar/ui/perkey/palette.py +++ b/lib/solaar/ui/perkey/palette.py @@ -24,8 +24,6 @@ itself — see `GradientSwatch` below, used by `editor.py`. from __future__ import annotations -import logging - from enum import Enum import gi @@ -37,9 +35,7 @@ from gi.repository import Gtk # NOQA: E402 from solaar.i18n import _ # NOQA: E402 -from ._icons import themed_icon_image # NOQA: E402 - -logger = logging.getLogger(__name__) +from ._icons import attach_themed_icon # NOQA: E402 _UNSET_ICON_NAME = "solaar-tool-palette-off-symbolic" @@ -97,42 +93,19 @@ class Palette(Gtk.Box): self._unset_btn = Gtk.ToggleButton() self._unset_btn.set_tooltip_text(_("Paint as 'no change' — clears the cell to the zone base color")) - self._unset_label = _("Unset") - self._unset_image = themed_icon_image(_UNSET_ICON_NAME, self._unset_btn) - if self._unset_image is not None: - self._unset_btn.add(self._unset_image) - self._unset_btn.get_accessible().set_name(self._unset_label) + unset_label = _("Unset") + if attach_themed_icon(self._unset_btn, _UNSET_ICON_NAME) is not None: + self._unset_btn.get_accessible().set_name(unset_label) else: - self._unset_btn.set_label(self._unset_label) + self._unset_btn.set_label(unset_label) self._unset_btn.connect(GtkSignal.TOGGLED.value, self._on_unset_toggled) self.pack_start(self._unset_btn, False, False, 0) - # Track theme changes so the unset icon re-renders to match. - self._theme_signal_handlers: list[tuple[object, int]] = [] - if self._unset_image is not None: - settings = Gtk.Settings.get_default() - for prop in ("notify::gtk-theme-name", "notify::gtk-application-prefer-dark-theme"): - hid = settings.connect(prop, self._on_gtk_theme_changed) - self._theme_signal_handlers.append((settings, hid)) - def shutdown(self) -> None: - for obj, hid in self._theme_signal_handlers: - try: - obj.disconnect(hid) - except Exception as e: - logger.debug("palette theme signal disconnect failed: %s", e) - self._theme_signal_handlers = [] - - def _on_gtk_theme_changed(self, _settings, _pspec) -> None: - new_image = themed_icon_image(_UNSET_ICON_NAME, self._unset_btn) - if new_image is None: - return - old = self._unset_btn.get_child() - if old is not None: - self._unset_btn.remove(old) - self._unset_btn.add(new_image) - new_image.show() - self._unset_image = new_image + # attach_themed_icon connects to the button's own style-updated + # signal; GTK disconnects it automatically when the button is + # destroyed, so there is nothing to clean up here. + pass def _on_color_set(self, btn: Gtk.ColorButton) -> None: c = _rgb_to_int(btn.get_rgba())