Community
Participate
Working Groups
Created attachment 282519 [details] Popup Unreadable text with window 10 and Eclipse dark theme in content assist popup
Isn't this a critical bug for a developer who prefer to use the dark theme ?
(In reply to Gayan Perera from comment #1) > Isn't this a critical bug for a developer who prefer to use the dark theme ? While annoying, that doesn't qualify this bug as critical: https://wiki.eclipse.org/Bug_Reporting_FAQ#What_is_the_difference_between_Severity_and_Priority.3F
Alexandr, can this colour configured in SWT?
Can this be in any help on changing colors https://git.eclipse.org/r/#/c/45555/ ?
This is the ticket which introduced those https://bugs.eclipse.org/bugs/show_bug.cgi?id=434309
I prefer to get my other patches merged first.
If you want a workaround for the unreadable text, try changing the the 'Content Assist foreground color' to some green, magenta or any color that works well with the dark and light background. This setting is in: Window > Preferences > General > Appearance > Colors and Fonts > Basic
When can we expect a proper fix for this issue ? I means if this is already done for tree and table ?
I checked in eclipse 2020-3 this problem is not there. At least in redhat codeready ide.
@Lars was there any recent changes that can cause the selection color to be like in the screenshot? Because this is working as expected in previous release. And i checked all content assist popups have the same problem.
(In reply to Gayan Perera from comment #10) > @Lars was there any recent changes that can cause the selection color to be > like in the screenshot? Because this is working as expected in previous > release. And i checked all content assist popups have the same problem. Not that I'm aware of.
@Lars this should be a regression issue. This issue is not there in ibuild I20200330-1800. Can someone look at whats has happened during April in this area ? Otherwise the whole effort of making dark theme shine on upcoming releases will be in vain.
When giving the context assist popup focus, i.e. clicking on it, the selection color turns dark and the text is readable. Also in the light theme there is a change in selection color when the popup gains focus. In older versions of the dark them the initial selection is dark, now it is a light color.
(In reply to Gayan Perera from comment #12) > @Lars this should be a regression issue. This issue is not there in ibuild > I20200330-1800. Can someone look at whats has happened during April in this > area ? Otherwise the whole effort of making dark theme shine on upcoming > releases will be in vain. Can't reproduce under Linux. I'm currently setting up a windows machine to test better, this may take a few days as this is a side activity for me.
yes it cannot be reproduced on linux. Its only windows that happened to me. My office machine is windows and personal one is linux. But i do all eclipse development on my linux box. So i also thought of having a windows vm inside linux to test things like this.
@Lars @Andrey found the commit which cause this issue https://github.com/eclipse/eclipse.platform.ui/commit/b987db6af682f49cfae9d828efa26ac97bf11e3c I had to run eclipse and debug with commit by commit on a windows VM to find this out :(
Actually this particular call is the responsible for this color change OS.setTheme (isDark);
(In reply to Gayan Perera from comment #17) > Actually this particular call is the responsible for this color change > OS.setTheme (isDark); This might indicate an SWT issue
Seems like i'm not going to use the dark theme for sometime then :(
(In reply to Gayan Perera from comment #17) > Actually this particular call is the responsible for this color change > OS.setTheme (isDark); Niraj/Alexandr please check. Calling this seem to disable / break other styling.
Calling OS.setTheme (false) seems to break functionality introduced by Bug 434309. Not only in code completion, in all places, e.g., in the Git Clone Repository dialog. Updating this to critical as it makes the dark theme unsuable under Windows.
Created attachment 282550 [details] Clone dialog
The class which does the custom painting is org.eclipse.e4.ui.internal.css.swt.dom.AbstractControlSelectionEraseListener modifying the drawing for the forground and background in its handleEvent method. I don't see a reason in the debugger why this is affected by the OS.setTheme (true) setting.
Creating a SWT only reproducer took me a very long time, here it is: Alexandr / Niraj, please have a look. If OS.setTheme (false) is used, the columns get pained correct, in the below form not. package org.eclipse.swt.snippets; /* * Table example snippet: create a table (no columns, no headers) * * For a list of all SWT example snippets see * http://www.eclipse.org/swt/snippets/ */ import org.eclipse.swt.*; import org.eclipse.swt.graphics.*; import org.eclipse.swt.internal.win32.OS; import org.eclipse.swt.widgets.*; public class BrokenHighlighter { public static void main(String[] args) { Display display = new Display(); OS.setTheme (true); Shell shell = new Shell(display); shell.setText("Snippet 35"); Table table = new Table(shell, SWT.BORDER | SWT.V_SCROLL | SWT.H_SCROLL); for (int i = 0; i < 12; i++) { TableItem item = new TableItem(table, 0); item.setText("Item " + i); item = new TableItem(table, 1); item.setText("Item " + i); } table.select(0); Rectangle clientArea = shell.getClientArea(); table.setBounds(clientArea.x, clientArea.y, 100, 100); table.addListener(SWT.EraseItem, new TableControlSelectionEraseListener()); shell.setSize(200, 200); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); } public static class TableControlSelectionEraseListener extends AbstractControlSelectionEraseListener { @Override protected void fixEventDetail(Control control, Event event) { if ((event.detail & SWT.FOCUSED) != 0 || event.display.getFocusControl() == control) { // it has focus: remove the selected state as we // just painted it and it no longer needs to be // painted. // Note: it's not enough checking if the cell is focused because // if the table has the MULTI select style, only one will be // focused, but we still want to remove the selected state on // the other selected cells. event.detail &= ~SWT.SELECTED; } // it doesn't have focus: don't change the drawing // as the table selection won't appear properly if we // remove the selected state. } @Override protected int getNumberOfColumns(Control control) { return ((Table) control).getColumnCount(); } } public static abstract class AbstractControlSelectionEraseListener implements Listener { @Override public void handleEvent(Event event) { Scrollable control = (Scrollable) event.widget; int columnCount = getNumberOfColumns(control); boolean selected = (event.detail & SWT.SELECTED) != 0; boolean hot = (event.detail & SWT.HOT) != 0; // If we're painting the selection we also deal with hotness to be // consistent (i.e.: hotness is a 'selection preview on hover'). event.detail &= ~SWT.HOT; if (selected || hot) { GC gc = event.gc; Rectangle area = control.getClientArea(); // Handling hotness with more than one column doesn't // work well because we'd have to change the clipping // for all of that (which isn't really possible)... boolean handlingOnlyHot = !selected && hot; if (handlingOnlyHot) { if (columnCount > 1) { return; } } String dataBackgroundKey; String dataBorderKey; Color background = Display.getCurrent().getSystemColor(SWT.COLOR_RED); System.out.println("Background " + background); Color border = background; Color selectionForeground = Display.getCurrent().getSystemColor(SWT.COLOR_BLACK);; // Update clip to cover the whole column. int width = area.width; if (event.index == columnCount - 1 || columnCount == 0) { // i.e.: we only need to fix this for the last column // or if the tree/table reports having no columns (which // means single column... really weird hum?) if (width > 0) { Region region = new Region(); gc.getClipping(region); region.add(event.x, event.y, width, event.height); gc.setClipping(region); region.dispose(); } } if (background != null) { Color oldbackground = gc.getBackground(); gc.setBackground(background); try { gc.fillRectangle(0, area.y, area.width + 2, area.height); } finally { gc.setBackground(oldbackground); } } if (border != null) { gc.setForeground(border); gc.drawRectangle(0, event.y, width - 1, event.height - 1); } // Restore the foreground for proper drawing later on. // This should be the color that SWT uses to draw the // foreground later on! gc.setForeground(selectionForeground); // we just painted the background... event.detail &= ~SWT.BACKGROUND; fixEventDetail(control, event); } } /** * Subclasses should override to fix the event.detail after the selection event * was handled. * * @param control the control which had the erase event. * @param event the event to have the details fixed. */ protected abstract void fixEventDetail(Control control, Event event); /** * @param control * @return the number of columns from the control */ protected abstract int getNumberOfColumns(Control control); } }
I could have a look next week.
(In reply to Alexandr Miloslavskiy from comment #25) > I could have a look next week. Thanks
Lars, could you please explain your snippet? I've tested it but I don't see any problems, whether I use `OS.setTheme(false)` or `OS.setTheme(true)`. In both cases, I see a table with red rows. The only difference is that with `OS.setTheme(false)` I can also see items highlight with red when mouse hovers over them.
(In reply to Alexandr Miloslavskiy from comment #27) > Lars, could you please explain your snippet? > > I've tested it but I don't see any problems, whether I use > `OS.setTheme(false)` or `OS.setTheme(true)`. > > In both cases, I see a table with red rows. The only difference is that with > `OS.setTheme(false)` I can also see items highlight with red when mouse > hovers over them. I confirm the observations from Alexandr.
Are trying this on windows 10 ? My windows build number is 1909.
Created attachment 282592 [details] Screenshot You have to select the row and afterwards click outside the application window. If theme is not set the red color is preserved. Here is a screenshot with OS.setTheme (true);
Created attachment 282593 [details] Screencast Animated Gif for demo
Created attachment 282594 [details] Screencast demoning both cases
If you see "Screencast demoning both cases" you see the different table selection color behavior depending on OS.setTheme call.
Alexandr / Niraj, can you reproduce?
(In reply to Lars Vogel from comment #30) > Created attachment 282592 [details] > Screenshot > > You have to select the row and afterwards click outside the application > window. If theme is not set the red color is preserved. > > Here is a screenshot with OS.setTheme (true); Yes, problem is reproducible with above steps. Not sure if it's some limitation with the DARKMODE_EXPLORER mode. (In reply to Alexandr Miloslavskiy from comment #6) > I prefer to get my other patches merged first. @Alexandr: Until you are available, please share any pointers for investigation. We can involve Conrad to take this further. Thanks!
Been studying this yesterday (along with few other tasks I needed to do). I have narrowed the problem (or one of problems?) to `if (explorerTheme)` block in `Table.sendEraseItemEvent()`. This means that the problem is in fact caused by SWT itself. I also reduced the repro snippet to this: ---------------- OS.setTheme (true); Display display = new Display(); Shell shell = new Shell(display); shell.setLayout(new RowLayout(SWT.VERTICAL)); Table table = new Table(shell, 0); for (int i = 0; i < 20; i++) { TableItem item = new TableItem(table, 0); item.setText("Item \u25a0 " + i); } table.select(0); Text hint = new Text(shell, SWT.MULTI | SWT.READ_ONLY); hint.setText( "1) Set focus to Table\n" + "2) Set focus to this Text\n" + "3) Notice that Table's item has bad background\n" + "4) Try without OS.setTheme (true)" ); final Color backColor = new Color(display, 0x28, 0x28, 0x28); final Color foreColor = new Color(display, 0xD0, 0xD0, 0xD0); table.setBackground(backColor); table.setForeground(foreColor); table.addListener(SWT.EraseItem, event -> { event.gc.setBackground(backColor); event.gc.fillRectangle(event.getBounds()); }); shell.pack(); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); ----------------
I tried to understand the purpose of `if (explorerTheme)` block in `Table.sendEraseItemEvent()`, but didn't arrive to any clear conclusions yet.
I intend to continue investigating at least today. I will try to remember to post a summary of my findings.
New Gerrit change created: https://git.eclipse.org/r/161783
This patch removes one of two problems (white background that is partially seen behind grey background). To solve the second problem, I guess all styling shall be removed from 'Table', like in Bug 562576. I also recommend to use 'SWT.FULL_SELECTION' for Dark Table.
Gerrit change https://git.eclipse.org/r/161783 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=ca3497b8f70d3659aca75bfd191bad536a5d0767
(In reply to Eclipse Genie from comment #41) > Gerrit change https://git.eclipse.org/r/161783 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=ca3497b8f70d3659aca75bfd191bad536a5d0767 Thanks Alexandr for the fix patch, resolving.
(In reply to Alexandr Miloslavskiy from comment #40) > This patch removes one of two problems (white background that is partially > seen behind grey background). > > To solve the second problem, I guess all styling shall be removed from > 'Table', like in Bug 562576. > > I also recommend to use 'SWT.FULL_SELECTION' for Dark Table. Please raise a separate bug describing the second problem(may be with a screen shot). Thanks!
(In reply to Alexandr Miloslavskiy from comment #40) > I also recommend to use 'SWT.FULL_SELECTION' for Dark Table. There's been some questioning as to why SWT.FULL_SELECTION is not enabled on Windows but seems to be on other platforms. See Bug 561896 for example.
> Please raise a separate bug describing the second problem > There's been some questioning as to why SWT.FULL_SELECTION is not enabled on > Windows but seems to be on other platforms. See Bug 561896 for example. Pure Eclipse issues are outside the scope of my experience and priorities. I think Lars would be a better fit to try it. The second problem, as I see it, is that Eclipse draws grey text on grey background. Grey background comes from Windows dark theme and is fine on its own, because Windows also uses dark text on it. So the only problem is that Eclipse draws grey text. I imagine that removing 'Table' portion of Eclipse styling (similar to Bug 562576) should help.
Verified in Eclipse SDK Version: 2020-06 (4.16) Build id: I20200501-0520 OS: Windows 10, v.10.0, x86_64 / win32 Java version: 11.0.7 Thanks Gayan for pushing on this issue and Alexandr for providing the fix! The next build should also have nicer tree arrows under Windows, see Bug 562576.
(In reply to Alexandr Miloslavskiy from comment #45) > The second problem, as I see > it, > is that Eclipse draws grey text on grey background. Grey background comes > from > Windows dark theme and is fine on its own, because Windows also uses dark > text > on it. So the only problem is that Eclipse draws grey text. I imagine that > removing 'Table' portion of Eclipse styling (similar to Bug 562576) should > help. Created Bug 562781 for that.
