Browse Source

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
pull/3973/head
Christoph Wille 1 month ago committed by Siegfried Pammer
parent
commit
c9aee827bf
  1. 64
      ILSpy.Tests/ThemeAwareHighlightingColorizerTests.cs
  2. 11
      ILSpy/Themes/ThemeManager.cs

64
ILSpy.Tests/ThemeAwareHighlightingColorizerTests.cs

@ -64,6 +64,39 @@ public class ThemeAwareHighlightingColorizerTests @@ -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<HighlightingColor> NamedHighlightingColors => GetDefinition().NamedHighlightingColors;
public IDictionary<string, string> 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 @@ -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()
{

11
ILSpy/Themes/ThemeManager.cs

@ -108,6 +108,17 @@ namespace ICSharpCode.ILSpy.Themes @@ -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);

Loading…
Cancel
Save