[cp-patches] FYI: javax.swing.border.TitledBorder fixes

[David Gilbert]

It is great to see all the Swing app screen shots being posted.  In a few of them, I 
noticed that the insets for instances of TitledBorder are not quite right, so I 
wrote a bunch of Mauve tests for that class and then fixed the problems.  The 
committed patch is:

2005-10-10  David Gilbert  <address@hidden>

        * javax/swing/border/TitledBorder.java
        (TitledBorder(String)): Changed default justification and position,
        (TitledBorder(Border)): Likewise,
        (TitledBorder(Border, String)): Likewise,
        (BorderPainter.paint(Graphics)): Use full width and height,
        (BorderPainter.paintExcept): Fixed bounds for region 3,
        (paintBorder): indented left justified title further, adjusted
        ABOVE_TOP text position, adjusted bounds for call to paintExcept(),
        updated for renamed fields in Measurements class,
        (getMeasurements): added special handling for null text, increased
        outer spacing, adjusted spacing for TOP, BOTTOM and BELOW_BOTTOM title
        positions,
        (Measurements.borderSpacing): renamed outerSpacing,
        (Measurements.edgeSpacing): renamed innerSpacing,
        (Measurements.getContentInsets): updated for renamed fields,
        (Measurements.getMinimumSize): wrap at 80 columns.

I've run a few different tests on this (in addition to the Mauve tests) and it seems 
to be working well, but please let me know if there are special cases that I've missed.

Regards,

Dave

Index: javax/swing/border/TitledBorder.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/border/TitledBorder.java,v
retrieving revision 1.10
diff -u -r1.10 TitledBorder.java
--- javax/swing/border/TitledBorder.java        19 Sep 2005 09:59:59 -0000      
1.10
+++ javax/swing/border/TitledBorder.java        10 Oct 2005 08:11:27 -0000
@@ -1,5 +1,5 @@
 /* TitledBorder.java -- 
-   Copyright (C) 2003, 2004  Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2005  Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -304,7 +304,7 @@
   public TitledBorder(String title)
   {
     this(/* border */ null,
-         title, DEFAULT_JUSTIFICATION, DEFAULT_POSITION,
+         title, LEADING, TOP,
          /* titleFont */ null, /* titleColor */ null);
   }
 
