8242548: Wrapped labeled controls using -fx-line-spacing cut text off

Reviewed-by: aghaisas, kcr

Author: hjohn

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/Utils.java

@@ -417,17 +417,27 @@ public static String computeClippedText(Font font, String text, double width,
     }
 
     public static String computeClippedWrappedText(Font font, String text, double width,
-                                            double height, OverrunStyle truncationStyle,
+                                            double height, double lineSpacing, OverrunStyle truncationStyle,
                                             String ellipsisString, TextBoundsType boundsType) {
         if (font == null) {
             throw new IllegalArgumentException("Must specify a font");
         }
 
+        // The height given does not need to include the line spacing after
+        // the last line to be able to render that last line correctly.
+        //
+        // However the calculations include the line spacing as part of a
+        // line's height.  In order to not cut off the last line because its
+        // line spacing wouldn't fit, the height used for the calculation
+        // is increased here with the line spacing amount.
+
+        height += lineSpacing;
+
         String ellipsis = (truncationStyle == CLIP) ? "" : ellipsisString;
         int eLen = ellipsis.length();
         // Do this before using helper, as it's not reentrant.
         double eWidth = computeTextWidth(font, ellipsis, 0);
-        double eHeight = computeTextHeight(font, ellipsis, 0, boundsType);
+        double eHeight = computeTextHeight(font, ellipsis, 0, lineSpacing, boundsType);
 
         if (width < eWidth || height < eHeight) {
             // The ellipsis doesn't fit.
@@ -438,7 +448,7 @@ public static String computeClippedWrappedText(Font font, String text, double wi
         helper.setFont(font);
         helper.setWrappingWidth((int)Math.ceil(width));
         helper.setBoundsType(boundsType);
-        helper.setLineSpacing(0);
+        helper.setLineSpacing(lineSpacing);
 
         boolean leading =  (truncationStyle == LEADING_ELLIPSIS ||
                             truncationStyle == LEADING_WORD_ELLIPSIS);

modules/javafx.controls/src/main/java/javafx/scene/control/skin/LabeledSkinBase.java

@@ -1107,7 +1107,7 @@ private void updateDisplayedText(double w, double h) {
             String ellipsisString = labeled.getEllipsisString();
 
             if (labeled.isWrapText()) {
-                result = Utils.computeClippedWrappedText(font, s, wrapWidth, wrapHeight, truncationStyle, ellipsisString, text.getBoundsType());
+                result = Utils.computeClippedWrappedText(font, s, wrapWidth, wrapHeight, labeled.getLineSpacing(), truncationStyle, ellipsisString, text.getBoundsType());
             } else if (multiline) {
                 StringBuilder sb = new StringBuilder();
 

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java

@@ -136,6 +136,12 @@ public class LabelSkinTest {
         assertTrue(skin.propertyChanged);
     }
 
+    @Test public void lineSpacingChangesOnLabelShouldInvoke_handleControlPropertyChanged() {
+        skin.addWatchedProperty(label.lineSpacingProperty());
+        label.setLineSpacing(1.0);
+        assertTrue(skin.propertyChanged);
+    }
+
     @Test public void textChangesOnLabelShouldInvoke_handleControlPropertyChanged() {
         skin.addWatchedProperty(label.textProperty());
         label.setText("Bust my buffers!");
@@ -279,6 +285,15 @@ public class LabelSkinTest {
         assertFalse(skin.get_invalidText());
     }
 
+    @Test public void lineSpacingChangesOnLabelShouldInvalidateLayoutAndDisplayText() {
+        label.layout();
+        skin.updateDisplayedText();
+
+        label.setLineSpacing(1.0);
+        assertTrue(label.isNeedsLayout());
+        assertTrue(skin.get_invalidText());
+    }
+
     @Test public void textChangesOnLabelShouldInvalidateLayoutAndDisplayTextAndTextWidth() {
         label.layout();
         skin.updateDisplayedText();
@@ -1145,6 +1160,26 @@ public class LabelSkinTest {
         assertTrue(height >= singleLineHeight * 5);
     }
 
+    @Test public void whenTextHasNewlinesAndPositiveLineSpacing_computePrefHeight_IncludesTheMultipleLinesAndLineSpacingInThePrefHeight() {
+        label.setLineSpacing(2);
+        label.setText("This\nis a test\nof the emergency\nbroadcast system.\nThis is only a test");
+        label.setPadding(new Insets(0, 0, 0, 0));
+        final double singleLineHeight = Utils.computeTextHeight(label.getFont(), " ", 0, text.getBoundsType());
+        final double expectedHeight = singleLineHeight * 5 + label.getLineSpacing() * 5 - label.getLineSpacing();
+        final double height = label.prefHeight(-1);
+        assertEquals(expectedHeight, height, 0);
+    }
+
+    @Test public void whenTextHasNewlinesAndNegativeLineSpacing_computePrefHeight_IncludesTheMultipleLinesAndLineSpacingInThePrefHeight() {
+        label.setLineSpacing(-2);
+        label.setText("This\nis a test\nof the emergency\nbroadcast system.\nThis is only a test");
+        label.setPadding(new Insets(0, 0, 0, 0));
+        final double singleLineHeight = Utils.computeTextHeight(label.getFont(), " ", 0, text.getBoundsType());
+        final double expectedHeight = singleLineHeight * 5 + label.getLineSpacing() * 5 - label.getLineSpacing();
+        final double height = label.prefHeight(-1);
+        assertEquals(expectedHeight, height, 0);
+    }
+
     @Test public void whenTextHasNewlinesAfterPreviousComputationOf_computePrefHeight_IncludesTheMultipleLinesInThePrefHeight() {
         label.setText("This is a test");
         final double oldPrefHeight = label.prefHeight(-1);

modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java

@@ -48,6 +48,7 @@ public boolean setContent(TextSpan[] spans) {
     private Font font;
     private int tabSize = DEFAULT_TAB_SIZE;
     private int nullFontSize = 0;
+    private float spacing;
 
     @Override
     public boolean setContent(String text, Object font) {
@@ -69,6 +70,8 @@ public boolean setWrapWidth(float wrapWidth) {
 
     @Override
     public boolean setLineSpacing(float spacing) {
+        this.spacing = spacing;
+
         return true;
     }
 
@@ -92,7 +95,7 @@ public BaseBounds getBounds(TextSpan filter, BaseBounds bounds) {
         final double fontSize = (font == null ? nullFontSize : ((Font)font).getSize());
         final String[] lines = getText().split("\n");
         double width = 0.0;
-        double height = fontSize * lines.length;
+        double height = fontSize * lines.length + spacing * (lines.length - 1);
         for (String line : lines) {
             final int length;
             if (line.contains("\t")) {