eclipse.platform icon indicating copy to clipboard operation
eclipse.platform copied to clipboard

set background color for variables view in dark mode

Open tobiasmelcher opened this issue 9 months ago • 13 comments

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

https://github.com/user-attachments/assets/4e254658-fcb1-418d-9436-ee4f03a21942

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.

tobiasmelcher avatar Mar 24 '25 08:03 tobiasmelcher

@tobias-melcher Would you please try instead removing setBackground calls in https://github.com/eclipse-platform/eclipse.platform/blob/a791b17ec7febfe5631ea6e5a3df88e53ec0ca5b/debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/variables/VariablesView.java#L1165 ? 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.

akurtakov avatar Mar 24 '25 09:03 akurtakov

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 b9adeb09. ± Comparison against base commit 9e339b2f.

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

github-actions[bot] avatar Mar 24 '25 09:03 github-actions[bot]

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

https://github.com/eclipse-platform/eclipse.platform/blob/a791b17ec7febfe5631ea6e5a3df88e53ec0ca5b/debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/variables/VariablesView.java#L1165

? 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 avatar Mar 24 '25 09:03 tobiasmelcher

tree_background_color

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?

tobiasmelcher avatar Mar 24 '25 14:03 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 https://github.com/eclipse-platform/eclipse.platform.ui/issues/259 and https://bugs.eclipse.org/bugs/show_bug.cgi?id=577912

vogella avatar Mar 24 '25 15:03 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

@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.

tobiasmelcher avatar Mar 28 '25 14:03 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.

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.

BeckerWdf avatar Mar 28 '25 14:03 BeckerWdf

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.

tobiasmelcher avatar Mar 28 '25 15:03 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

BeckerWdf avatar Mar 28 '25 15:03 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

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?

tobiasmelcher avatar Mar 28 '25 15:03 tobiasmelcher

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?

vogella avatar Mar 31 '25 10:03 vogella

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?

tobiasmelcher avatar Apr 01 '25 08:04 tobiasmelcher

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

BeckerWdf avatar Apr 01 '25 09:04 BeckerWdf