Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8230833

LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY

    XMLWordPrintable

Details

    • b05
    • x86_64
    • windows_10

    Description

      ADDITIONAL SYSTEM INFORMATION :
      Microsoft Windows [Version 10.0.17134.765]
      openjdk version "11.0.3" 2019-04-16
      OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.3+7)
      OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.3+7, mixed mode)

      A DESCRIPTION OF THE PROBLEM :
      LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY - isIgnoreText() is ommitted in computePrefHeight and computeMinHeight methods, unlike in computePrefWidth and computeMinWidth which takes isIgnoreText() into the account.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      1. Open provided source code as an JavaFX application.
      2. Observe the green (aquamarine) color size.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      Label on the right side has does not show green background, because it fits to inner graphic node.
      ACTUAL -
      Label preferred size is computed based on text, which should be ignored, and it grows the size of the label, so there are gaps (green color) on top and bottom of graphic node.

      ---------- BEGIN SOURCE ----------

      import java.util.stream.Collectors;
      import java.util.stream.IntStream;

      import javafx.application.Application;
      import javafx.geometry.Insets;
      import javafx.scene.Scene;
      import javafx.scene.control.ContentDisplay;
      import javafx.scene.control.Label;
      import javafx.scene.control.TitledPane;
      import javafx.scene.layout.Background;
      import javafx.scene.layout.BackgroundFill;
      import javafx.scene.layout.BorderPane;
      import javafx.scene.layout.CornerRadii;
      import javafx.scene.layout.HBox;
      import javafx.scene.paint.Color;
      import javafx.stage.Stage;

      public class LabeledIssue
      {

          public static void main( String[] args )
          {
              System.err.println( Runtime.version().toString() );
              Application.launch( LabeledIssue.MainFx.class, args );
          }

          public static class MainFx extends Application
          {

              @Override
              public void start( final Stage primaryStage ) throws Exception
              {
                  final var left = new TitledPane( "ContentDisplay.LEFT", createLabel() );
                  final var right =
                      new TitledPane( "ContentDisplay.GRAPHIC_ONLY", createLabelWithContentDisplay() );
                  final var hBox = new HBox( 5, left, right );
                  final BorderPane borderPane = new BorderPane( hBox );
                  borderPane.setPadding( new Insets( 5 ) );
                  Scene scene = new Scene( borderPane, 800, 600 );
                  primaryStage.setScene( scene );
                  primaryStage.show();
              }

              private static Label createLabel()
              {
                  final Label label = new Label( "simple\ntext" );
                  label.setPadding( new Insets( 5 ) );
                  label.setBackground(
                      new Background( new BackgroundFill( Color.YELLOW, CornerRadii.EMPTY, Insets.EMPTY ) ) );

                  final var mainLabel = new Label();
                  final var longText =
                      IntStream.range( 0, 10 ).mapToObj( i -> "Line: " + i + "\n" ).collect( Collectors.joining() );
                  System.out.println( longText );
                  mainLabel.setText( longText );
                  mainLabel.setGraphic( label );
                  mainLabel.setBackground(
                      new Background( new BackgroundFill( Color.AQUAMARINE, CornerRadii.EMPTY, Insets.EMPTY ) ) );
                  return mainLabel;
              }

              private static Label createLabelWithContentDisplay()
              {
                  final var label = createLabel();
                  label.setContentDisplay( ContentDisplay.GRAPHIC_ONLY );
                  return label;
              }

          }

      }
      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      I have patched LabeledSkinBase with below two methods:


          /** {@inheritDoc} */
          @Override protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
              final Labeled labeled = getSkinnable();
              final Font font = text.getFont();
              final ContentDisplay contentDisplay = labeled.getContentDisplay();
              final double gap = labeled.getGraphicTextGap();

              width -= leftInset + rightInset;

              if (!isIgnoreText() || isLayoutWithLabelPadding()) {
                  width -= leftLabelPadding() + rightLabelPadding();
              }

              String str = labeled.getText();
              if (str != null && str.endsWith("\n")) {
                  // Strip ending newline so we don't count another row.
                  str = str.substring(0, str.length() - 1);
              }

              double textWidth = width;
              if (!isIgnoreGraphic() &&
                      (contentDisplay == LEFT || contentDisplay == RIGHT)) {
                  textWidth -= (graphic.prefWidth(-1) + gap);
              }

              // TODO figure out how to cache this effectively.
              final double textHeight = Utils.computeTextHeight(font, str,
                      labeled.isWrapText() ? textWidth : 0,
                      labeled.getLineSpacing(), text.getBoundsType());

              final Node graphic = labeled.getGraphic();
              double height;
              if (isIgnoreGraphic()) {
                  height = snapSizeY( textHeight );
              } else if (isIgnoreText()) {
                  height = snapSizeY(graphic.prefHeight(-1));
              } else if (contentDisplay == TOP || contentDisplay == BOTTOM){
                  height = snapSizeY(textHeight + graphic.prefHeight(-1) ) + snapSpaceY( labeled.getGraphicTextGap() );
              } else {
                  height = snapSizeY( Math.max(textHeight, graphic.prefHeight(-1)) );
              }

              double padding = topInset + bottomInset;
              if (!isIgnoreText() || isLayoutWithLabelPadding() ) {
                  padding += topLabelPadding() - bottomLabelPadding();
              }
              return height + padding;
          }


          private double computeMinLabeledPartHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
              final Labeled labeled = getSkinnable();
              final Font font = text.getFont();

              String str = labeled.getText();
              if (str != null && str.length() > 0) {
                  int newlineIndex = str.indexOf('\n');
                  if (newlineIndex >= 0) {
                      str = str.substring(0, newlineIndex);
                  }
              }

              // TODO figure out how to cache this effectively.
              // Base minimum height on one line (ignoring wrapping here).
              final double s = labeled.getLineSpacing();
              final double textHeight = Utils.computeTextHeight(font, str, 0, s, text.getBoundsType());

              final ContentDisplay contentDisplay = labeled.getContentDisplay();
              final Node graphic = labeled.getGraphic();
              double height;
              if (isIgnoreGraphic()) {
                  height = snapSizeY( textHeight );
              } else if (isIgnoreText()) {
                  height = snapSizeY(graphic.minWidth(-1));
              } else if (contentDisplay == TOP || contentDisplay == BOTTOM){
                  height = snapSizeY(textHeight + graphic.minHeight(-1) ) + snapSpaceY( labeled.getGraphicTextGap() );
              } else {
                  height = snapSizeY( Math.max(textHeight, graphic.minHeight(-1)) );
              }

              double padding = topInset + bottomInset;
              if (!isIgnoreText() || isLayoutWithLabelPadding() ) {
                  padding += topLabelPadding() - bottomLabelPadding();
              }

              return height + padding;
          }



      FREQUENCY : always


      Attachments

        Issue Links

          Activity

            People

              kpk Karthik P K
              webbuggrp Webbug Group
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: