Skip to content

Add themes to form loading (Issue #325)#348

Open
jpgaribotti wants to merge 3 commits intotexus:1.xfrom
jpgaribotti:theme-support
Open

Add themes to form loading (Issue #325)#348
jpgaribotti wants to merge 3 commits intotexus:1.xfrom
jpgaribotti:theme-support

Conversation

@jpgaribotti
Copy link
Copy Markdown

This PR has been broken into individual commits that can be built and tested separately for ease of review. Some commits only add interface changes and delay implementation to a later commit.

Summary

This change implements theme-aware form loading so .txt forms can bind renderers to named themes and optional in-form fallbacks, with explicit load options on Container / BackendGui. It closes the gap between file and stream loading for default-theme handling and adds tests around the new behavior.

Proposed in Issue #325.

Theme / form loading

  • FormLoadOptions (themesByAlias, applyDefaultTheme) is accepted by overloads of loadWidgetsFromFile and loadWidgetsFromStream (lvalue and rvalue stream).
  • loadWidgetsFromStream now follows the same default-theme clear/restore pattern as loadWidgetsFromFile, unless applyDefaultTheme is used to keep the global default theme during load (so widgets without a Renderer line match programmatic construction when desired).
  • Form files may define top-level Theme.<Alias> sections with per-widget renderer fallbacks (e.g. Button = &1;).
  • Widget Renderer values may use @Alias / @Alias.Section, resolved from FormLoadOptions::themesByAlias at runtime and/or form-defined fallbacks.
  • Widget loading now takes WidgetLoadResources (extends the previous renderers map with theme alias resolution), with Container::loadWidgetsFromNodeTree available where a parsed node tree is loaded with options.

Tests

  • Added/extended loading tests (e.g. tests/Loading/FormThemeLoad.cpp) covering alias binding, applyDefaultTheme, and parity with the two-argument stream API when using default FormLoadOptions.

Backward compatibility

Existing two-argument loadWidgetsFromFile / loadWidgetsFromStream call sites keep prior behavior by routing through default-constructed FormLoadOptions.

Implement Container::loadWidgetsImpl with clear/restore of Theme::getDefault()
for both loadWidgetsFromFile and loadWidgetsFromStream.

Tests: default theme restored; file vs stream parity for widget colors.
Changelog: document stream behavior alignment.
@texus
Copy link
Copy Markdown
Owner

texus commented Apr 6, 2026

I haven't had the time to look at all the changes in detail yet, but it looked fine on first sight except for one big thing.

Changes to the Widget::load function signature break the backwards-compatibility of the API, as anyone with a custom widget will need to make this change. Code that was written to use an earlier TGUI 1.x version should ideally continue to work without changes.

Annoyingly, this means you also can't simply add another load overload and call that one, because then the old function would no longer be called. The old overload must continue to function correctly. One way to do it might be something like this:

void Widget::load(const std::unique_ptr<DataIO::Node>& node, const LoadingRenderersMap& renderers)
{
    // Implementation here. Does same as before if m_loadRuntimeThemesByAlias and m_loadThemeFallbacks are nullptr.
    
    // Would contains something like this:
    //   WidgetLoadResources widgetResources;
    //   widgetResources.renderers = availableRenderers;
    //   widgetResources.runtimeThemesByAlias = m_loadRuntimeThemesByAlias;
    //   widgetResources.themeFallbacks = m_loadThemeFallbacks;
    //   loadRendererFromFormValue(this, node->propertyValuePairs[U"Renderer"]->value, widgetResources);

    // Implementation of Container::load has to be careful with how it calls the Widget::load function on itself.
}

// This function overload would NOT be virtual
void Widget::load(const std::unique_ptr<DataIO::Node>& node, const WidgetLoadResources& resources)
{
    m_loadRuntimeThemesByAlias = resources.runtimeThemesByAlias;
    m_loadThemeFallbacks = resources.themeFallbacks;
    load(renderers);
    m_loadRuntimeThemesByAlias = nullptr;
    m_loadThemeFallbacks = nullptr;
}

If I ever make breaking changes in the future then I will replace the code to implement it like you did right now.

@jpgaribotti
Copy link
Copy Markdown
Author

Ah, I see what you mean, I kept backwards compatibility of the loadWidgetsFrom... functions and forgot the existence of client subclasses. I got too focused on my feature and didn't respect the API on a minor version bump.

We can't have an overload because if we call widgetPtr->load(node, resources) on a subclass that overrode load(node, renderers), it will not run the subclass' custom code, leading to a broken widget. So we have to always call load(node, renderers), assuming the old behavior, and opt-in to the new behavior by setting the extra variables on a side-channel instead of passing them as a parameter.

I'll have a look at it and update the PR to maintain backwards compatibility of the Widget API.

@jpgaribotti jpgaribotti marked this pull request as draft April 7, 2026 21:03
@jpgaribotti jpgaribotti force-pushed the theme-support branch 4 times, most recently from b06be9b to ee4b8b0 Compare April 7, 2026 22:12
Introduce FormLoadOptions with applyDefaultTheme and overloads of loadWidgetsFromFile / loadWidgetsFromStream on Container and BackendGui. When applyDefaultTheme is false (default), behavior matches the unified loader: clear the global default theme for the duration of the load. When true, the default theme stays active so widgets without an explicit Renderer match programmatic construction.
@jpgaribotti jpgaribotti force-pushed the theme-support branch 2 times, most recently from 6b10806 to 13512f0 Compare April 8, 2026 09:04
@jpgaribotti jpgaribotti marked this pull request as ready for review April 8, 2026 09:04
Extend FormLoadOptions with themesByAlias. Forms may use Renderer = @alias or @Alias.Section, resolved from the runtime map or Theme.<Alias> fallback sections. Add WidgetLoadResources, Widget::load(node, WidgetLoadResources) dispatching to virtual load(node, LoadingRenderersMap), and FormThemeLoad tests. Updates changelog.
@jpgaribotti
Copy link
Copy Markdown
Author

@texus updated the PR to avoid breaking the API, following your suggested approach. Added various tests to confirm containers and pre-existing subclasses continue to work as expected. Rebased the changes to avoid having the API break in the history.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants