Draw action glyphs in the theme foreground colour (and recolour live)

[SubliminalsTV]

Reworked per your note on the original #778 split. Action glyphs now draw with the theme's foreground colour from ColorInformation (#782) instead of a hard-coded colour, and recolour live when the theme changes.

Includes a fix to how #782's colours are consumed: refreshing on foregroundChanged rather than isDarkThemeChanged (the dark flag flips before the colour bindings settle). Details in a comment below.

[WhiteMagic]

While this fixes the issue, it does so in a way that breaks the second the theme itself changes. The core issue is that the Python code has no idea of the foreground/background colors used by the UI. The original code used a value that worked but then I forgot to do the step that is required to make it work reliably, i.e. expose the required information to Python.

I'll try to put together the code that provides the required information to python such that this can be fixed in a way that holds up should the main theme colors change. Getting data once on startup is easy, the tricky bit atm is to ensure it's maintained properly since one of your next changes is going to remove to need to reload for light/dark switch, thus this code needs to deal with that too.

[WhiteMagic]

You should be able to use

JoystickGremlin/gremlin/ui/util.py

Line 458 in 5ab8493

class ColorInformation(metaclass=SingletonMetaclass):

to access the foreground and background colors of the theme and use those to draw the icon strips.

[SubliminalsTV]

Thanks for #782 — reworked this to use ColorInformation().foreground as you suggested.

Testing it live surfaced the maintenance issue you predicted. Refreshing the cached colours on isDarkThemeChanged reads them too early: the dark flag flips before the colour bindings settle, so you capture the outgoing theme's colours (and a fresh launch straight into dark mode was unreliable for the same reason). Switching the refresh to foregroundChanged fixes it — that fires once the new colour is actually in place.

For the live switch itself: after refreshing the colours and clearing the cached glyph images, I emit a small themeColorsChanged signal so the action-summary images re-request and repaint without needing the input list to rebuild. That's the "deal with the live switch" part from your note.

Tab-label legibility from the same dark-mode pass is a separate small PR.

[WhiteMagic]

This is an unnecessary indirection that makes it actually harder to understand what the intent is. Just using ColorInformation().foreground in the _render function is good enough. Also the description is a bit lengthy in the sense that it describes the "origin" for the existence. That's fine for a MR etc. but not very useful for a function's docstring (in many cases at least).

[WhiteMagic]

If we only clear the cache in one place, just adding the code there is sufficient and not need to add a function that does exactly what the name is and the function calls are. Similar comment for the docstring as the _glyph_color method.

[WhiteMagic]

As there is no guarantee that the order stays the same and is thus brittle it would be better to trigger on any of the color information to change and coalesce the almost instantaneous signals to be handled only once.

Claude thinks this should work (and claims it tried the code but you know ... :-) )

from PySide6.QtCore import QTimer

