Provide Explicit "isDarkTheme" Attribute

[BeckerWdf]

Up to now we had some heuristics to check if a theme is a dark theme. This now makes this explicit.

• Add attribute to extension point
• Fill this attribute for the platforms dark theme
• Remove heuristics and now check this explicit flag

See also https://github.com/eclipse-platform/eclipse.platform.ui/pull/2797#issue-2850971179

[akurtakov]

My main concern with such "api" is what happens with e.g. gray #808080 are they dark or light? What about some dark(-ish) "blue"? It becomes a very thin line to decide with such boolean.

[sratz]

I think it's pretty common for themes to be either dark or light. At least all major operating systems also have this kind of primary boolean distinction.

[akurtakov]

@sratz I agree that OSes(GTK I know has it for sure) have such boolean distinction and I am not against this addition, just raising some concern about theming in general (not only in Eclipse) for which no one found better solution yet.

[github-actions[bot]]

Test Results

 1 823 files   -  1   1 823 suites   - 1   1h 27m 36s ⏱️ - 4m 16s  7 922 tests + 4   7 693 ✅ + 3  228 💤 ± 0  1 ❌ +1  23 808 runs   - 33  23 069 ✅  - 24  738 💤  - 10  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 257c857a. ± Comparison against base commit 037c5d70.

:recycle: This comment has been updated with latest results.

[eclipse-platform-bot]

This pull request changes some projects for the first time in this development cycle. Therefore the following files need a version increment:

bundles/org.eclipse.e4.ui.css.swt.theme/META-INF/MANIFEST.MF
bundles/org.eclipse.e4.ui.swt.gtk/META-INF/MANIFEST.MF
bundles/org.eclipse.e4.ui.swt.win32/META-INF/MANIFEST.MF
bundles/org.eclipse.e4.ui.workbench.renderers.swt.cocoa/META-INF/MANIFEST.MF
bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
tests/org.eclipse.e4.ui.tests.css.swt/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 854143c9a63bdee4996e6fb25480987763afa801 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Fri, 11 Apr 2025 11:37:33 +0000
Subject: [PATCH] Version bump(s) for 4.36 stream


diff --git a/bundles/org.eclipse.e4.ui.css.swt.theme/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.css.swt.theme/META-INF/MANIFEST.MF
index f140ef150e..ddf1170b83 100644
--- a/bundles/org.eclipse.e4.ui.css.swt.theme/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.css.swt.theme/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.e4.ui.css.swt.theme;singleton:=true
-Bundle-Version: 0.14.500.qualifier
+Bundle-Version: 0.14.600.qualifier
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
diff --git a/bundles/org.eclipse.e4.ui.swt.gtk/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.swt.gtk/META-INF/MANIFEST.MF
index c8a8100502..30a21c7959 100644
--- a/bundles/org.eclipse.e4.ui.swt.gtk/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.swt.gtk/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %fragmentName
 Bundle-SymbolicName: org.eclipse.e4.ui.swt.gtk;singleton:=true
-Bundle-Version: 1.2.200.qualifier
+Bundle-Version: 1.2.300.qualifier
 Fragment-Host: org.eclipse.e4.ui.css.swt.theme;bundle-version="0.10.0"
 Bundle-RequiredExecutionEnvironment: JavaSE-17
 Bundle-Localization: fragment-gtk
diff --git a/bundles/org.eclipse.e4.ui.swt.win32/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.swt.win32/META-INF/MANIFEST.MF
index 187d89e9d4..a08a75878f 100644
--- a/bundles/org.eclipse.e4.ui.swt.win32/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.swt.win32/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %fragmentName
 Bundle-SymbolicName: org.eclipse.e4.ui.swt.win32;singleton:=true
-Bundle-Version: 1.2.300.qualifier
+Bundle-Version: 1.2.400.qualifier
 Fragment-Host: org.eclipse.e4.ui.css.swt.theme;bundle-version="0.10.0"
 Bundle-RequiredExecutionEnvironment: JavaSE-17
 Bundle-Localization: fragment-win32
diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt.cocoa/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.workbench.renderers.swt.cocoa/META-INF/MANIFEST.MF
index 843b08f8fc..811e91bffb 100644
--- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt.cocoa/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt.cocoa/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Eclipse-PlatformFilter: (osgi.ws=cocoa)
 Bundle-SymbolicName: org.eclipse.e4.ui.workbench.renderers.swt.cocoa;singleton:=true
