Bug 486291 - StyledTextArea - Fix monospace issues

  fixed monospace grid
  fixed newline looses caret issue
  fixed selection issue 

3 files changed, 164 insertions, 96 deletions

diff --git a/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextLayoutContainer.java b/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextLayoutContainer.java
index dd9c78be1..9d41dc27d 100644
--- a/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextLayoutContainer.java
+++ b/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextLayoutContainer.java
@@ -153,9 +153,6 @@ public class StyledTextLayoutContainer extends Region {
 	private StyledTextNode selectionStartNode;

 	private StyledTextNode selectionEndNode;

 

-	private double selectionStartX;

-	private double selectionEndX;

-

 	private Animation caretAnimation;

 

 	private final ReadOnlyBooleanProperty ownerFocusedProperty;

@@ -203,10 +200,13 @@ public class StyledTextLayoutContainer extends Region {
 		Bindings.bindContent(this.textLayoutNode.getChildren(), this.textNodes);

 		getChildren().setAll(this.selectionMarker, this.textLayoutNode, this.caret);

 		selectionProperty().addListener(this::handleSelectionChange);

+		this.textNodes.addListener(this::handleSelectionChange);

 

 		this.ownerFocusedProperty.addListener(this::updateCaretVisibility);

 		this.caretIndex.addListener(this::updateCaretVisibility);

 

+		updateCaretVisibility(null);

+

 //		this.annotationsProperty.addListener(new SetChangeListener<StyledTextAnnotation>() {

 //			@Override

 //			public void onChanged(javafx.collections.SetChangeListener.Change<? extends StyledTextAnnotation> change) {

@@ -296,33 +296,27 @@ public class StyledTextLayoutContainer extends Region {
 		}

 	}

 

-	private void handleSelectionChange(Observable o, TextSelection oldSelection, TextSelection newSelection) {

-		if (newSelection.length == 0) {

+	private void handleSelectionChange(Observable o) {

+		TextSelection selection = getSelection();

+		if (selection.length == 0) {

 			this.selectionMarker.setVisible(false);

-			this.selectionMarker.resize(0, 0);

-		} else {

+		}

+		else {

 			this.selectionMarker.setVisible(true);

-			int start = newSelection.offset;

-			int end = newSelection.offset + newSelection.length;

+			int startOffset = selection.offset;

+			int endOffset = selection.offset + selection.length;

 

 			this.selectionStartNode = null;

 			this.selectionEndNode = null;

 			for (StyledTextNode t : this.textNodes) {

-				if (t.intersectOffset(start, end)) {

+				if (t.intersectOffset(startOffset, endOffset)) {

 					if (this.selectionStartNode == null) {

 						this.selectionStartNode = t;

 					}

 					this.selectionEndNode = t;

 				}

 			}

-

-			if (this.selectionStartNode != null && this.selectionEndNode != null) {

-				int charIndex = start - this.selectionStartNode.getStartOffset();

-				this.selectionStartX = this.selectionStartNode.getCharLocation(charIndex);

-				charIndex = end - this.selectionEndNode.getStartOffset();

-				this.selectionEndX = this.selectionEndNode.getCharLocation(charIndex);

-				requestLayout();

-			}

+			requestLayout();

 		}

 	}

 

@@ -332,65 +326,64 @@ public class StyledTextLayoutContainer extends Region {
 		return d;

 	}

 

-	private double findX(int localOffset) {

-

-		double len = 0;

-		for (StyledTextNode t : this.textNodes) {

-			if (t.getStartOffset() <= localOffset && t.getEndOffset() > localOffset || this.textNodes.get(this.textNodes.size() - 1) == t) {

-				return len + t.getCharLocation(localOffset - t.getStartOffset());

-			}

-			len += t.getWidth();

-		}

-		return -1;

-	}

-

-	@Override

-	protected void layoutChildren() {

-		super.layoutChildren();

-

-		this.textLayoutNode.relocate(getInsets().getLeft(), getInsets().getTop());

+//	private double findX(int localOffset) {

+//

+//		double len = 0;

+//		for (StyledTextNode t : this.textNodes) {

+//			if (t.getStartOffset() <= localOffset && t.getEndOffset() > localOffset || this.textNodes.get(this.textNodes.size() - 1) == t) {

+//				return len + t.getCharLocation(localOffset - t.getStartOffset());

+//			}

+//			len += t.getWidth();

+//		}

+//		return -1;

+//	}

 

+	private void layoutAnnotations() {

 //		for (Entry<StyledTextAnnotation, Rectangle> e : annotationMarkers.entrySet()) {

-//			System.err.println("LAYOUTING MARKER: " + e.getKey().getText());

-//			final int globalBeginIndex = e.getKey().getStartOffset();

-//			final int globalEndIndex = e.getKey().getStartOffset() + e.getKey().getLength();

+//		System.err.println("LAYOUTING MARKER: " + e.getKey().getText());

+//		final int globalBeginIndex = e.getKey().getStartOffset();

+//		final int globalEndIndex = e.getKey().getStartOffset() + e.getKey().getLength();

 //

-//			System.err.println("global: " + globalBeginIndex + " - " + globalEndIndex);

+//		System.err.println("global: " + globalBeginIndex + " - " + globalEndIndex);

 //

-//			final int localBeginIndex = Math.max(0, globalBeginIndex - getStartOffset());

-//			final int localEndIndex = Math.min(getText().length(), globalEndIndex - getStartOffset());

+//		final int localBeginIndex = Math.max(0, globalBeginIndex - getStartOffset());

+//		final int localEndIndex = Math.min(getText().length(), globalEndIndex - getStartOffset());

 //

-//			System.err.println("local: " + localBeginIndex + " - " + localEndIndex);

+//		System.err.println("local: " + localBeginIndex + " - " + localEndIndex);

 //

-//			double xBegin = findX(localBeginIndex);

-//			double xEnd = findX(localEndIndex);

+//		double xBegin = findX(localBeginIndex);

+//		double xEnd = findX(localEndIndex);

 //

-////			System.err.println(xBegin + ", " + getInsets().getTop() + ", " + (xEnd - xBegin)+ ", " + textLayoutNode.prefHeight(-1));

-////			e.getValue().resizeRelocate(xBegin, getInsets().getTop(), xEnd - xBegin, textLayoutNode.prefHeight(-1));

-//			e.getValue().setX(xBegin);

-//			e.getValue().setY(getInsets().getTop());

-//			e.getValue().setWidth(xEnd - xBegin);

-//			e.getValue().setHeight(textLayoutNode.prefHeight(-1));

-//			e.getValue().toFront();

-//			System.err.println(" -> " + e.getValue());

-//		}

-

+////		System.err.println(xBegin + ", " + getInsets().getTop() + ", " + (xEnd - xBegin)+ ", " + textLayoutNode.prefHeight(-1));

+////		e.getValue().resizeRelocate(xBegin, getInsets().getTop(), xEnd - xBegin, textLayoutNode.prefHeight(-1));

+//		e.getValue().setX(xBegin);

+//		e.getValue().setY(getInsets().getTop());

+//		e.getValue().setWidth(xEnd - xBegin);

+//		e.getValue().setHeight(textLayoutNode.prefHeight(-1));

+//		e.getValue().toFront();

+//		System.err.println(" -> " + e.getValue());

+//	}

+	}

 

+	private void layoutSelection() {

 		if (this.selectionStartNode != null && this.selectionEndNode != null) {

-			double x1 = this.textLayoutNode.localToParent(this.selectionStartNode.getBoundsInParent().getMinX(), 0).getX() + this.selectionStartX;

-			double x2 = this.textLayoutNode.localToParent(this.selectionEndNode.getBoundsInParent().getMinX(), 0).getX() + this.selectionEndX;

-			this.selectionMarker.resizeRelocate(x1, 0, x2 - x1, getHeight());

+			final TextSelection selection = getSelection();

+			final double selectionStart = this.selectionStartNode.getLayoutX() + this.selectionStartNode.getCharLocation(selection.offset - this.selectionStartNode.getStartOffset());

+			final double selectionEnd = this.selectionEndNode.getLayoutX() + this.selectionEndNode.getCharLocation(selection.offset + selection.length - this.selectionEndNode.getStartOffset());

+			this.selectionMarker.resizeRelocate(selectionStart, 0, selectionEnd - selectionStart, getHeight());

 		}

+	}

 

+	private void layoutCaret() {

 		if (this.getCaretIndex() >= 0) {

-			this.textLayoutNode.layout();

-			this.textLayoutNode.applyCss();

 			for (StyledTextNode t : this.textNodes) {

-				t.applyCss();

+				// XXX do we really need to apply the css on the StyledTextNode here??

+//				t.applyCss();

 				if (t.getStartOffset() <= this.getCaretIndex() && (t.getEndOffset() > this.getCaretIndex() || this.textNodes.get(this.textNodes.size() - 1) == t)) {

 					double caretX = t.getCharLocation(this.getCaretIndex() - t.getStartOffset());

 					double x = this.textLayoutNode.localToParent(t.getBoundsInParent().getMinX(), 0).getX() + caretX;

-					double h = t.prefHeight(-1);

+

+					double h = getHeight();

 

 					this.caret.setStartX(x);

 					this.caret.setEndX(x);

@@ -414,6 +407,24 @@ public class StyledTextLayoutContainer extends Region {
 		}

 	}

 

+	@Override

+	protected void layoutChildren() {

+		super.layoutChildren();

+

+		this.textLayoutNode.relocate(getInsets().getLeft(), getInsets().getTop());

+

+		// we need to ensure that the text is layouted correctly before processing any kind of markers

+		// since we need the correct letter positions to place them

+		this.textLayoutNode.layout();

+		this.textLayoutNode.applyCss();

+

+		layoutSelection();

+		layoutCaret();

+

+		layoutAnnotations();

+

+	}

+

 	/**

 	 * @return list of text nodes rendered

 	 */

@@ -432,7 +443,10 @@ public class StyledTextLayoutContainer extends Region {
 		Point2D scenePoint = localToScene(point);

 		for (StyledTextNode t : this.textNodes) {

 			if (t.localToScene(t.getBoundsInLocal()).contains(scenePoint)) {

-				return t.getCaretIndexAtPoint(t.sceneToLocal(scenePoint)) + t.getStartOffset();

+				int idx = t.getCaretIndexAtPoint(t.sceneToLocal(scenePoint));

+				if( idx != -1 ) {

+					return idx + t.getStartOffset();

+				}

 			}

 		}

 

diff --git a/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextNode.java b/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextNode.java
index d78533bd8..8ced0733b 100644
--- a/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextNode.java
+++ b/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/StyledTextNode.java
@@ -14,8 +14,15 @@ import java.util.ArrayList;
 import java.util.Collections;

 import java.util.List;

 import java.util.Map;

+import java.util.Optional;

 import java.util.stream.Collectors;

 

+import org.eclipse.fx.core.Util;

+import org.eclipse.jdt.annotation.NonNull;

+import org.eclipse.jdt.annotation.Nullable;

+

+import com.sun.javafx.css.converters.PaintConverter;

+

 import javafx.beans.property.IntegerProperty;

 import javafx.beans.property.ObjectProperty;

 import javafx.beans.value.ObservableValue;

@@ -27,24 +34,17 @@ import javafx.css.SimpleStyleableObjectProperty;
 import javafx.css.StyleConverter;

 import javafx.css.Styleable;

 import javafx.css.StyleableProperty;

+import javafx.geometry.Bounds;

 import javafx.geometry.Point2D;

 import javafx.scene.Node;

+import javafx.scene.layout.HBox;

 import javafx.scene.layout.Region;

 import javafx.scene.paint.Color;

 import javafx.scene.paint.Paint;

-import javafx.scene.shape.MoveTo;

-import javafx.scene.shape.PathElement;

 import javafx.scene.text.Font;

 import javafx.scene.text.Text;

 import javafx.scene.text.TextBoundsType;

 

-import org.eclipse.fx.core.Util;

-import org.eclipse.jdt.annotation.NonNull;

-import org.eclipse.jdt.annotation.Nullable;

-

-import com.sun.javafx.css.converters.PaintConverter;

-import com.sun.javafx.scene.text.HitInfo;

-

 /**

  * A node who allows to decorate the text

  *

@@ -65,7 +65,7 @@ public class StyledTextNode extends Region {
 		 * @param textNode

 		 *            the text node decorated

 		 */

-		public void attach(StyledTextNode node, Text textNode);

+		public void attach(StyledTextNode node, Node textNode);

 

 		/**

 		 * Remove the decoration from the text

@@ -75,7 +75,7 @@ public class StyledTextNode extends Region {
 		 * @param textNode

 		 *            the text node decorated

 		 */

-		public void unattach(StyledTextNode node, Text textNode);

+		public void unattach(StyledTextNode node, Node textNode);

 

 		/**

 		 * Layout the decoration

@@ -85,11 +85,9 @@ public class StyledTextNode extends Region {
 		 * @param textNode

 		 *            the text node decorated

 		 */

-		public void layout(StyledTextNode node, Text textNode);

+		public void layout(StyledTextNode node, Node textNode);

 	}

 

-	private final Text textNode;

-

 	@SuppressWarnings("null")

 	@NonNull

 	private static final CssMetaData<StyledTextNode, @NonNull Paint> FILL = new CssMetaData<StyledTextNode, @NonNull Paint>("-fx-fill", PaintConverter.getInstance(), Color.BLACK) { //$NON-NLS-1$

@@ -279,6 +277,8 @@ public class StyledTextNode extends Region {
 	private int startOffset;

 	private List<Integer> tabPositions = new ArrayList<>();

 	private String originalText;

+	private final HBox textNode;

+//	private final Text textNode;

 

 	/**

 	 * Create a new styled text node

@@ -290,17 +290,29 @@ public class StyledTextNode extends Region {
 		getStyleClass().add("styled-text-node"); //$NON-NLS-1$

 		this.originalText = text;

 

-		this.textNode = new Text(processText(text));

-		this.textNode.setBoundsType(TextBoundsType.LOGICAL_VERTICAL_CENTER);

-		this.textNode.fillProperty().bind(fillProperty());

+		this.textNode = new HBox();

+

+		this.textNode.getChildren().setAll(rebuildText(text));

+

 		getChildren().add(this.textNode);

 		this.decorationStrategy.addListener(this::handleDecorationChange);

 

 		this.tabCharAdvance.addListener(o -> {

-			this.textNode.setText(processText(text));

+			this.textNode.getChildren().setAll(rebuildText(text));

 		});

 	}

 

+	private List<Text> rebuildText(String text) {

+		List<Text> l = new ArrayList<>();

+		for( char c : processText(text).toCharArray() ) {

+			Text textNode = new Text(c+""); //$NON-NLS-1$

+			textNode.setBoundsType(TextBoundsType.LOGICAL_VERTICAL_CENTER);

+			textNode.fillProperty().bind(fillProperty());

+			l.add(textNode);

+		}

+		return l;

+	}

+

 	private String processText(String text) {

 		String tmp = text;

 		StringBuilder b = new StringBuilder();

@@ -364,7 +376,7 @@ public class StyledTextNode extends Region {
 

 	@Override

 	public String toString() {

-		return this.originalText;

+		return "'" + this.originalText + "'";  //$NON-NLS-1$//$NON-NLS-2$

 	}

 

 	@Override

@@ -387,7 +399,6 @@ public class StyledTextNode extends Region {
 		if (decorationStrategy2 != null) {

 			decorationStrategy2.layout(this, this.textNode);

 		}

-

 	}

 

 	/**

@@ -398,10 +409,20 @@ public class StyledTextNode extends Region {
 	 * @return the index or <code>-1</code>

 	 */

 	public int getCaretIndexAtPoint(Point2D point) {

-		@SuppressWarnings("deprecation")

-		HitInfo info = this.textNode.impl_hitTestChar(this.textNode.sceneToLocal(localToScene(point)));

-		if (info != null) {

-			int idx = info.getInsertionIndex();

+		Point2D local = this.textNode.sceneToLocal(localToScene(point));

+//		System.err.println(local);

+

+		Optional<Node> charNode = this.textNode.getChildren().stream().filter( n -> n.getBoundsInParent().contains(local)).findFirst();

+		if( charNode.isPresent() ) {

+			Node node = charNode.get();

+			int idx = this.textNode.getChildren().indexOf(node);

+			Bounds bounds = node.getBoundsInParent();

+

+			// if it is the 2nd half of the character

+			if( bounds.getMinX() + bounds.getWidth() / 2 < local.getX() ) {

+				idx += 1;

+			}

+

 			int toRemove = 0;

 			for (Integer i : this.tabPositions) {

 				if (i.intValue() <= idx && idx < i.intValue() + this.tabCharAdvance.get()) {

@@ -418,7 +439,32 @@ public class StyledTextNode extends Region {
 			}

 			idx -= toRemove;

 			return idx;

+		} else {

+//			System.err.println("COULD NOT FIND NODE AT: " + local);

+//			this.textNode.getChildren().stream().map( n -> n.getBoundsInParent()).forEach(System.err::println);

 		}

+

+//		@SuppressWarnings("deprecation")

+//		HitInfo info = this.textNode.impl_hitTestChar(this.textNode.sceneToLocal(localToScene(point)));

+//		if (info != null) {

+//			int idx = info.getInsertionIndex();

+//			int toRemove = 0;

+//			for (Integer i : this.tabPositions) {

+//				if (i.intValue() <= idx && idx < i.intValue() + this.tabCharAdvance.get()) {

+//					toRemove += idx - i.intValue();

+//					// If we are in the 2nd half of the tab we

+//					// simply move one past the value

+//					if ((idx - i.intValue()) % this.tabCharAdvance.get() >= this.tabCharAdvance.get() / 2) {

+//						idx += 1;

+//					}

+//					break;

+//				} else if (i.intValue() < idx) {

+//					toRemove += this.tabCharAdvance.get() - 1;

+//				}

+//			}

+//			idx -= toRemove;

+//			return idx;

+//		}

 		return -1;

 	}

 

@@ -429,7 +475,6 @@ public class StyledTextNode extends Region {
 	 *            the index

 	 * @return the location or <code>0</code> if not found

 	 */

-	@SuppressWarnings("deprecation")

 	public double getCharLocation(int index) {

 		int realIndex = index;

 		for (Integer i : this.tabPositions) {

@@ -437,14 +482,22 @@ public class StyledTextNode extends Region {
 				realIndex += this.tabCharAdvance.get() - 1;

 			}

 		}

-		this.textNode.setImpl_caretPosition(realIndex);

-		PathElement[] pathElements = this.textNode.getImpl_caretShape();

-		for (PathElement e : pathElements) {

-			if (e instanceof MoveTo) {

-				return this.textNode.localToParent(((MoveTo) e).getX(), 0).getX();

-			}

+

+		if( realIndex >= 0 && realIndex < this.textNode.getChildren().size() ) {

+			return this.textNode.localToParent(this.textNode.getChildren().get(realIndex).getBoundsInParent()).getMinX();

+		} else if( ! this.textNode.getChildren().isEmpty() ) {

+			return this.textNode.localToParent(this.textNode.getChildren().get(this.textNode.getChildren().size()-1).getBoundsInParent()).getMaxX();

 		}

 

 		return 0.0;

+

+//		this.textNode.setImpl_caretPosition(realIndex);

+//		PathElement[] pathElements = this.textNode.getImpl_caretShape();

+//		for (PathElement e : pathElements) {

+//			if (e instanceof MoveTo) {

+//				return this.textNode.localToParent(((MoveTo) e).getX(), 0).getX();

+//			}

+//		}

+

 	}

 }
\ No newline at end of file
diff --git a/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/UnderlineStrategyFactory.java b/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/UnderlineStrategyFactory.java
index d67f2c51a..c8070238f 100644
--- a/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/UnderlineStrategyFactory.java
+++ b/bundles/runtime/org.eclipse.fx.ui.controls/src/org/eclipse/fx/ui/controls/styledtext/UnderlineStrategyFactory.java
@@ -10,6 +10,7 @@
  *******************************************************************************/

 package org.eclipse.fx.ui.controls.styledtext;

 

+import javafx.scene.Node;

 import javafx.scene.shape.Line;

 import javafx.scene.text.Text;

 

@@ -40,7 +41,7 @@ public class UnderlineStrategyFactory implements DecorationStrategyFactory {
 

 	static class UnderlineStrategy implements DecorationStrategy {

 		@Override

-		public void attach(StyledTextNode node, Text textNode) {

+		public void attach(StyledTextNode node, Node textNode) {

 			Line l = (Line) textNode.getUserData();

 			if( l == null ) {

 				l = new Line();

@@ -53,7 +54,7 @@ public class UnderlineStrategyFactory implements DecorationStrategyFactory {
 		}

 

 		@Override

-		public void unattach(StyledTextNode node, Text textNode) {

+		public void unattach(StyledTextNode node, Node textNode) {

 			Line l = (Line) textNode.getUserData();

 			if( l != null ) {

 				textNode.setUserData(null);

@@ -62,7 +63,7 @@ public class UnderlineStrategyFactory implements DecorationStrategyFactory {
 		}

 

 		@Override

-		public void layout(StyledTextNode node, Text textNode) {

+		public void layout(StyledTextNode node, Node textNode) {

 			Line l = (Line) textNode.getUserData();

 			if( l != null ) {

 				l.setEndX(node.getWidth());