_single_shot_timer = QTimer()
_single_shot_timer.setSingleShot(True
_single_shot_timer.setInterval(0)
_single_shot_timer.timeout.connect(some_function)
for sig in (probe.foregroundChanged, probe.backgroundChanged, probe.accentChanged):
    sig.connect(_single_shot_timer.start)

The reason this should work is that setInterval(0) would trigger once all events in the queue right now are done, i.e. it fires once all things queued up have been processed, which means all colors should be up to date and we're not relying on any ordering from Qt code.

[WhiteMagic]

What is that change fixing? The engine will take ownership of the provider, which should prevent Python's GC from deleting the object and so far this hasn't caused problems.

Sets the provider to use for images requested via the image: url scheme, with host providerId. The QQmlEngine takes ownership of provider.

See: https://doc.qt.io/qt-6/qqmlengine.html#addImageProvider

[WhiteMagic]

Why is this needed when the cache of the image provider is cleared? After clearing the cache the image ought to be regenerated. If that's not the case then something else is wrong and all this does is mask that.

[SubliminalsTV]

Adopted the QTimer approach — a zero-interval single-shot restarted on foregroundChanged/backgroundChanged/accentChanged, so the refresh runs once after the event queue drains. That actually fixed a real bug I'd been chasing: triggering on foregroundChanged alone fired asymmetrically (recoloured fine going to dark, but intermittently left glyphs stale going back to light). Coalescing on all three signals and deferring is deterministic in both directions — thanks for the steer.

Also from the review:

• Dropped _glyph_color — it's ColorInformation().foreground inline in _render now.
• Removed the cache-invalidation method. The action-summary image id carries the theme token, so a theme change misses the cache naturally and re-renders; nothing to invalidate.
• Reverted the provider back to a local now that nothing else holds a reference.
• Trimmed the docstrings.

[WhiteMagic]

If this works then not because it should. That signal would be the one from the signal side which this connection is not making connections with. So This should be at least a Connections against that but it also would increment the revision repeatedly, once for however many InputButtons happen to kind of exist which isn't desired. Also, as mentioned above this shouldn't be needed as it tries to solve something the cache clearing already should take care of.

[WhiteMagic]

As mentioned down below this should not be required to be able to force a refresh.

[WhiteMagic]

If we add a new signal instead of reusing something like reloadUi which may be the better on in this case. Then this should also drive everything. That means that the image provider connects the cache clearing to this. Qt has the signal/slot mechanism to avoid a lot of annoying calling of methods to get something done in response to something happening.

[WhiteMagic]

Overall the changes make sense just what may look like a lot of changes based around of how Qt does things (or allows you to do) and then follow on things of what "should" happen but may not which in my view would point to something being broken that needs fixing rather than adding a workaround.

[SubliminalsTV]

A couple of these are on an earlier revision of the diff, so quick status first: the cache clear is already gone — I dropped invalidate_cache per your earlier note — so there's no "cache clear + redundant token" anymore; the revision is the only mechanism now.

On why clearing the provider cache isn't enough by itself: it doesn't make the on-screen Image elements re-fetch. A QQuickImageProvider image is only re-requested when the Image's source changes or its delegate is rebuilt — there's no signal telling live Images "the provider's data changed, reload." So after an in-place theme switch the visible glyphs keep their loaded pixmap regardless of the cache. That matched testing exactly: switching device tabs repainted them (delegates rebuild → re-request), but toggling the theme in place did not, until the source URL changed. Bumping a token in the URL is the usual Qt idiom for forcing a provider image to reload.

On the per-button increment — agreed, not good. I've moved it to a single shared counter (signal.themeRevision, an int Property bumped once per theme change) and dropped the per-InputButton handler; each image just binds its source to that one value.

If there's a provider-reload path you'd prefer that avoids the URL token, I'm glad to switch — I just couldn't find one that re-fetches without a source change.

[WhiteMagic]

I'd go with something like this. Worth explaining what the behavior overall is but I usually don't explain what API calls do as that gets very verbose very quickly.

"Coalesce the color change signals into a single update call that triggers only once the current event queue is empty."

[WhiteMagic]

I looked around a bit and tried some things, there is the option of using QQuickWindow.releaseResources() which seems to drop literally any cached visual resources, thus forcing a refresh. The trouble with that one is, that while it seems to clear it doesn't force a refresh per se. So you'd still need to use something like signal.reloadUi hook that up to the InputButton.qml bit and find a way to make the images actually redraw.

If we go with the "have to mangle image string" approach I don't think this should live in signals. Those are really just stateless things that can be used to propagate things around that otherwise are too painful or global. I don't really want to add state etc. to them. Especially for this case I think the UIState class is a better fit. It's a context property of the engine, thus always available and has as its sole purpose the tracking of state, i.e. what is selected, etc. So in that regard the theme state would fit well in there.

[SubliminalsTV]

Makes sense — signal shouldn't be carrying state, and UIState is the natural home for it. Moved the revision counter there: it's now a themeRevision property on UIState with its own change signal, and the theme-refresh handler bumps it via uiState instead of signal. The image source in InputButton.qml binds to uiState.themeRevision accordingly, so signal is back to being purely stateless.

Re releaseResources() — thanks for digging into that; matches what I found, it drops the cache but doesn't trigger the repaint, so I've kept the URL-token approach now that it lives in the right place.

Also trimmed that comment down to your wording.

[WhiteMagic]

Thanks, looks good to me with all those changes.