@@ -317,7 +317,7 @@
    */
   public TitledBorder(Border border)
   {
-    this(border, /* title */ "", DEFAULT_JUSTIFICATION, DEFAULT_POSITION,
+    this(border, /* title */ "", LEADING, TOP,
          /* titleFont */ null, /* titleColor */ null);
   }
   
@@ -333,7 +333,7 @@
    */
   public TitledBorder(Border border, String title)
   {
-    this(border, title, DEFAULT_JUSTIFICATION, DEFAULT_POSITION,
+    this(border, title, LEADING, TOP,
          /* titleFont */ null, /* titleColor */ null);
   }
   
@@ -507,7 +507,7 @@
       public void paint(Graphics g)
       {
         if (b != null)
-          b.paintBorder(c, g, x, y, width - 1, height - 1);
+          b.paintBorder(c, g, x, y, width, height);
       }
 
 
@@ -565,7 +565,7 @@
         if (stripeHeight > 0)
         {
           paint(g, x, holeY, holeX - x, stripeHeight);  // patches #2 and #3
-          paint(g, holeX + holeWidth, holeY, width - (holeX + holeWidth), 
stripeHeight);
+          paint(g, holeX + holeWidth, holeY, x + width - (holeX + holeWidth), 
stripeHeight);
         }
 
         stripeHeight = height - (holeY - y + holeHeight);
@@ -577,16 +577,16 @@
     BorderPainter bp;
     int textX, textY, borderWidth, borderHeight;
 
-    borderWidth = width - (mes.borderSpacing.left + mes.borderSpacing.right);
-    borderHeight = height - (mes.borderSpacing.top + mes.borderSpacing.bottom);
+    borderWidth = width - (mes.outerSpacing.left + mes.outerSpacing.right);
+    borderHeight = height - (mes.outerSpacing.top + mes.outerSpacing.bottom);
     bp = new BorderPainter(c, getBorder(),
-                           x + mes.borderSpacing.left, y + 
mes.borderSpacing.top,
+                           x + mes.outerSpacing.left, y + mes.outerSpacing.top,
                            borderWidth, borderHeight);
 
     switch (getRealTitleJustification(c))
     {
     case LEFT:
-      textX = x + TEXT_INSET_H;
+      textX = x + EDGE_SPACING + TEXT_INSET_H;
       break;
 
     case CENTER:
@@ -604,22 +604,22 @@
     switch (titlePosition)
     {
     case ABOVE_TOP:
-      textY = y;
+      textY = y + EDGE_SPACING;
       break;
 
     case TOP:
     case DEFAULT_POSITION:
     default:
-      textY = y + mes.borderSpacing.top + mes.borderInsets.top - mes.textAscent
+      textY = y + mes.outerSpacing.top + mes.borderInsets.top - mes.textAscent
               + mes.lineHeight;
       break;
 
     case BELOW_TOP:
-      textY = y + mes.borderSpacing.top + mes.borderInsets.top + TEXT_SPACING;
+      textY = y + mes.outerSpacing.top + mes.borderInsets.top + TEXT_SPACING;
       break;
 
     case ABOVE_BOTTOM:
-      textY = y + height - mes.borderSpacing.bottom - mes.borderInsets.bottom
+      textY = y + height - mes.outerSpacing.bottom - mes.borderInsets.bottom
         - TEXT_SPACING - (mes.textAscent + mes.textDescent);
       break;
 
@@ -644,8 +644,8 @@
         g.setFont(oldFont);
         g.setColor(oldColor);
       }
-      bp.paintExcept(g, textX - 2, textY,
-                     mes.textWidth + 2, mes.textAscent + mes.textDescent);
+      bp.paintExcept(g, textX, textY,
+                     mes.textWidth, mes.textAscent + mes.textDescent);
     }
   }
   
@@ -1002,49 +1002,63 @@
         m.trimmedText = null;
     }
     
-    m.textAscent = fmet.getAscent();
-    m.textDescent = fmet.getDescent();
-
-    FontRenderContext frc = new FontRenderContext(new AffineTransform(), false,
-                                                  false);
-    LineMetrics lmet = m.font.getLineMetrics(m.trimmedText, 0,
-                                             m.trimmedText.length(), frc);
-    m.lineHeight = (int) lmet.getStrikethroughOffset();
-    // Fallback in case that LineMetrics is not available/working.
-    if (m.lineHeight == 0)
-      m.lineHeight = (int) (0.3333 * (double) m.textAscent);
-
     if (m.trimmedText != null)
