| Summary: | [Graphics] Improve view menu button to make it look good on a dark background | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Lars Vogel <Lars.Vogel> | ||||||||||||||
| Component: | UI | Assignee: | Lars Vogel <Lars.Vogel> | ||||||||||||||
| Status: | CLOSED DUPLICATE | QA Contact: | |||||||||||||||
| Severity: | enhancement | ||||||||||||||||
| Priority: | P3 | CC: | andrea.guarinoni, daniel.rolka, daniel_megert, Lars.Vogel, markus.kell.r, tmccrary, tmccrary | ||||||||||||||
| Version: | 4.4 | ||||||||||||||||
| Target Milestone: | --- | ||||||||||||||||
| Hardware: | PC | ||||||||||||||||
| OS: | All | ||||||||||||||||
| Whiteboard: | |||||||||||||||||
| Bug Depends on: | 466511, 495033, 501491 | ||||||||||||||||
| Bug Blocks: | |||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Lars Vogel
Created attachment 240801 [details]
Screenshot
Got the info from Daniel Rolka that this icon is defined in CTabFolderRenderer. Looks like this one is not an icon but drawn directly.
void drawChevron(GC gc, Rectangle chevronRect, int chevronImageState) {
if (chevronRect.width == 0 || chevronRect.height == 0) return;
int selectedIndex = parent.selectedIndex;
// draw chevron (10x7)
Display display = parent.getDisplay();
Point dpi = display.getDPI();
int fontHeight = 72 * 10 / dpi.y;
FontData fd = parent.getFont().getFontData()[0];
fd.setHeight(fontHeight);
Font f = new Font(display, fd);
int fHeight = f.getFontData()[0].getHeight() * dpi.y / 72;
int indent = Math.max(2, (chevronRect.height - fHeight - 4) /2);
int x = chevronRect.x + 2;
int y = chevronRect.y + indent;
int count;
int itemCount = parent.getItemCount();
if (parent.single) {
count = selectedIndex == -1 ? itemCount : itemCount - 1;
} else {
int showCount = 0;
while (showCount < parent.priority.length && parent.items[parent.priority[showCount]].showing) {
showCount++;
}
count = itemCount - showCount;
}
String chevronString = count > 99 ? "99+" : String.valueOf(count); //$NON-NLS-1$
switch (chevronImageState & (SWT.HOT | SWT.SELECTED)) {
case SWT.NONE: {
Color chevronBorder = parent.single ? parent.getSelectionForeground() : parent.getForeground();
gc.setForeground(chevronBorder);
gc.setFont(f);
gc.drawLine(x,y, x+2,y+2);
gc.drawLine(x+2,y+2, x,y+4);
gc.drawLine(x+1,y, x+3,y+2);
gc.drawLine(x+3,y+2, x+1,y+4);
gc.drawLine(x+4,y, x+6,y+2);
gc.drawLine(x+6,y+2, x+5,y+4);
gc.drawLine(x+5,y, x+7,y+2);
gc.drawLine(x+7,y+2, x+4,y+4);
gc.drawString(chevronString, x+7, y+3, true);
break;
}
case SWT.HOT: {
gc.setForeground(display.getSystemColor(BUTTON_BORDER));
gc.setBackground(display.getSystemColor(BUTTON_FILL));
gc.setFont(f);
gc.fillRoundRectangle(chevronRect.x, chevronRect.y, chevronRect.width, chevronRect.height, 6, 6);
gc.drawRoundRectangle(chevronRect.x, chevronRect.y, chevronRect.width - 1, chevronRect.height - 1, 6, 6);
gc.drawLine(x,y, x+2,y+2);
gc.drawLine(x+2,y+2, x,y+4);
gc.drawLine(x+1,y, x+3,y+2);
gc.drawLine(x+3,y+2, x+1,y+4);
gc.drawLine(x+4,y, x+6,y+2);
gc.drawLine(x+6,y+2, x+5,y+4);
gc.drawLine(x+5,y, x+7,y+2);
gc.drawLine(x+7,y+2, x+4,y+4);
gc.drawString(chevronString, x+7, y+3, true);
break;
}
case SWT.SELECTED: {
gc.setForeground(display.getSystemColor(BUTTON_BORDER));
gc.setBackground(display.getSystemColor(BUTTON_FILL));
gc.setFont(f);
gc.fillRoundRectangle(chevronRect.x, chevronRect.y, chevronRect.width, chevronRect.height, 6, 6);
gc.drawRoundRectangle(chevronRect.x, chevronRect.y, chevronRect.width - 1, chevronRect.height - 1, 6, 6);
gc.drawLine(x+1,y+1, x+3,y+3);
gc.drawLine(x+3,y+3, x+1,y+5);
gc.drawLine(x+2,y+1, x+4,y+3);
gc.drawLine(x+4,y+3, x+2,y+5);
gc.drawLine(x+5,y+1, x+7,y+3);
gc.drawLine(x+7,y+3, x+6,y+5);
gc.drawLine(x+6,y+1, x+8,y+3);
gc.drawLine(x+8,y+3, x+5,y+5);
gc.drawString(chevronString, x+8, y+4, true);
break;
}
}
f.dispose();
}
We're responsible for the CTabFolder and others. PW (In reply to Lars Vogel from comment #3) > Looks like this one is not an icon but drawn directly. > > void drawChevron(GC gc, Rectangle chevronRect, int chevronImageState) { This is the chevron for when there are 2 many tabs to display, it's not the view dropdown menu button. PW (In reply to Paul Webster from comment #5) > This is the chevron for when there are 2 many tabs to display, it's not the > view dropdown menu button. > > PW Thanks. From the other bug it is StackRenderer.getViewMenuImage() (In reply to Lars Vogel from comment #6) > (In reply to Paul Webster from comment #5) > Thanks. From the other bug it is StackRenderer.getViewMenuImage() With the help from Paul, I was able to replace the currently hand-drawn I asked Tony, if he can create a fitting icon, meanwhile I did already the required code change. The original image was not disposed, I have not change that logic. https://git.eclipse.org/r/23253 Created attachment 240820 [details]
Screenshot
Tony created an icon and I updated the Gerrit review. If there are no objections I commit this tomorrow.
(In reply to Lars Vogel from comment #9) > Created attachment 240820 [details] > Screenshot > > Tony created an icon and I updated the Gerrit review. If there are no > objections I commit this tomorrow. For me it looks great, thanks Daniel Created attachment 240822 [details]
New icon on light background
For comparison, this is the new icon on a light background
(In reply to Daniel Rolka from comment #10) > (In reply to Lars Vogel from comment #9) > > Created attachment 240820 [details] > > Screenshot > > > > Tony created an icon and I updated the Gerrit review. If there are no > > objections I commit this tomorrow. > > For me it looks great, thanks > > Daniel Thanks for the feedback Daniel. Fixed with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=2cc39fc98fc6e7e4901c82cbeaab40ac60cde46b Lars, this doesn't look good on Windows 7 (too dark, not using the color scheme of min/max buttons) and the horizontal edge of the triangle is no longer horizontally aligned with the minimize and maximize button's top. (In reply to Dani Megert from comment #13) > Lars, this doesn't look good on Windows 7 (too dark, not using the color > scheme of min/max buttons) and the horizontal edge of the triangle is no > longer horizontally aligned with the minimize and maximize button's top. Thanks. Tony can you update the icon? Can we get a screenshot of the issue? I'm looking at the previous icon in Windows 7 and it looks wonky as well. Created attachment 241367 [details]
Picture with correct colors and alignment
(In reply to Dani Megert from comment #16) > Created attachment 241367 [details] > Picture with correct colors and alignment Any update on this one? (In reply to Dani Megert from comment #17) > (In reply to Dani Megert from comment #16) > > Created attachment 241367 [details] > > Picture with correct colors and alignment > > Any update on this one? I think the original color was SWT.COLOR_WIDGET_DARK_SHADOW. Where do I see the real color behind this constant? I don't think it is 17. public static final int COLOR_WIDGET_DARK_SHADOW = 17; (In reply to Lars Vogel from comment #18) > (In reply to Dani Megert from comment #17) > > (In reply to Dani Megert from comment #16) > > > Created attachment 241367 [details] > > > Picture with correct colors and alignment > > > > Any update on this one? > > I think the original color was SWT.COLOR_WIDGET_DARK_SHADOW. Where do I see > the real color behind this constant? I don't think it is 17. > > public static final int COLOR_WIDGET_DARK_SHADOW = 17; The image was computed to look right on all color schemes. To see how it was before, just look at the change ;-). (In reply to Dani Megert from comment #19) > The image was computed to look right on all color schemes. Except on a dark theme, in which is look extremely ugly. In the change I think I see that SWT.COLOR_WIDGET_DARK_SHADOW is used. Can you tell me where I find the mapping to the color for that constant? (In reply to Lars Vogel from comment #20) > (In reply to Dani Megert from comment #19) > > The image was computed to look right on all color schemes. > > Except on a dark theme, in which is look extremely ugly. In the change I > think I see that SWT.COLOR_WIDGET_DARK_SHADOW is used. Can you tell me where > I find the mapping to the color for that constant? This is a system color read in by SWT [1]. That's the point of it. So, if in the dark scheme that color is the same as the background, then it's really a bug in the OS color scheme and not Eclipse. [1] org.eclipse.swt.widgets.Display.getSystemColor(int) (In reply to Dani Megert from comment #21) > This is a system color read in by SWT [1]. That's the point of it. So, if in > the dark scheme that color is the same as the background, then it's really a > bug in the OS color scheme and not Eclipse. > > [1] org.eclipse.swt.widgets.Display.getSystemColor(int) Ah, sorry, I was talking about Eclipse CSS themes not system themes. I try to fix the image. On Windows 7 with Kepler (Build id: 20130614-0229), looks good to me. See screenshots. With Luna Integration Build (Build id: I20140402-0100) there is a black border. (The 'Close' icon in CTabFolder instead, looks ugly on any OS/platform/release) Created attachment 241687 [details]
CTabFolder icons on Kepler/Win7
Created attachment 241688 [details]
CTabFolder icons on Luna/Win7
(In reply to Dani Megert from comment #13) > and the horizontal edge of the triangle is no > longer horizontally aligned with the minimize and maximize button's top. OK, that part already got broken a little bit in 4.0 and then even worse in later builds. But still something that needs to be fixed. Ping! I've reverted the change and also fixed the wrong horizontal alignment. (In reply to Dani Megert from comment #28) > I've reverted the change and also fixed the wrong horizontal alignment. I though I still had today for this change? ;-) Thanks for the revert, I would potentially not be able to fix this tonight. The menu icon should not be replaced with a pre-computed image. It should look the same as the Minimize and Maximize buttons, and as long a those are drawn in org.eclipse.swt.custom.CTabFolderRenderer#drawMinimize(GC, Rectangle, int), the menu button should not be rendered differently. (In reply to Markus Keller from comment #30) > The menu icon should not be replaced with a pre-computed image. It should > look the same as the Minimize and Maximize buttons, and as long a those are > drawn in org.eclipse.swt.custom.CTabFolderRenderer#drawMinimize(GC, > Rectangle, int), the menu button should not be rendered differently. I agree that we should not replace it with a png but the drawing should be still improved. Compared to the minimize / maximize it looks fuzzy. (In reply to Lars Vogel from comment #31) > (In reply to Markus Keller from comment #30) > > The menu icon should not be replaced with a pre-computed image. It should > > look the same as the Minimize and Maximize buttons, and as long a those are > > drawn in org.eclipse.swt.custom.CTabFolderRenderer#drawMinimize(GC, > > Rectangle, int), the menu button should not be rendered differently. As we are planning to support HDPI, I think replacing the view icon and the minimize and maximize custom drawing with images will help in having a consistent experience. Mass move to 4.7 as M7 is approaching. Please move back in case you are planning to fix it for Neon. (In reply to Markus Keller from comment #30) > The menu icon should not be replaced with a pre-computed image. It should > look the same as the Minimize and Maximize buttons, and as long a those are > drawn in org.eclipse.swt.custom.CTabFolderRenderer#drawMinimize(GC, > Rectangle, int), the menu button should not be rendered differently. Opened Bug 495033 for SWT. *** This bug has been marked as a duplicate of bug 466511 *** |