Bug 536008 - [Dark Theme] Disabled label looks bad in dark theme
This is a preparation patch. Changes:
1) Get rid of `IMAGE_AND_TEXT` that was disabled for 15 years.
Owner draw code for text Labels is preserved for the next patch.
It is also fixed to always draw text on top, as native Label does.
2) Also remove duplicate code in `Label.setAlignment()`.
Change-Id: I74b048d8f12eba13000b5264fdd4549359a61f56
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java
index 96bb548..ce060c9 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java
@@ -55,8 +55,8 @@
public class Label extends Control {
String text = "";
Image image;
+ boolean isImageMode; // Resolves ambiguity when both image and text are set
static final int MARGIN = 4;
- static /*final*/ boolean IMAGE_AND_TEXT = false;
static final long LabelProc;
static final TCHAR LabelClass = new TCHAR (0, "STATIC", true);
static {
@@ -147,22 +147,11 @@
width += border * 2; height += border * 2;
return new Point (width, height);
}
- int bits = OS.GetWindowLong (handle, OS.GWL_STYLE);
- boolean drawText = true;
- boolean drawImage = (bits & OS.SS_OWNERDRAW) == OS.SS_OWNERDRAW;
- if (drawImage) {
- if (image != null) {
- Rectangle rect = image.getBoundsInPixels();
- width += rect.width;
- height += rect.height;
- if (IMAGE_AND_TEXT) {
- if (text.length () != 0) width += MARGIN;
- } else {
- drawText = false;
- }
- }
- }
- if (drawText) {
+ if (isImageMode) {
+ Rectangle rect = image.getBoundsInPixels();
+ width += rect.width;
+ height += rect.height;
+ } else {
long hDC = OS.GetDC (handle);
long newFont = OS.SendMessage (handle, OS.WM_GETFONT, 0, 0);
long oldFont = OS.SelectObject (hDC, newFont);
@@ -324,20 +313,7 @@
if ((alignment & (SWT.LEFT | SWT.RIGHT | SWT.CENTER)) == 0) return;
style &= ~(SWT.LEFT | SWT.RIGHT | SWT.CENTER);
style |= alignment & (SWT.LEFT | SWT.RIGHT | SWT.CENTER);
- int bits = OS.GetWindowLong (handle, OS.GWL_STYLE);
- if ((bits & OS.SS_OWNERDRAW) != OS.SS_OWNERDRAW) {
- bits &= ~(OS.SS_LEFTNOWORDWRAP | OS.SS_CENTER | OS.SS_RIGHT);
- if ((style & SWT.LEFT) != 0) {
- if ((style & SWT.WRAP) != 0) {
- bits |= OS.SS_LEFT;
- } else {
- bits |= OS.SS_LEFTNOWORDWRAP;
- }
- }
- if ((style & SWT.CENTER) != 0) bits |= OS.SS_CENTER;
- if ((style & SWT.RIGHT) != 0) bits |= OS.SS_RIGHT;
- OS.SetWindowLong (handle, OS.GWL_STYLE, bits);
- }
+ updateStyleBits();
OS.InvalidateRect (handle, null, true);
}
@@ -360,7 +336,8 @@
if ((style & SWT.SEPARATOR) != 0) return;
if (image != null && image.isDisposed()) error(SWT.ERROR_INVALID_ARGUMENT);
this.image = image;
- updateStyleBits(image == null);
+ isImageMode = (image != null);
+ updateStyleBits();
OS.InvalidateRect (handle, null, true);
}
@@ -399,7 +376,8 @@
checkWidget ();
if (string == null) error (SWT.ERROR_NULL_ARGUMENT);
if ((style & SWT.SEPARATOR) != 0) return;
- updateStyleBits(true);
+ isImageMode = false;
+ updateStyleBits();
/*
* Feature in Windows. For some reason, SetWindowText() for
* static controls redraws the control, even when the text has
@@ -416,17 +394,18 @@
}
}
-/**
- * Update the control's static style bits to reflect the changed image vs. text
- * situation.
- *
- * @param showText if <code>true</code> set required style bits to render text
- * otherwise to render the image
- */
-void updateStyleBits(boolean showText) {
- if (showText) {
- int oldBits = OS.GetWindowLong (handle, OS.GWL_STYLE), newBits = oldBits;
- newBits &= ~OS.SS_OWNERDRAW;
+void updateStyleBits() {
+ boolean useOwnerDraw = isImageMode;
+
+ int oldBits = OS.GetWindowLong(handle, OS.GWL_STYLE);
+
+ int newBits = oldBits;
+ newBits &= ~OS.SS_OWNERDRAW;
+ newBits &= ~(OS.SS_LEFTNOWORDWRAP | OS.SS_CENTER | OS.SS_RIGHT);
+
+ if (useOwnerDraw) {
+ newBits |= OS.SS_OWNERDRAW;
+ } else {
if ((style & SWT.LEFT) != 0) {
if ((style & SWT.WRAP) != 0) {
newBits |= OS.SS_LEFT;
@@ -436,15 +415,9 @@
}
if ((style & SWT.CENTER) != 0) newBits |= OS.SS_CENTER;
if ((style & SWT.RIGHT) != 0) newBits |= OS.SS_RIGHT;
- if (oldBits != newBits) OS.SetWindowLong (handle, OS.GWL_STYLE, newBits);
- } else {
- int bits = OS.GetWindowLong (handle, OS.GWL_STYLE);
- if ((bits & OS.SS_OWNERDRAW) != OS.SS_OWNERDRAW) {
- bits &= ~(OS.SS_LEFTNOWORDWRAP | OS.SS_CENTER | OS.SS_RIGHT);
- bits |= OS.SS_OWNERDRAW;
- OS.SetWindowLong (handle, OS.GWL_STYLE, bits);
- }
}
+
+ if (oldBits != newBits) OS.SetWindowLong (handle, OS.GWL_STYLE, newBits);
}
@Override
@@ -541,84 +514,79 @@
return result;
}
+void wmDrawChildSeparator(DRAWITEMSTRUCT struct) {
+ if ((style & SWT.SHADOW_NONE) != 0) return;
+
+ RECT rect = new RECT ();
+ int lineWidth = OS.GetSystemMetrics (OS.SM_CXBORDER);
+ int flags = (style & SWT.SHADOW_IN) != 0 ? OS.EDGE_SUNKEN : OS.EDGE_ETCHED;
+ if ((style & SWT.HORIZONTAL) != 0) {
+ int bottom = struct.top + Math.max (lineWidth * 2, (struct.bottom - struct.top) / 2);
+ OS.SetRect (rect, struct.left, struct.top, struct.right, bottom);
+ OS.DrawEdge (struct.hDC, rect, flags, OS.BF_BOTTOM);
+ } else {
+ int right = struct.left + Math.max (lineWidth * 2, (struct.right - struct.left) / 2);
+ OS.SetRect (rect, struct.left, struct.top, right, struct.bottom);
+ OS.DrawEdge (struct.hDC, rect, flags, OS.BF_RIGHT);
+ }
+}
+
+void wmDrawChildImage(DRAWITEMSTRUCT struct) {
+ int width = struct.right - struct.left;
+ int height = struct.bottom - struct.top;
+ if (width == 0 || height == 0) return;
+
+ Rectangle imageRect = image.getBoundsInPixels ();
+
+ int x = 0;
+ if ((style & SWT.CENTER) != 0) {
+ x = Math.max (0, (width - imageRect.width) / 2);
+ } else if ((style & SWT.RIGHT) != 0) {
+ x = width - imageRect.width;
+ }
+
+ GCData data = new GCData();
+ data.device = display;
+ GC gc = GC.win32_new (struct.hDC, data);
+ Image image = getEnabled () ? this.image : new Image (display, this.image, SWT.IMAGE_DISABLE);
+ gc.drawImage (image, DPIUtil.autoScaleDown(x), DPIUtil.autoScaleDown(Math.max (0, (height - imageRect.height) / 2)));
+ if (image != this.image) image.dispose ();
+ gc.dispose ();
+}
+
+void wmDrawChildText(DRAWITEMSTRUCT struct) {
+ int width = struct.right - struct.left;
+ int height = struct.bottom - struct.top;
+ if (width == 0 || height == 0) return;
+
+ RECT rect = new RECT ();
+ rect.left = struct.left;
+ rect.top = struct.top;
+ rect.right = struct.right;
+ rect.bottom = struct.bottom;
+
+ int flags = OS.DT_EDITCONTROL | OS.DT_EXPANDTABS;
+ if ((style & SWT.LEFT) != 0) flags |= OS.DT_LEFT;
+ if ((style & SWT.CENTER) != 0) flags |= OS.DT_CENTER;
+ if ((style & SWT.RIGHT) != 0) flags |= OS.DT_RIGHT;
+ if ((style & SWT.WRAP) != 0) flags |= OS.DT_WORDBREAK;
+
+ char [] buffer = text.toCharArray ();
+ OS.DrawText (struct.hDC, buffer, buffer.length, rect, flags);
+}
+
@Override
LRESULT wmDrawChild (long wParam, long lParam) {
DRAWITEMSTRUCT struct = new DRAWITEMSTRUCT ();
OS.MoveMemory (struct, lParam, DRAWITEMSTRUCT.sizeof);
drawBackground (struct.hDC);
- if ((style & SWT.SEPARATOR) != 0) {
- if ((style & SWT.SHADOW_NONE) != 0) return null;
- RECT rect = new RECT ();
- int lineWidth = OS.GetSystemMetrics (OS.SM_CXBORDER);
- int flags = (style & SWT.SHADOW_IN) != 0 ? OS.EDGE_SUNKEN : OS.EDGE_ETCHED;
- if ((style & SWT.HORIZONTAL) != 0) {
- int bottom = struct.top + Math.max (lineWidth * 2, (struct.bottom - struct.top) / 2);
- OS.SetRect (rect, struct.left, struct.top, struct.right, bottom);
- OS.DrawEdge (struct.hDC, rect, flags, OS.BF_BOTTOM);
- } else {
- int right = struct.left + Math.max (lineWidth * 2, (struct.right - struct.left) / 2);
- OS.SetRect (rect, struct.left, struct.top, right, struct.bottom);
- OS.DrawEdge (struct.hDC, rect, flags, OS.BF_RIGHT);
- }
- } else {
- int width = struct.right - struct.left;
- int height = struct.bottom - struct.top;
- if (width != 0 && height != 0) {
- boolean drawImage = image != null;
- boolean drawText = IMAGE_AND_TEXT && text.length () != 0;
- int margin = drawText && drawImage ? MARGIN : 0;
- int imageWidth = 0, imageHeight = 0;
- if (drawImage) {
- Rectangle rect = image.getBoundsInPixels ();
- imageWidth = rect.width;
- imageHeight = rect.height;
- }
- RECT rect = null;
- char [] buffer = null;
- int textWidth = 0, textHeight = 0, flags = 0;
- if (drawText) {
- rect = new RECT ();
- flags = OS.DT_CALCRECT | OS.DT_EDITCONTROL | OS.DT_EXPANDTABS;
- if ((style & SWT.LEFT) != 0) flags |= OS.DT_LEFT;
- if ((style & SWT.CENTER) != 0) flags |= OS.DT_CENTER;
- if ((style & SWT.RIGHT) != 0) flags |= OS.DT_RIGHT;
- if ((style & SWT.WRAP) != 0) {
- flags |= OS.DT_WORDBREAK;
- rect.right = Math.max (0, width - imageWidth - margin);
- }
- buffer = text.toCharArray ();
- OS.DrawText (struct.hDC, buffer, buffer.length, rect, flags);
- textWidth = rect.right - rect.left;
- textHeight = rect.bottom - rect.top;
- }
- int x = 0;
- if ((style & SWT.CENTER) != 0) {
- x = Math.max (0, (width - imageWidth - textWidth - margin) / 2);
- } else {
- if ((style & SWT.RIGHT) != 0) {
- x = width - imageWidth - textWidth - margin;
- }
- }
- if (drawImage) {
- GCData data = new GCData();
- data.device = display;
- GC gc = GC.win32_new (struct.hDC, data);
- Image image = getEnabled () ? this.image : new Image (display, this.image, SWT.IMAGE_DISABLE);
- gc.drawImage (image, DPIUtil.autoScaleDown(x), DPIUtil.autoScaleDown(Math.max (0, (height - imageHeight) / 2)));
- if (image != this.image) image.dispose ();
- gc.dispose ();
- x += imageWidth + margin;
- }
- if (drawText) {
- flags &= ~OS.DT_CALCRECT;
- rect.left = x;
- rect.right += rect.left;
- rect.top = Math.max (0, (height - textHeight) / 2);
- rect.bottom += rect.top;
- OS.DrawText (struct.hDC, buffer, buffer.length, rect, flags);
- }
- }
- }
+ if ((style & SWT.SEPARATOR) != 0)
+ wmDrawChildSeparator(struct);
+ else if (isImageMode)
+ wmDrawChildImage(struct);
+ else
+ wmDrawChildText(struct);
+
return null;
}
diff --git a/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug536008_DarkDisabledLabel.java b/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug536008_DarkDisabledLabel.java
new file mode 100644
index 0000000..0cb65e8
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug536008_DarkDisabledLabel.java
@@ -0,0 +1,252 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Syntevo and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Syntevo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.win32.snippets;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.graphics.*;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.layout.FormData;
+import org.eclipse.swt.layout.FormLayout;
+import org.eclipse.swt.layout.RowLayout;
+import org.eclipse.swt.widgets.*;
+
+import java.util.function.Consumer;
+
+public class Bug536008_DarkDisabledLabel {
+ static void moveControl(Control control, Point point) {
+ Point size = control.getSize();
+ control.setBounds(point.x, point.y, size.x, size.y);
+ }
+
+ static void setColors(Control control, Color backColor, Color foreColor) {
+ control.setBackground(backColor);
+ control.setForeground(foreColor);
+
+ if (control instanceof Composite) {
+ for (Control child : ((Composite)control).getChildren()) {
+ setColors(child, backColor, foreColor);
+ }
+ }
+ }
+
+ static void addSelectionListener(Button button, Label label1, Label label2, Consumer<Label> handler, boolean isDefault) {
+ Runnable runnable = () -> {
+ handler.accept(label1);
+ handler.accept(label2);
+
+ label2.setSize(label2.computeSize(SWT.DEFAULT, SWT.DEFAULT));
+ };
+
+ button.addListener(SWT.Selection, e -> {
+ if (!((Button)e.widget).getSelection ()) return;
+ runnable.run();
+ });
+
+ if (isDefault) {
+ button.setSelection(true);
+ runnable.run();
+ }
+ }
+
+ public static void main (String [] args) {
+ Display display = new Display ();
+ Shell shell = new Shell (display);
+ shell.setLayout(new RowLayout(SWT.VERTICAL));
+
+ Color backColor = new Color(display, 0x30, 0x30, 0x30);
+ Color foreColor = new Color(display, 0xD0, 0xD0, 0xD0);
+
+ int margin = 10;
+ int cx = 60;
+ int cy = 60;
+ int nRow = 0;
+
+ Image image = new Image(display, cx / 2, cy / 2);
+ {
+ GC gc = new GC(image);
+ gc.setBackground(backColor);
+ gc.setForeground(foreColor);
+
+ Rectangle imageBounds = image.getBounds();
+ gc.fillRectangle(imageBounds);
+ gc.drawOval(imageBounds.x, imageBounds.y, imageBounds.width - 1, imageBounds.height - 1);
+ }
+
+ FillLayout fillLayout = new FillLayout();
+ fillLayout.marginWidth = 10;
+ fillLayout.marginHeight = 10;
+
+ Group group = new Group(shell, SWT.NONE);
+ {
+ group.setText("Various Label styles && states");
+ group.setLayout(fillLayout);
+
+ Composite labelComposite = new Composite(group, SWT.NONE);
+
+ for (int style : new int[] {SWT.LEFT, SWT.CENTER, SWT.RIGHT})
+ {
+ int nCol = 0;
+
+ for (int test1 = 0; test1 < 5; test1++)
+ for (int test2 = 0; test2 < 2; test2++)
+ for (int test3 = 0; test3 < 2; test3++)
+ {
+ Label label = new Label(labelComposite, SWT.BORDER | style);
+
+ switch (test1) {
+ case 0:
+ label.setText("&Label");
+ break;
+ case 1:
+ label.setText("&Mul-\nlabel");
+ break;
+ case 2:
+ label.setImage(image);
+ break;
+ case 3:
+ label.setText("L&abel");
+ label.setImage(image);
+ break;
+ case 4:
+ label.setImage(image);
+ label.setText("La&bel");
+ break;
+ default:
+ throw new RuntimeException("Unknown test");
+ }
+
+ switch (test2) {
+ case 0:
+ label.setEnabled(false);
+ break;
+ case 1:
+ break;
+ default:
+ throw new RuntimeException("Unknown test");
+ }
+
+ switch (test3) {
+ case 0:
+ label.setSize(cx, cy);
+ break;
+ case 1:
+ label.setSize(label.computeSize(SWT.DEFAULT, SWT.DEFAULT));
+ break;
+ default:
+ throw new RuntimeException("Unknown test");
+ }
+
+ moveControl(label, new Point(nCol*(cx+margin), nRow*(cy+margin)));
+ nCol++;
+ }
+
+ nRow++;
+ }
+ }
+
+ group = new Group(shell, SWT.NONE);
+ {
+ group.setText("Label playground");
+ group.setLayout(new RowLayout());
+
+ Composite labelComposite1 = new Composite(group, SWT.NONE);
+ labelComposite1.setLayout(new FormLayout());
+ Label label1 = new Label(labelComposite1, SWT.BORDER);
+ label1.setLayoutData(new FormData(cx, cy));
+
+ Composite labelComposite2 = new Composite(group, SWT.NONE);
+ labelComposite2.setLayout(new FormLayout());
+ Composite labelComposite2a = new Composite(labelComposite2, SWT.NONE);
+ labelComposite2a.setLayoutData(new FormData(cx, cy));
+ Label label2 = new Label(labelComposite2a, SWT.BORDER);
+ moveControl(label2, new Point(0, 0));
+
+ Composite subgroup = new Composite(group, SWT.NONE);
+ {
+ subgroup.setLayout(new RowLayout(SWT.VERTICAL));
+
+ Button button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setText(text)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setText("Lab&el");
+ }, true);
+
+ button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setImage(image)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setImage(image);
+ }, false);
+
+ button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setImage(null)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setImage(null);
+ }, false);
+ }
+
+ subgroup = new Composite(group, SWT.NONE);
+ {
+ subgroup.setLayout(new RowLayout(SWT.VERTICAL));
+
+ Button button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setEnabled(true)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setEnabled(true);
+ }, true);
+
+ button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setEnabled(false)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setEnabled(false);
+ }, false);
+ }
+
+ subgroup = new Composite(group, SWT.NONE);
+ {
+ subgroup.setLayout(new RowLayout(SWT.VERTICAL));
+
+ Button button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setAlignment(SWT.LEFT)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setAlignment(SWT.LEFT);
+ }, true);
+
+ button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setAlignment(SWT.CENTER)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setAlignment(SWT.CENTER);
+ }, false);
+
+ button = new Button(subgroup, SWT.RADIO);
+ button.setText("Label.setAlignment(SWT.RIGHT)");
+ addSelectionListener(button, label1, label2, label -> {
+ label.setAlignment(SWT.RIGHT);
+ }, false);
+ }
+ }
+
+ // Set shell colors
+ setColors(shell, backColor, foreColor);
+
+ // Pack and show shell
+ shell.pack();
+ shell.open();
+
+ while (!shell.isDisposed()) {
+ if (!display.readAndDispatch ()) display.sleep ();
+ }
+
+ display.dispose ();
+ }
+}