-      m.textWidth = fmet.stringWidth(m.trimmedText) + 3;
+      {
+        m.textAscent = fmet.getAscent();
+        m.textDescent = fmet.getDescent() + fmet.getLeading();
 
-    m.edgeSpacing = new Insets(EDGE_SPACING, EDGE_SPACING, EDGE_SPACING, 
EDGE_SPACING);
-    m.borderSpacing = new Insets(0, 0, 0, 0);
+        FontRenderContext frc = new FontRenderContext(new AffineTransform(), 
+            false, false);
+        LineMetrics lmet = m.font.getLineMetrics(m.trimmedText, 0,
+            m.trimmedText.length(), frc);
+        m.lineHeight = (int) lmet.getStrikethroughOffset();
+        
+        // Fallback in case that LineMetrics is not available/working.
+        if (m.lineHeight == 0)
+          m.lineHeight = (int) (0.3333 * (double) m.textAscent);
+        m.textWidth = fmet.stringWidth(m.trimmedText) + 3;
+      }
+    else
+      {
+        m.textAscent = 0;
+        m.textDescent = 0;
+      }
+
+    m.innerSpacing = new Insets(EDGE_SPACING, EDGE_SPACING, EDGE_SPACING, 
+            EDGE_SPACING);
+    m.outerSpacing = new Insets(EDGE_SPACING, EDGE_SPACING, EDGE_SPACING, 
+            EDGE_SPACING);
 
     switch (titlePosition)
     {
     case ABOVE_TOP:
-      m.borderSpacing.top += m.textAscent + m.textDescent + TEXT_SPACING;
+      m.outerSpacing.top += m.textAscent + m.textDescent + TEXT_SPACING;
       break;
 
+    case TOP:
+      m.outerSpacing.top += m.textDescent + m.lineHeight;
+      m.innerSpacing.top += m.textAscent - m.lineHeight;
+      break;
+      
     case BELOW_TOP:
-      m.edgeSpacing.top += m.textAscent + m.textDescent + TEXT_SPACING;
+      m.innerSpacing.top += m.textAscent + m.textDescent + TEXT_SPACING;
       break;
 
     case ABOVE_BOTTOM:
-      m.edgeSpacing.bottom += m.textAscent + m.textDescent + TEXT_SPACING;
+      m.innerSpacing.bottom += m.textAscent + m.textDescent + TEXT_SPACING;
       break;
 
     case BOTTOM:
-      m.edgeSpacing.bottom += Math.max(m.textAscent - m.borderInsets.bottom, 
0);
-      m.borderSpacing.bottom += m.textDescent;
+      m.innerSpacing.bottom += Math.max(m.textAscent - m.lineHeight, 0);
+      m.outerSpacing.bottom += m.textDescent + m.lineHeight;
       break;
 
     case BELOW_BOTTOM:
-      m.borderSpacing.bottom += m.textAscent + m.textDescent + TEXT_SPACING;
+      m.outerSpacing.bottom += m.textAscent + m.textDescent;
       break;
 
     default:
-      m.borderSpacing.top += m.textAscent;
+      m.outerSpacing.top += m.textAscent;
     }
 
     return m;
@@ -1067,7 +1081,7 @@
      * which means that the font is to be retrieved from the current
      * LookAndFeel. In this case, this <code>font</code> field will
      * contain the result of the retrieval. Therefore, it is safe
-     * to assume that his <code>font</code> field will never have
+     * to assume that this <code>font</code> field will never have
      * a <code>null</code> value.
      */
     Font font;
@@ -1107,7 +1121,7 @@
 
 
     /**
-     * The border that constitues the interior border
+     * The border that constitutes the interior border
      * underneath the title text.
      */
     Border border;
@@ -1116,8 +1130,7 @@
     /**
      * The distance between the TitledBorder and the interior border.
      */
-    Insets borderSpacing;
-
+    Insets outerSpacing;
     
     /**
      * The width of the interior border, as returned by
@@ -1130,7 +1143,7 @@
      * The distance between the interior border and the nested
      * Component for which this TitledBorder is a border.
      */
-    Insets edgeSpacing;
+    Insets innerSpacing;
 
 
     /**
@@ -1149,10 +1162,10 @@
     {
       if (i == null)
         i = new Insets(0, 0, 0, 0);
-      i.left = borderSpacing.left + borderInsets.left + edgeSpacing.left;
-      i.right = borderSpacing.right + borderInsets.right + edgeSpacing.right;
-      i.top = borderSpacing.top + borderInsets.top + edgeSpacing.top;
-      i.bottom = borderSpacing.bottom + borderInsets.bottom + 
edgeSpacing.bottom;
+      i.left = outerSpacing.left + borderInsets.left + innerSpacing.left;
+      i.right = outerSpacing.right + borderInsets.right + innerSpacing.right;
+      i.top = outerSpacing.top + borderInsets.top + innerSpacing.top;
+      i.bottom = outerSpacing.bottom + borderInsets.bottom + 
innerSpacing.bottom;
       return i;
     }
 
@@ -1167,7 +1180,8 @@
       Insets insets;
 
       insets = getContentInsets(null);
-      width = Math.max(insets.left + insets.right, textWidth + 2 * 
TEXT_INSET_H);
+      width = Math.max(insets.left + insets.right, textWidth + 2 
+              * TEXT_INSET_H);
       return new Dimension(width, insets.top + insets.bottom);
     }
   }