Bug 568740 - [Win32] TextLayout renders underscore, strikeout and border
only on last line
Each of the three styles underscore, strikeout and border are rendered
separately in there own method. TextLayout implementation for windows
tries to optimize rendering of those styles by drawing adjacent styles
in one a single call. (for border it also looks much better)
It will test if two adjacent style parts (separate for underscore,
strikeout and border) are 'adherent' which seem to mean they look the
same. This can happen for once if the user supplies suboptimal styles,
i.e. the same or an equal style instance for adjacent ranges instead of
one style for the whole range at once. But even if adjacent styles are
different it will still try to optimize rendering if a specific substyle
is equal, e.g. two adjacent styles got different fonts but both use
underline with the same color. In this case it will draw the underline
in one call for both ranges.
Now for the failing case. If a style expands over more than one line
TextLayout will internally split those styles on line delimiter because
it renders per line. This will include a separate StyleItem for the line
delimiter to mark it as lineBreak which inherits the style of the range
before. An example. The two lines content "abc\n123" with an underline
over the hole range produce internally three StyleItems all with same
style.
While drawing the first line it will see underline for the "abc" part
but postpone rendering because the adjacent style for "\n" has the same
underline style. But line breaks are rendered differently so the
postponed underline drawing will never happen.
Change-Id: Ic19f13a559c79d148f62fc7944101dd59ae17dde
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java
index 8aaa76c..2a13c54 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/TextLayout.java
@@ -813,7 +813,7 @@
if (style.borderStyle == SWT.NONE) return null;
clipRect = addClipRect(run, clipRect, pRect, selectionStart, selectionEnd);
boolean lastRunVisible = drawClip != null && (x + run.x + run.width) > (drawClip.x + drawClip.width);
- if (index + 1 >= line.length || lastRunVisible || !style.isAdherentBorder(line[index + 1].style)) {
+ if (index + 1 >= line.length || lastRunVisible || line[index + 1].lineBreak || !style.isAdherentBorder(line[index + 1].style)) {
int left = run.x;
int start = run.start;
int end = run.start + run.length - 1;
@@ -900,7 +900,7 @@
if (style.borderStyle == SWT.NONE) return null;
clipRect = addClipRect(run, clipRect, pRect, selectionStart, selectionEnd);
boolean lastRunVisible = drawClip != null && (x + run.x + run.width) > (drawClip.x + drawClip.width);
- if (index + 1 >= line.length || lastRunVisible || !style.isAdherentBorder(line[index + 1].style)) {
+ if (index + 1 >= line.length || lastRunVisible || line[index + 1].lineBreak || !style.isAdherentBorder(line[index + 1].style)) {
int left = run.x;
int start = run.start;
int end = run.start + run.length - 1;
@@ -1175,7 +1175,7 @@
if (!style.strikeout) return null;
clipRect = addClipRect(run, clipRect, pRect, selectionStart, selectionEnd);
boolean lastRunVisible = drawClip != null && (x + run.x + run.width) > (drawClip.x + drawClip.width);
- if (index + 1 >= line.length || lastRunVisible || !style.isAdherentStrikeout(line[index + 1].style)) {
+ if (index + 1 >= line.length || lastRunVisible || line[index + 1].lineBreak || !style.isAdherentStrikeout(line[index + 1].style)) {
int left = run.x;
int start = run.start;
int end = run.start + run.length - 1;
@@ -1225,7 +1225,7 @@
if (!style.strikeout) return null;
clipRect = addClipRect(run, clipRect, pRect, selectionStart, selectionEnd);
boolean lastRunVisible = drawClip != null && (x + run.x + run.width) > (drawClip.x + drawClip.width);
- if (index + 1 >= line.length || lastRunVisible || !style.isAdherentStrikeout(line[index + 1].style)) {
+ if (index + 1 >= line.length || lastRunVisible || line[index + 1].lineBreak || !style.isAdherentStrikeout(line[index + 1].style)) {
int left = run.x;
int start = run.start;
int end = run.start + run.length - 1;
@@ -1283,7 +1283,7 @@
if (!style.underline) return null;
clipRect = addClipRect(run, clipRect, pRect, selectionStart, selectionEnd);
boolean lastRunVisible = drawClip != null && (x + run.x + run.width) > (drawClip.x + drawClip.width);
- if (index + 1 >= line.length || lastRunVisible || !style.isAdherentUnderline(line[index + 1].style)) {
+ if (index + 1 >= line.length || lastRunVisible || line[index + 1].lineBreak || !style.isAdherentUnderline(line[index + 1].style)) {
int left = run.x;
int start = run.start;
int end = run.start + run.length - 1;
@@ -1414,7 +1414,7 @@
if (!style.underline) return null;
clipRect = addClipRect(run, clipRect, pRect, selectionStart, selectionEnd);
boolean lastRunVisible = drawClip != null && (x + run.x + run.width) > (drawClip.x + drawClip.width);
- if (index + 1 >= line.length || lastRunVisible || !style.isAdherentUnderline(line[index + 1].style)) {
+ if (index + 1 >= line.length || lastRunVisible || line[index + 1].lineBreak || !style.isAdherentUnderline(line[index + 1].style)) {
int left = run.x;
int start = run.start;
int end = run.start + run.length - 1;
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
index de3d726..0175f14 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
@@ -24,6 +24,13 @@
import org.eclipse.swt.SWTException;
import org.eclipse.swt.events.PaintEvent;
import org.eclipse.swt.events.PaintListener;
+import org.eclipse.swt.graphics.Color;
+import org.eclipse.swt.graphics.GC;
+import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.graphics.ImageData;
+import org.eclipse.swt.graphics.RGB;
+import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
@@ -157,4 +164,70 @@
}
}
+/**
+ * Check if widget contains the given color.
+ *
+ * @param control widget to check
+ * @param expectedColor color to find
+ * @return <code>true</code> if the given color was found in current text widget
+ * bounds
+ */
+public static boolean hasPixel(Control control, Color expectedColor) {
+ return hasPixel(control, expectedColor, null);
+}
+
+/**
+ * Check if widget contains the given color in given bounds. The effective
+ * search range to find the color is the union of current widget bounds and
+ * given rectangle.
+ *
+ * @param control widget to check
+ * @param expectedColor color to find
+ * @param rect the bounds where the color is searched in. Can overlap
+ * the control bounds or <code>null</code> to check the
+ * widgets full bounds.
+ * @return <code>true</code> if the given color was found in search range of
+ * widget
+ */
+public static boolean hasPixel(Control control, Color expectedColor, Rectangle rect) {
+ GC gc = new GC(control);
+ final Image image = new Image(control.getDisplay(), control.getSize().x, control.getSize().y);
+ gc.copyArea(image, 0, 0);
+ gc.dispose();
+ boolean result = hasPixel(image, expectedColor, rect);
+ image.dispose();
+ return result;
+}
+
+/**
+ * Check if image contains the given color in given bounds. The effective search
+ * range to find the color is the union of image size and given rectangle.
+ *
+ * @param image image to check for the expected color
+ * @param expectedColor color to find
+ * @param rect the bounds where the color is searched in. Can overlap
+ * the image bounds or <code>null</code> to check the whole
+ * image.
+ * @return <code>true</code> if the given color was found in search range of
+ * image
+ */
+public static boolean hasPixel(Image image, Color expectedColor, Rectangle rect) {
+ ImageData imageData = image.getImageData();
+ if (rect == null) {
+ rect = new Rectangle(0, 0, image.getBounds().width, image.getBounds().height);
+ }
+ RGB expectedRGB = expectedColor.getRGB();
+ int xEnd = rect.x + rect.width;
+ int yEnd = rect.y + rect.height;
+ for (int x = Math.max(rect.x, 1); x < xEnd && x < image.getBounds().width - 1; x++) { // ignore first and last columns
+ for (int y = rect.y; y < yEnd && y < image.getBounds().height; y++) {
+ RGB pixelRGB = imageData.palette.getRGB(imageData.getPixel(x, y));
+ if (expectedRGB.equals(pixelRGB)) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
}
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_StyledText.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_StyledText.java
index f890c6a..0c72fb2 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_StyledText.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_StyledText.java
@@ -60,10 +60,7 @@
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.Font;
import org.eclipse.swt.graphics.FontData;
-import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.GlyphMetrics;
-import org.eclipse.swt.graphics.Image;
-import org.eclipse.swt.graphics.ImageData;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.graphics.Rectangle;
@@ -5467,54 +5464,12 @@
assertFalse(hasPixel(text, colorForVariableHeight));
}
-/**
- * Check if StyledText widget contains the given color.
- *
- * @param text widget to check
- * @param expectedColor color to find
- * @return <code>true</code> if the given color was found in current text widget
- * bounds
- */
private boolean hasPixel(StyledText text, Color expectedColor) {
- return hasPixel(text, expectedColor, null);
+ return SwtTestUtil.hasPixel(text, expectedColor);
}
-/**
- * Check if StyledText widget contains the given color in given bounds. The
- * effective search range to find the color is the union of current widget
- * bounds and given rectangle.
- *
- * @param text widget to check
- * @param expectedColor color to find
- * @param rect the bounds where the color is searched in. Can overlap
- * the text widget bounds or <code>null</code> to check the
- * widgets full bounds.
- * @return <code>true</code> if the given color was found in search range of
- * text widget
- */
private boolean hasPixel(StyledText text, Color expectedColor, Rectangle rect) {
- GC gc = new GC(text);
- final Image image = new Image(text.getDisplay(), text.getSize().x, text.getSize().y);
- gc.copyArea(image, 0, 0);
- gc.dispose();
- ImageData imageData = image.getImageData();
- if (rect == null) {
- rect = new Rectangle(0, 0, image.getBounds().width, image.getBounds().height);
- }
- RGB expectedRGB = expectedColor.getRGB();
- int xEnd = rect.x + rect.width;
- int yEnd = rect.y + rect.height;
- for (int x = Math.max(rect.x, 1); x < xEnd && x < image.getBounds().width - 1; x++) { // ignore first and last columns
- for (int y = rect.y; y < yEnd && y < image.getBounds().height; y++) {
- RGB pixelRGB = imageData.palette.getRGB(imageData.getPixel(x, y));
- if (expectedRGB.equals(pixelRGB)) {
- image.dispose();
- return true;
- }
- }
- }
- image.dispose();
- return false;
+ return SwtTestUtil.hasPixel(text, expectedColor, rect);
}
private String blockSelectionTestText() {
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java
index ff2b8bf..9f66451 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2015, 2016 IBM Corporation and others.
+ * Copyright (c) 2015, 2020 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -16,10 +16,14 @@
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import org.eclipse.swt.SWT;
+import org.eclipse.swt.graphics.Font;
+import org.eclipse.swt.graphics.GC;
+import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.Rectangle;
import org.eclipse.swt.graphics.TextLayout;
@@ -949,4 +953,75 @@
}
layout.dispose();
}
+
+/**
+ * Bug 568740 - [Win32] TextLayout renders underscore, strikeout and border only on last line
+ */
+@Test
+public void test_bug568740_multilineTextStyle() {
+ Font font = null;
+ Image image = null;
+ GC gc = null;
+ TextLayout layout = null;
+ try {
+ font = new Font(display, SwtTestUtil.testFontName, 16, SWT.NORMAL);
+ image = new Image(display, 200, 100);
+ gc = new GC(image);
+ gc.setBackground(display.getSystemColor(SWT.COLOR_WHITE));
+ gc.fillRectangle(image.getBounds());
+ gc.setAntialias(SWT.OFF); // aa can change colors and break the test in worst case
+
+ layout = new TextLayout(display);
+ layout.setFont(font);
+ layout.setText("first line\nsecond line");
+
+ // The test has one multi-line style containing all the problematic properties
+ // in different colors and a second control style with other colors at the
+ // end of the second line. Searching for the colors anywhere would even work
+ // before the bug was fixed. So we search only in the area of the first line and
+ // if we find any of the control colors we know the search area was calculated
+ // wrong.
+
+ TextStyle style = new TextStyle();
+ style.borderStyle = SWT.BORDER_DOT;
+ style.borderColor = display.getSystemColor(SWT.COLOR_BLUE);
+ style.underline = true;
+ style.underlineColor = display.getSystemColor(SWT.COLOR_GREEN);
+ style.strikeout = true;
+ style.strikeoutColor = display.getSystemColor(SWT.COLOR_RED);
+ layout.setStyle(style, 2, 14);
+
+ TextStyle controlStyle = new TextStyle(style);
+ controlStyle.borderColor = display.getSystemColor(SWT.COLOR_DARK_BLUE);
+ controlStyle.underlineColor = display.getSystemColor(SWT.COLOR_DARK_GREEN);
+ controlStyle.strikeoutColor = display.getSystemColor(SWT.COLOR_DARK_RED);
+ layout.setStyle(controlStyle, 15, 23);
+
+ int offset = 10;
+ layout.draw(gc, offset, offset);
+
+ Rectangle firstLineBounds = layout.getLineBounds(0);
+ Rectangle searchRangeBorder = new Rectangle(0, 0, image.getBounds().width, offset + (int)(firstLineBounds.height * 0.3));
+ Rectangle searchRangeStrike = new Rectangle(0, 0, image.getBounds().width, offset + (int)(firstLineBounds.height * 1.0));
+ Rectangle searchRangeUnder = new Rectangle(0, 0, image.getBounds().width, offset + (int)(firstLineBounds.height * 1.3));
+
+ assertFalse("Invalid test range for border test. Fix test!", SwtTestUtil.hasPixel(image, display.getSystemColor(SWT.COLOR_DARK_BLUE), searchRangeBorder));
+ assertTrue("Found no border style in first line", SwtTestUtil.hasPixel(image, display.getSystemColor(SWT.COLOR_BLUE), searchRangeBorder));
+
+ assertFalse("Invalid test range for strikeout test. Fix test!", SwtTestUtil.hasPixel(image, display.getSystemColor(SWT.COLOR_DARK_RED), searchRangeStrike));
+ assertTrue("Found no strikeout style in first line", SwtTestUtil.hasPixel(image, display.getSystemColor(SWT.COLOR_RED), searchRangeStrike));
+
+ assertFalse("Invalid test range for underline test. Fix test!", SwtTestUtil.hasPixel(image, display.getSystemColor(SWT.COLOR_DARK_GREEN), searchRangeUnder));
+ assertTrue("Found no underline style in first line", SwtTestUtil.hasPixel(image, display.getSystemColor(SWT.COLOR_GREEN), searchRangeUnder));
+ } finally {
+ if (layout != null)
+ layout.dispose();
+ if (gc != null)
+ gc.dispose();
+ if (image != null)
+ image.dispose();
+ if (font != null)
+ font.dispose();
+ }
+}
}
