Bug 551588: [GTK] Performance of TextLayout.getBounds() is very

bad for long text

Avoid the fetching of many PangoLayoutIter* objects by re-using
them where possible. Refactored and optimized some of the TextLayout
getLineBounds() and getBounds() methods to avoid unnecessary native
calls and slowdowns. Return to pre-bug 491712 behaviour by using
the height returned by pango_layout_get_size().

Tested with the snippets attached on Fedora 30, GTK3.24, and X11.
No ill effects observed in the IDE, and no AllNonBrowser JUnit tests
fail.

Change-Id: I6feb11e6cf7515508f4ef1a155256c2987a83613
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Also-by: Peter Severin <peter@wireframesketcher.com>
Also-by: Eric Williams <ericwill@redhat.com>

4 files changed, 266 insertions, 13 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java
index fabec60987..5965596aaa 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java
@@ -743,17 +743,11 @@ public int getAscent () {
  * @see #getLineBounds(int)
  */
 public Rectangle getBounds() {
-	checkLayout();
-	Rectangle bounds = DPIUtil.autoScaleDown(getDevice(), getBoundsInPixels());
-	int lineCount = OS.pango_layout_get_line_count(layout);
-	int totalLineheight = getScaledVerticalIndent();
-	for (int i = 0; i < lineCount; i++) {
-		totalLineheight += this.getLineBounds(i).height + OS.PANGO_PIXELS(OS.pango_layout_get_spacing(layout));
-	}
-	bounds.height = totalLineheight;
-	return bounds;
+	int spacingInPixels = getSpacingInPixels();
+	return DPIUtil.autoScaleDown(getDevice(), getBoundsInPixels(spacingInPixels));
 }
-Rectangle getBoundsInPixels() {
+
+Rectangle getBoundsInPixels(int spacingInPixels) {
 	checkLayout();
 	computeRuns();
 	int[] w = new int[1], h = new int[1];
@@ -765,7 +759,7 @@ Rectangle getBoundsInPixels() {
 	if (ascentInPoints != -1 && descentInPoints != -1) {
 		height = Math.max (height, DPIUtil.autoScaleUp(getDevice(), ascentInPoints + descentInPoints));
 	}
-	height += OS.PANGO_PIXELS(OS.pango_layout_get_spacing(layout));
+	height += spacingInPixels;
 	return new Rectangle(0, 0, width, height + getScaledVerticalIndent());
 }
 
@@ -981,11 +975,18 @@ Rectangle getLineBoundsInPixels(int lineIndex) {
 	int lineCount = OS.pango_layout_get_line_count(layout);
 	if (!(0 <= lineIndex && lineIndex < lineCount)) SWT.error(SWT.ERROR_INVALID_RANGE);
 	long iter = OS.pango_layout_get_iter(layout);
+	for (int i = 0; i < lineIndex; i++) {
+		OS.pango_layout_iter_next_line(iter);
+	}
+	Rectangle lineBoundsInPixels = getLineBoundsInPixels(lineIndex, iter);
+	OS.pango_layout_iter_free(iter);
+	return lineBoundsInPixels;
+}
+
+private Rectangle getLineBoundsInPixels(int lineIndex, long iter) {
 	if (iter == 0) SWT.error(SWT.ERROR_NO_HANDLES);
-	for (int i = 0; i < lineIndex; i++) OS.pango_layout_iter_next_line(iter);
 	PangoRectangle rect = new PangoRectangle();
 	OS.pango_layout_iter_get_line_extents(iter, null, rect);
-	OS.pango_layout_iter_free(iter);
 	int x = OS.PANGO_PIXELS(rect.x);
 	int y = OS.PANGO_PIXELS(rect.y);
 	int width = OS.PANGO_PIXELS(rect.width);
diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutBenchmark.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutBenchmark.java
new file mode 100644
index 0000000000..c89d468c0e
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutBenchmark.java
@@ -0,0 +1,50 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Peter Severin 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:
+ *     Peter Severin - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.gtk.snippets;
+
+import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.graphics.TextLayout;
+import org.eclipse.swt.widgets.Display;
+
+public class Bug551588_TextLayoutBenchmark {
+	final static String longString;
+
+	static {
+		String loremIpsum = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.";
+
+		StringBuilder b = new StringBuilder();
+		for (int i = 0; i < 10; i++) {
+			b.append(loremIpsum);
+		}
+		longString = b.toString();
+	}
+
+	public static void main(String[] args) {
+		Display display = new Display();
+
+		long start = System.currentTimeMillis();
+		TextLayout textLayout = new TextLayout(display);
+		textLayout.setText(longString);
+		textLayout.setWidth(1);
+		Rectangle textBounds = textLayout.getBounds();
+		long end = System.currentTimeMillis();
+
+		System.out.println("Time: " + (end - start) + "ms");
+		System.out.println("Text bounds: " + textBounds);
+
+		textLayout.dispose();
+
+		display.dispose();
+	}
+}
\ No newline at end of file
diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutExample.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutExample.java
new file mode 100644
index 0000000000..bed67ced9d
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutExample.java
@@ -0,0 +1,152 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Peter Severin 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:
+ *     Peter Severin - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.gtk.snippets;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.ScrolledComposite;
+import org.eclipse.swt.graphics.GC;
+import org.eclipse.swt.graphics.Point;
+import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.graphics.TextLayout;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.widgets.Canvas;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Event;
+import org.eclipse.swt.widgets.Listener;
+import org.eclipse.swt.widgets.Shell;
+
+public class Bug551588_TextLayoutExample {
+	final static String longString;
+
+	static {
+		String loremIpsum = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.";
+
+		StringBuilder b = new StringBuilder();
+		for (int i = 0; i < 20; i++) {
+			b.append(loremIpsum);
+		}
+		longString = b.toString();
+	}
+
+	public static void main(String[] args) {
+		Display display = new Display();
+		final Shell shell = new Shell(display);
+		shell.setText("Bug551588");
+		shell.setLayout(new FillLayout());
+
+		final ScrolledComposite sc = new ScrolledComposite(shell, SWT.V_SCROLL);
+
+		final TextLayoutControl textLayout = new TextLayoutControl(sc, SWT.NONE);
+		textLayout.setText(longString);
+
+		sc.setContent(textLayout);
+		sc.addListener(SWT.Resize, event -> {
+			int availableWidth = sc.getClientArea().width;
+			textLayout.setSize(textLayout.computeSize(availableWidth, SWT.DEFAULT));
+			int availableWidthPrime = sc.getClientArea().width;
+			if (availableWidth != availableWidthPrime) {
+				textLayout.setSize(textLayout.computeSize(availableWidthPrime, SWT.DEFAULT));
+			}
+		});
+		shell.setSize(300, 300);
+		shell.open();
+		while (!shell.isDisposed()) {
+			if (!display.readAndDispatch())
+				display.sleep();
+		}
+		display.dispose();
+	}
+
+}
+
+class TextLayoutControl extends Canvas {
+	private final TextLayout textLayout;
+
+	private Listener listener = event -> TextLayoutControl.this.handleEvent(event);
+
+	public TextLayoutControl(Composite parent, int style) {
+		super(parent, checkStyle(style));
+
+		textLayout = new TextLayout(parent.getDisplay());
+
+		final int[] events = new int[] { SWT.Paint, SWT.Resize, SWT.Dispose };
+
+		for (int i = 0; i < events.length; i++) {
+			addListener(events[i], listener);
+		}
+	}
+
+	public void setText(String string) {
+		textLayout.setText(string);
+		redraw();
+	}
+
+	private static int checkStyle(int style) {
+		style &= ~SWT.H_SCROLL;
+		style &= ~SWT.V_SCROLL;
+		style |= SWT.DOUBLE_BUFFERED;
+
+		return style;
+	}
+
+	private void handleEvent(Event event) {
+		switch (event.type) {
+		case SWT.Paint:
+			onPaint(event.gc);
+			break;
+		case SWT.Resize:
+			onResize();
+			redraw();
+			break;
+		case SWT.Dispose:
+			onDispose();
+			break;
+		}
+	}
+
+	private void onDispose() {
+		textLayout.dispose();
+	}
+
+	private void onPaint(GC gc) {
+		Rectangle clientArea = getClientArea();
+		gc.fillRectangle(clientArea);
+
+		int x = clientArea.x;
+		int y = clientArea.y;
+
+		textLayout.draw(gc, x, y);
+	}
+
+	private void onResize() {
+		final Rectangle bounds = getBounds();
+		textLayout.setWidth(bounds.width);
+	}
+
+	@Override
+	public Point computeSize(int wHint, int hHint, boolean changed) {
+		int oldWidth = textLayout.getWidth();
+		Rectangle size;
+
+		try {
+			textLayout.setWidth(wHint);
+			size = textLayout.getBounds();
+		} finally {
+			textLayout.setWidth(oldWidth);
+		}
+
+		return new Point(size.width, size.height);
+	}
+}
\ No newline at end of file
diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutRegressionTest.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutRegressionTest.java
new file mode 100644
index 0000000000..396739fc2e
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug551588_TextLayoutRegressionTest.java
@@ -0,0 +1,50 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Peter Severin 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:
+ *     Peter Severin - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.gtk.snippets;
+
+import org.eclipse.swt.graphics.Font;
+import org.eclipse.swt.graphics.FontData;
+import org.eclipse.swt.graphics.TextLayout;
+import org.eclipse.swt.graphics.TextStyle;
+import org.eclipse.swt.widgets.Display;
+
+public class Bug551588_TextLayoutRegressionTest {
+
+	public static void main(String[] args) {
+		Display display = new Display();
+
+		Font font = display.getSystemFont();
+		FontData fontData = font.getFontData()[0];
+		fontData.setHeight(100);
+		Font bigFont = new Font(display, fontData);
+
+		test(display, bigFont, 0);
+		test(display, bigFont, 1);
+		test(display, bigFont, 2);
+		test(display, bigFont, 3);
+
+		bigFont.dispose();
+
+		display.dispose();
+	}
+
+	private static void test(Display display, Font bigFont, int line)
+	{
+		TextLayout textLayout = new TextLayout(display);
+		textLayout.setText("a\na\na\na");
+		textLayout.setStyle(new TextStyle(bigFont, null, null), line * 2, line * 2);
+		System.out.println("Text bounds: " + textLayout.getBounds());
+		textLayout.dispose();
+	}
+}
\ No newline at end of file