From c9aee827bf85b7ac825312eb98fab342dfee2a09 Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Wed, 12 Aug 2026 18:22:30 +0200 Subject: [PATCH] Fix washed-out colors when dark theme is active at startup With dark preselected, the first document of a session rendered with a double-converted (washed-out) palette; resources showed it across every token, C# only on tokens outside the hand-authored dark palette. The same definition was registered with the theme manager under two identities: HighlightingManager hands out a delay-loaded wrapper whose members forward to the inner definition that HighlightingService.Load registers during materialization. Registering the wrapper afterwards snapshotted the shared colours AFTER the inner registration had already darkened them, so the snapshot's "light originals" were dark values and the rewrite darkened them a second time. In-session theme switches were unaffected because the first touch happens in Light, where both snapshots are pristine -- which is why the bug only appeared when dark was already active at first touch. Skip registration when the definition is already theme-aware: reading the marker forces the wrapper to materialize, so the check observes the inner registration. This also stops the remap from clobbering definitions whose XSHD opts out via ILSpy.IsThemeAware. Assisted-by: Claude:claude-fable-5:Claude Code --- .../ThemeAwareHighlightingColorizerTests.cs | 64 +++++++++++++++++++ ILSpy/Themes/ThemeManager.cs | 11 ++++ 2 files changed, 75 insertions(+) diff --git a/ILSpy.Tests/ThemeAwareHighlightingColorizerTests.cs b/ILSpy.Tests/ThemeAwareHighlightingColorizerTests.cs index 05db23704..c83f744d8 100644 --- a/ILSpy.Tests/ThemeAwareHighlightingColorizerTests.cs +++ b/ILSpy.Tests/ThemeAwareHighlightingColorizerTests.cs @@ -64,6 +64,39 @@ public class ThemeAwareHighlightingColorizerTests public HighlightingColor GetNamedColor(string name) => colors.Find(c => c.Name == name)!; } + // Mimics AvaloniaEdit's DelayLoadedHighlightingDefinition: every member forwards through + // materialization, and materializing runs a load callback that (like + // HighlightingService.Load) registers the INNER definition with the theme manager. The + // wrapper and the inner definition are distinct objects sharing the same live colours + // and Properties dictionary. + sealed class LazyLoadedDefinition : IHighlightingDefinition + { + readonly StubHighlightingDefinition inner; + bool materialized; + + public LazyLoadedDefinition(StubHighlightingDefinition inner) + { + this.inner = inner; + } + + StubHighlightingDefinition GetDefinition() + { + if (!materialized) + { + materialized = true; + ThemeManager.Current.RegisterThemableDefinition(inner); + } + return inner; + } + + public string Name => GetDefinition().Name; + public HighlightingRuleSet MainRuleSet => GetDefinition().MainRuleSet; + public IEnumerable NamedHighlightingColors => GetDefinition().NamedHighlightingColors; + public IDictionary Properties => GetDefinition().Properties; + public HighlightingRuleSet GetNamedRuleSet(string name) => GetDefinition().GetNamedRuleSet(name); + public HighlightingColor GetNamedColor(string name) => GetDefinition().GetNamedColor(name); + } + static HighlightingColor MakeColor(string name, Color foreground) => new() { Name = name, Foreground = new SimpleHighlightingBrush(foreground) }; @@ -76,6 +109,37 @@ public class ThemeAwareHighlightingColorizerTests return settings; } + [AvaloniaTest] + public void LazyLoadedDefinitionIsNotDarkenedTwiceAtDarkStartup() + { + var settings = AttachThemeSettings(); + try + { + // Dark is active BEFORE the definition is first touched -- the "dark theme + // preselected, session restores a document" startup ordering. + settings.Theme = "Dark"; + + var color = MakeColor("String", Colors.Red); + var pristine = (HighlightingColor)color.Clone(); + var wrapper = new LazyLoadedDefinition(new StubHighlightingDefinition(color)); + + // HighlightingService.GetByExtension registers what the HighlightingManager + // returns: the wrapper. Its first member access materializes the inner + // definition, whose own registration has then ALREADY darkened the shared + // colours in place -- so the wrapper registration must not treat those dark + // values as light originals and convert them a second time. + ThemeManager.Current.RegisterThemableDefinition(wrapper); + + var singleConversion = ThemeManager.GetColorForDarkTheme(pristine); + color.Should().Be(singleConversion, + "registering the lazy wrapper after its inner definition must not compound the dark conversion"); + } + finally + { + settings.Theme = "Light"; + } + } + [AvaloniaTest] public void RegistrationAfterConstructionDisablesPerPaintRemap() { diff --git a/ILSpy/Themes/ThemeManager.cs b/ILSpy/Themes/ThemeManager.cs index 93a9a18b2..357bd130f 100644 --- a/ILSpy/Themes/ThemeManager.cs +++ b/ILSpy/Themes/ThemeManager.cs @@ -108,6 +108,17 @@ namespace ICSharpCode.ILSpy.Themes public void RegisterThemableDefinition(IHighlightingDefinition definition) { ArgumentNullException.ThrowIfNull(definition); + // An already-theme-aware definition must not be registered again under a second + // identity. HighlightingManager hands out a delay-loaded WRAPPER whose members + // forward to the inner definition our Load() callback registers, so registering + // the wrapper too would snapshot the shared colours AFTER the inner registration + // already themed them -- and, when dark is active before the first touch (dark + // theme preselected at startup), dark-convert the already-dark values a second + // time, washing the palette out. Reading IsThemeAware here also forces the + // wrapper to materialize, so the check observes the inner registration's marker. + // This equally respects definitions whose XSHD opts out via ILSpy.IsThemeAware. + if (IsThemeAware(definition)) + return; if (!themableDefinitions.Contains(definition)) themableDefinitions.Add(definition); ApplyHighlightingColors(definition);