set background color for variables view in dark mode

[tobiasmelcher]

Variables View on MacOS flickers with white background in dark mode when variable nodes are expanded via the keyboard.
See screencast:

variable_viewer_white_background.mov

One can also see that the Variables View gets a white background after closing a debug session.
This pull request sets a dark background color for the Variables View in the css style settings so that the flickering disappears.

[akurtakov]

@tobias-melcher Would you please try instead removing setBackground calls in

eclipse.platform/debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/variables/VariablesView.java

Line 1165 in a791b17

if (control.getBackground().equals(fSashForm.getDisplay().getSystemColor(SWT.COLOR_WIDGET_BACKGROUND))) {

? IMO this flickering comes from background set once by the view and later overriden by the theme engine, if the view doesn't play with colors there shouldn't be flickering.
Please test in both light and dark.

[github-actions]

Test Results

 1 598 files  ±0   1 598 suites  ±0   1h 39m 5s ⏱️ + 8m 35s
 4 173 tests ±0   4 150 ✅ ±0   23 💤 ±0  0 ❌ ±0 
11 998 runs  ±0  11 832 ✅ ±0  166 💤 ±0  0 ❌ ±0 

Results for commit b9adeb0. ± Comparison against base commit 9e339b2.

♻️ This comment has been updated with latest results.

[tobiasmelcher]

@tobias-melcher Would you please try instead removing setBackground calls in

eclipse.platform/debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/variables/VariablesView.java

Line 1165 in a791b17

if (control.getBackground().equals(fSashForm.getDisplay().getSystemColor(SWT.COLOR_WIDGET_BACKGROUND))) {

? IMO this flickering comes from background set once by the view and later overriden by the theme engine, if the view doesn't play with colors there shouldn't be flickering.
Please test in both light and dark.

I tried this. It was still flickering after removing both calls
fSashForm.setBackground(fSashForm.getDisplay().getSystemColor(SWT.COLOR_LIST_BACKGROUND));
and
fSashForm.setBackground(fSashForm.getDisplay().getSystemColor(SWT.COLOR_WIDGET_BACKGROUND));

I have seen that tree.setBackground() is called with null of the VariablesView by the CSS engine and I assume that his is making the problem. The flickering does not appear on Windows, only on MacOS. So, the SWT tree implementation for Mac behaves differently than on Windows regarding setBackground(null) call.

[tobiasmelcher]

I think I found it.

@Override
Color defaultBackground () {
	return display.getWidgetColor (SWT.COLOR_LIST_BACKGROUND);
}

Tree#defaultBackground returns a white color also when theme is set to dark.
If no CSS style is set, then CSSSWTColorHelper is calling tree.setBackgroundColor(null) with null as argument and therefore the white background color is taken.

Looking at the windows implementation doesn't make it more clear.

@Override
int defaultBackground () {
	return OS.GetSysColor (OS.COLOR_WINDOW);
}

Both implementations do not take theming into account. Is that good? I think no. What is your opinion? Or should display.getWidgetColor (SWT.COLOR_LIST_BACKGROUND); return a dark color in dark theme?

[vogella]

IIRC we also configure the SWT colors according to the theme but they are set to late during startup.

@azoitl tried to fix this a long time ago. See eclipse-platform/eclipse.platform.ui#259 and https://bugs.eclipse.org/bugs/show_bug.cgi?id=577912

[tobiasmelcher]

IIRC we also configure the SWT colors according to the theme but they are set to late during startup.

@azoitl tried to fix this a long time ago. See eclipse-platform/eclipse.platform.ui#259 and https://bugs.eclipse.org/bugs/show_bug.cgi?id=577912

@vogella, could you please point me to the location where the Theming Engine is overriding the SWT.COLOR_LIST_BACKGROUND color for the dark mode. I cannot find it. Thanks a lot.

[BeckerWdf]

IIRC we also configure the SWT colors according to the theme but they are set to late during startup.
@azoitl tried to fix this a long time ago. See eclipse-platform/eclipse.platform.ui#259 and https://bugs.eclipse.org/bugs/show_bug.cgi?id=577912

@vogella, could you please point me to the location where the Theming Engine is overriding the SWT.COLOR_LIST_BACKGROUND color for the dark mode. I cannot find it. Thanks a lot.

I know of this location:
https://github.com/eclipse-platform/eclipse.platform.ui/blob/b6af990cac142bdba33768e29ec0d62482f0ba54/bundles/org.eclipse.ui.themes/css/dark/e4-dark_preferencestyle.css#L4

but there SWT.COLOR_LIST_BACKGROUND is not redefined.

[tobiasmelcher]

flickering appears because the tree background color is sometimes set to white in the dark mode because of following CSS:

Tree[swt-lines-visible="true"]
background-color: unset;

org.eclipse.e4.ui.css.swt.helpers.CSSSWTColorHelper#setBackground is then called with color argument "null" and the SWT Tree implementation on MacOS then takes its default background color via display.getWidgetColor (SWT.COLOR_LIST_BACKGROUND); . I don't see a mechanism where the display widget colors are overridden by the CSS styles.

Unfortunately, I don't know in which CSS file this "background-color: unset;" is written down. There are too many CSS files.

[BeckerWdf]

Unfortunately, I don't know in which CSS file this "background-color: unset;" is written down. There are too many CSS files.

It's here: https://github.com/eclipse-platform/eclipse.platform.ui/blob/b6af990cac142bdba33768e29ec0d62482f0ba54/bundles/org.eclipse.ui.themes/css/e4-dark_mac.css#L44

[tobiasmelcher]

Unfortunately, I don't know in which CSS file this "background-color: unset;" is written down. There are too many CSS files.

It's here: https://github.com/eclipse-platform/eclipse.platform.ui/blob/b6af990cac142bdba33768e29ec0d62482f0ba54/bundles/org.eclipse.ui.themes/css/e4-dark_mac.css#L44

thanks so much. Removing

Tree[swt-lines-visible=true] {
	background-color: unset;
}

from bundles/org.eclipse.ui.themes/css/e4-dark_mac.css fixes the flickering problem. Do you know the reason why this was introduced?

[vogella]

Unfortunately, I don't know in which CSS file this "background-color: unset;" is written down. There are too many CSS files.

from bundles/org.eclipse.ui.themes/css/e4-dark_mac.css fixes the flickering problem. Do you know the reason why this was introduced?

Maybe to handle an older now fixed bug in SWT Mac? If you do not see a bad impact in the light theme with this line removed, just send a PR to remove it. Maybe the same applies for the CSS for trees?

[tobiasmelcher]

I removed

Table[swt-lines-visible=true] {
	background-color: unset;
}

Tree[swt-lines-visible=true] {
	background-color: unset;
}

from e4-dark_mac.css locally and did some testing. Unfortunately, the zebra style then disappears from the "Problems" and other views. So, this approach is not the way to go because zebra seems quite important in the dark theme.

I would therefore vote that we go with the approach of this pull request by explicitly setting a background color for the Variables view so that the white flickering while expanding tree nodes disappears. This white flickering hurts in the eyes and must be fixed from my point of view.
Please let me know if you have an alternative proposal which will also solve the flickering issue?

[BeckerWdf]

windows build failure is unrelated.
Plan to merge tomorrow if nobody objects.

[BeckerWdf]

this change causes a side effect: The tree in the variables view does then no longer have the zebra 🦓 lines.

[BeckerWdf]

The interesting thing is that e.g. JDTs problems view also uses this zebra feature but does not have this flickering issue.