Bug 551335 - [StyledText] setStyleRanges reset less cache than necessary

Change-Id: I91f249646228a9046c2ec9ece9e6607583208b17
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>

2 files changed, 123 insertions, 7 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/StyledText.java b/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/StyledText.java
index 3d9ab72d8e..e2a9774295 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/StyledText.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/StyledText.java
@@ -10369,9 +10369,9 @@ private SortedSet<Integer> computeModifiedLines(int[] referenceRanges, StyleRang
 		StyleRange referenceStyleAtCurrentOffset = defaultStyle;
 		if (isInRange(referenceRanges, referenceRangeIndex, currentOffset)) { // has styling
 			referenceStyleAtCurrentOffset = referenceStyles[referenceRangeIndex];
-			nextMilestoneOffset =  Math.min(nextMilestoneOffset, endRangeOffset(referenceRanges, referenceRangeIndex));
-		} else if (referenceRangeIndex + 1 < referenceStyles.length) { // no range, default styling
-			nextMilestoneOffset = referenceRanges[2 * (referenceRangeIndex + 1)]; // beginning of next range
+			nextMilestoneOffset =  endRangeOffset(referenceRanges, referenceRangeIndex);
+		} else if (referenceRangeIndex < referenceStyles.length) { // no range, default styling
+			nextMilestoneOffset = referenceRanges[2 * referenceRangeIndex]; // beginning of next range
 		}
 
 		while (newRangeIndex < newStyles.length && endRangeOffset(newRanges, newRangeIndex) <= currentOffset) {
@@ -10381,8 +10381,8 @@ private SortedSet<Integer> computeModifiedLines(int[] referenceRanges, StyleRang
 		if (isInRange(newRanges, newRangeIndex, currentOffset)) {
 			newStyleAtCurrentOffset = newStyles[newRangeIndex];
 			nextMilestoneOffset = Math.min(nextMilestoneOffset, endRangeOffset(newRanges, newRangeIndex));
-		} else if (newRangeIndex + 1 < newStyles.length) {
-			nextMilestoneOffset = newRanges[2 * (newRangeIndex + 1)];
+		} else if (newRangeIndex < newStyles.length) {
+			nextMilestoneOffset = Math.min(nextMilestoneOffset, newRanges[2 * newRangeIndex]);
 		}
 
 		if (!referenceStyleAtCurrentOffset.similarTo(newStyleAtCurrentOffset)) {
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 897dbf680f..21dadb186b 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
@@ -5420,15 +5420,45 @@ private void testLineStyleListener(String content, LineStyleListener listener, B
 	}
 }
 
+/**
+ * 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);
+}
+
+/**
+ * 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();
-	for (int x = 1; x < image.getBounds().width - 1; x++) { // ignore first and last columns
-		for (int y = 0; y < image.getBounds().height; y++) {
+	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();
@@ -5610,4 +5640,90 @@ public void test_InsertWhenDisabled() {
 	assertEquals("Wrong insert in disabled StyledText", expectedText, actualText);
 }
 
+/**
+ * Test for:
+ * Bug 551335 - [StyledText] setStyleRanges reset less cache than necessary
+ */
+@Test
+public void test_bug551335_lostStyles() throws InterruptedException {
+	Assume.assumeFalse("Bug 536588 prevents test to work on Mac", SwtTestUtil.isCocoa);
+	shell.setVisible(true);
+	text.setText("012345678\n012345678\n0123456789");
+	text.setMargins(0, 0, 0, 0);
+	text.pack();
+	shell.pack();
+
+	StyleRange style = new StyleRange();
+	style.background = getColor(BLUE);
+	style.length = 3;
+
+	StyleRange[] styles = new StyleRange[3];
+	style.start = 3;
+	styles[0] = (StyleRange) style.clone();
+	style.start = 13;
+	styles[1] =  (StyleRange) style.clone();
+	style.start = 23;
+	styles[2] =  (StyleRange) style.clone();
+
+	int lineHeight = text.getBounds().height / text.getLineCount();
+	Rectangle[] testAreas = new Rectangle[text.getLineCount()];
+	for (int i = 0; i < testAreas.length; i++) {
+		// the test area is a small horizontal line approximate in the middle of the line
+		// the concrete width doesn't matter because it is clipped to the actual line width
+		testAreas[i] = new Rectangle(0, (int)((i + 0.48) * lineHeight), 1000, 3);
+	}
+	BooleanSupplier testAllLinesStyled = () -> {
+		for (Rectangle testArea : testAreas) {
+			if (!hasPixel(text, getColor(BLUE), testArea)) {
+				return false;
+			}
+		}
+		return true;
+	};
+	BooleanSupplier testUnstyledText = () -> {
+		for (Rectangle testArea : testAreas) {
+			if (hasPixel(text, getColor(BLUE), testArea)) {
+				return false;
+			}
+		}
+		return true;
+	};
+
+	// 1. apply style one by one
+	for (StyleRange s : styles) {
+		text.setStyleRange(s);
+	}
+	processEvents(1000, testAllLinesStyled);
+	assertTrue(testAllLinesStyled.getAsBoolean());
+
+	text.setStyleRange(new StyleRange(0, text.getCharCount(), null, null)); // reset style
+	processEvents(1000, testUnstyledText);
+	assertTrue(testUnstyledText.getAsBoolean());
+
+	// 2. apply styles as array
+	text.setStyleRanges(styles);
+	processEvents(1000, testAllLinesStyled);
+	assertTrue(testAllLinesStyled.getAsBoolean());
+
+	text.setStyleRange(new StyleRange(0, text.getCharCount(), null, null)); // reset style
+	processEvents(1000, testUnstyledText);
+	assertTrue(testUnstyledText.getAsBoolean());
+
+	// 3. apply styles using ranges
+	int[] ranges = new int[styles.length * 2];
+	style.start = style.length = 0;
+	for (int i = 0; i < styles.length; i++) {
+		ranges[i*2] =   styles[i].start;
+		ranges[i*2 + 1] = styles[i].length;
+		styles[i] = style;
+	}
+	text.setStyleRanges(ranges, styles);
+	processEvents(1000, testAllLinesStyled);
+	assertTrue(testAllLinesStyled.getAsBoolean());
+
+	text.setStyleRange(new StyleRange(0, text.getCharCount(), null, null)); // reset style
+	processEvents(1000, testUnstyledText);
+	assertTrue(testUnstyledText.getAsBoolean());
+}
+
 }