-Bundle-Version: 0.14.400.qualifier
+Bundle-Version: 0.14.500.qualifier
 Bundle-Name: %fragmentName
 Bundle-Vendor: %providerName
 Fragment-Host: org.eclipse.e4.ui.workbench.renderers.swt;bundle-version="[0.10.0,1.0.0)"
diff --git a/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
index 59abbfd238..11d118fd4c 100644
--- a/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.ui.themes;singleton:=true
-Bundle-Version: 1.2.2700.qualifier
+Bundle-Version: 1.2.2800.qualifier
 Bundle-Vendor: %Plugin.providerName
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.e4.ui.css.swt.theme
diff --git a/tests/org.eclipse.e4.ui.tests.css.swt/META-INF/MANIFEST.MF b/tests/org.eclipse.e4.ui.tests.css.swt/META-INF/MANIFEST.MF
index 1cb1884bb4..6a7fd90fc5 100644
--- a/tests/org.eclipse.e4.ui.tests.css.swt/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.e4.ui.tests.css.swt/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: org.eclipse.e4.ui.tests.css.swt; singleton:=true
-Bundle-Version: 0.12.800.qualifier
+Bundle-Version: 0.12.900.qualifier
 Require-Bundle: org.eclipse.e4.ui.css.core,
  org.eclipse.e4.ui.css.swt,
  org.eclipse.e4.ui.css.swt.theme;bundle-version="0.9.1",
-- 
2.49.0

Further information are available in Common Build Issues - Missing version increments.

[mickaelistria]

I'm still torn about whether "isDark" should be further declarative. Could we instead try to get the default background color from the them, and resolve whether the background color is dark or not; using eg the usual formula to convert RGB to greyscale (0.3 * R + 0.59 * B + 0.11 * G) and evaluating whether this grey is dark (eg < 122) ?

[BeckerWdf]

I'm still torn about whether "isDark" should be further declarative. Could we instead try to get the default background color from the them, and resolve whether the background color is dark or not; using eg the usual formula to convert RGB to greyscale (0.3 * R + 0.59 * B + 0.11 * G) and evaluating whether this grey is dark (eg < 122) ?

But that would replace one heuristic with another one. I think it's quite good to make this explicit. And keep in mind we don't have tons of themes in platform (and out there in the community). Adapting them should be easy.

[BeckerWdf]

get the default background color from the theme

What should that be? Pls. explain based on the existing themes which color definition would be the "default background color".

[mickaelistria]

But that would replace one heuristic with another one.

Is this really just an heuristic or is the greyscale value the actual definition of whether a theme is dark or not ? Note that I'm not against allowing people to explicit/override whether their theme is to be treated as dark or not if they want to; but I find very strange that the default decision made by the Platform wouldn't be sufficient in most cases. We can't expect every theme to adopt the extension (we do not control all the themes from the community, many of theme are probably built-in inside some low-maintained RCP apps...); and we cannot just force burden to theme developers about forcing them to set an extra flag if the value can be reliably deduced from the payload in the vast majority of cases.

[mickaelistria]

What should that be? Pls. explain based on the existing themes which color definition would be the "default background color".

Probably something like Composite > background-color; or even better way to infer it would be to be able to load the theme, create a dummy invisible shell, apply the theme on it and query the background color.

[BeckerWdf]

What should that be? Pls. explain based on the existing themes which color definition would be the "default background color".

Probably something like Composite > background-color; or even better way to infer it would be to be able to load the theme, create a dummy invisible shell, apply the theme on it and query the background color.

To be honest I find this overcomplicated. For example we have code like this:

boolean hasDarkTheme = getThemes().stream().anyMatch(t -> t.isDark());

Do you really like to create a dummy invisible shell for each of the themes when the alternative is setting a little flag in the plugin.xml? And we don't have that many themes anyway. So I think the the cost / benefit relation is not very good for that.

[sratz]

I agree with @BeckerWdf. Why implement a guessing algorithm when we can make it explicit?

Other IDEs also have an explicit flag, e.g. VS Code: https://github.com/microsoft/vscode/blob/544674f984e97b8aa9a444cb5487cfc4c27f2b7a/src/vs/platform/theme/common/theme.ts#L6-L14

[akurtakov]

@BeckerWdf Would you please rebase this one to fix the conflict?

[Mailaender]

There is precedent for a boolean setting in Eclipse already at the text editor themes:

[mickaelistria]

I would not take this part of TM4E as an example to rely on (it's not bad as it does the job well, but it didn't got through brainstorming to find the best sustainable solution).