This solves some problems in javax.swing.text:
- Sometimes the layout got borked and was very slow. This was caused by
a thinko in FlowView.FlowStrategy.
- Line breaking was not working really well. This is improved now.

BeanShell and jIRCii should now work again as expected.

2006-11-02  Roman Kennke  <[EMAIL PROTECTED]>

        PR 29644
        * javax/swing/text/FlowView.java
        (FlowStrategy.changedUpdate): Reversed condition. This caused
        wrong layout and bad performance.
        (FlowStrategy.insertUpdate): Reversed condition. This caused
        wrong layout and bad performance.
        (FlowStrategy.removeUpdate): Reversed condition. This caused
        wrong layout and bad performance.
        (LogicalView): Changed to be a subclass of CompositeView.
        (LogicalView()): Only take one Element argument.
        (LogicalView.childAllocation): New method for implementing
        the abstract CompositeView method.
        (LogicalView.forwardUpdateToView): Overridden for correct
        reparenting.
        (getMinimumSpan): Overridden to handle line breaking correctly.
        (getPreferredSpan): Implemented to handle line breaking correctly.
        (getViewAtPoint): New method for implementing
        the abstract CompositeView method.
        (getViewIndexAtPosition): Overridden to handle leaf elements
        correctly.
        (isAfter): New method for implementing
        the abstract CompositeView method.
        (isBefore): New method for implementing
        the abstract CompositeView method.
        (loadChildren): Overridden to handle leaf elements
        correctly.
        (paint): New method for implementing
        the abstract CompositeView method.
        (calculateMinorAxisRequirements): Use preferredSpan in calculation.
        (loadChildren): Initialize flow layout by sending a synthetic
        insertUpdate() to the layout strategy.
        * javax/swing/text/GlyphView.java
        (DefaultGlyphPainter.getBoundedPosition): Fall back to Toolkit's
        font metrics if component is not available. Add initial offset
        to result.
        (breakView): Be more clever when breaking the view.
        (getBreakLocation): New helper method to determine a good
        break location.
        (getBreakWeight): Be more clever when breaking the view.
        (getTabbedSpan): Make sure we have a painter. Use view's
        start and end offset rather than the element's.
        * javax/swing/text/Utilities.java
        (drawTabbedText): Avoid useless add and sub with the y offset.

/Roman

Index: javax/swing/text/FlowView.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/FlowView.java,v
retrieving revision 1.17
diff -u -1 -5 -r1.17 FlowView.java
--- javax/swing/text/FlowView.java	12 Oct 2006 15:12:56 -0000	1.17
+++ javax/swing/text/FlowView.java	2 Nov 2006 11:19:28 -0000
@@ -27,30 +27,31 @@
 permission to link this library with independent modules to produce an
 executable, regardless of the license terms of these independent
 modules, and to copy and distribute the resulting executable under
 terms of your choice, provided that you also meet, for each linked
 independent module, the terms and conditions of the license of that
 module.  An independent module is a module which is not derived from
 or based on this library.  If you modify this library, you may extend
 this exception to your version of the library, but you are not
 obligated to do so.  If you do not wish to do so, delete this
 exception statement from your version. */
 
 
 package javax.swing.text;
 
 import java.awt.Component;
+import java.awt.Graphics;
 import java.awt.Rectangle;
 import java.awt.Shape;
 
 import javax.swing.SizeRequirements;
 import javax.swing.event.DocumentEvent;
 
 /**
  * A <code>View</code> that can flows it's children into it's layout space.
  *
  * The <code>FlowView</code> manages a set of logical views (that are
  * the children of the [EMAIL PROTECTED] #layoutPool} field). These are translated
  * at layout time into a set of physical views. These are the views that
  * are managed as the real child views. Each of these child views represents
  * a row and are laid out within a box using the superclasses behaviour.
  * The concrete implementation of the rows must be provided by subclasses.
@@ -74,92 +75,92 @@
     }
 
     /**
      * Receives notification from a <code>FlowView</code> that some content
      * has been inserted into the document at a location that the
      * <code>FlowView</code> is responsible for.
      *
      * The default implementation simply calls [EMAIL PROTECTED] #layout}.
      *
      * @param fv the flow view that sends the notification
      * @param e the document event describing the change
      * @param alloc the current allocation of the flow view
      */
     public void insertUpdate(FlowView fv, DocumentEvent e, Rectangle alloc)
     {
-      if (alloc != null)
+      if (alloc == null)
         {
           fv.layoutChanged(X_AXIS);
           fv.layoutChanged(Y_AXIS);
         }
       else
         {
           Component host = fv.getContainer();
           if (host != null)
-            host.repaint();
+            host.repaint(alloc.x, alloc.y, alloc.width, alloc.height);
         }
     }
 
     /**
      * Receives notification from a <code>FlowView</code> that some content
      * has been removed from the document at a location that the
      * <code>FlowView</code> is responsible for.
      *
      * The default implementation simply calls [EMAIL PROTECTED] #layout}.
      *
      * @param fv the flow view that sends the notification
      * @param e the document event describing the change
      * @param alloc the current allocation of the flow view
      */
     public void removeUpdate(FlowView fv, DocumentEvent e, Rectangle alloc)
     {
-      if (alloc != null)
+      if (alloc == null)
         {
           fv.layoutChanged(X_AXIS);
           fv.layoutChanged(Y_AXIS);
         }
       else
         {
           Component host = fv.getContainer();
           if (host != null)
-            host.repaint();
+            host.repaint(alloc.x, alloc.y, alloc.width, alloc.height);
         }
     }
 
     /**
      * Receives notification from a <code>FlowView</code> that some attributes
      * have changed in the document at a location that the
      * <code>FlowView</code> is responsible for.
      *
      * The default implementation simply calls [EMAIL PROTECTED] #layout}.
      *
      * @param fv the flow view that sends the notification
      * @param e the document event describing the change
      * @param alloc the current allocation of the flow view
      */
     public void changedUpdate(FlowView fv, DocumentEvent e, Rectangle alloc)
     {
-      if (alloc != null)
+      if (alloc == null)
         {
           fv.layoutChanged(X_AXIS);
           fv.layoutChanged(Y_AXIS);
         }
       else
         {
           Component host = fv.getContainer();
           if (host != null)
-            host.repaint();
+            host.repaint(alloc.x, alloc.y, alloc.width, alloc.height);
         }
     }
 
     /**
      * Returns the logical view of the managed <code>FlowView</code>.
      *
      * @param fv the flow view for which to return the logical view
      *
      * @return the logical view of the managed <code>FlowView</code>
      */
     protected View getLogicalView(FlowView fv)
     {
       return fv.layoutPool;
     }
 
@@ -420,49 +421,160 @@
           View tmp = view.getView(i);
           if (contains(parent, tmp))
             tmp.setParent(parent);
           else
             reparent(tmp, parent);
         }
     }
   }
 
   /**
    * This special subclass of <code>View</code> is used to represent
    * the logical representation of this view. It does not support any
    * visual representation, this is handled by the physical view implemented
    * in the <code>FlowView</code>.
    */
-  class LogicalView extends BoxView
+  class LogicalView extends CompositeView
   {
     /**
      * Creates a new LogicalView instance.
      */
-    LogicalView(Element el, int axis)
+    LogicalView(Element el)
     {
-      super(el, axis);
+      super(el);
     }
 
     /**
      * Overridden to return the attributes of the parent
      * (== the FlowView instance).
      */
     public AttributeSet getAttributes()
     {
       View p = getParent();
       return p != null ? p.getAttributes() : null;
     }
+
+    protected void childAllocation(int index, Rectangle a)
+    {
+      // Nothing to do here (not visual).
+    }
+
+    protected View getViewAtPoint(int x, int y, Rectangle r)
+    {
+      // Nothing to do here (not visual).
+      return null;
+    }
+
+    protected boolean isAfter(int x, int y, Rectangle r)
+    {
+      // Nothing to do here (not visual).
+      return false;
+    }
+
+    protected boolean isBefore(int x, int y, Rectangle r)
+    {
+      // Nothing to do here (not visual).
+      return false;
+    }
+
+    public float getPreferredSpan(int axis)
+    {
+      float max = 0;
+      float pref = 0;
+      int n = getViewCount();
+      for (int i = 0; i < n; i++)
+        {
+          View v = getView(i);
+          pref += v.getPreferredSpan(axis);
+          if (v.getBreakWeight(axis, 0, Integer.MAX_VALUE)
+              >= ForcedBreakWeight)
+            {
+              max = Math.max(pref, pref);
+              pref = 0;
+            }
+        }
+      max = Math.max(max, pref);
+      return max;
+    }
+
+    public float getMinimumSpan(int axis)
+    {
+      float max = 0;
+      float min = 0;
+      boolean wrap = true;
+      int n = getViewCount();
+      for (int i = 0; i < n; i++)
+        {
+          View v = getView(i);
+          if (v.getBreakWeight(axis, 0, Integer.MAX_VALUE)
+              == BadBreakWeight)
+            {
+              min += v.getPreferredSpan(axis);
+              wrap = false;
+            }
+          else if (! wrap)
+            {
+              max = Math.max(min, max);
+              wrap = true;
+              min = 0;
+            }
+        }
+      max = Math.max(max, min);
+      return max;
+    }
+
+    public void paint(Graphics g, Shape s)
+    {
+      // Nothing to do here (not visual).
+    }
+
+    /**
+     * Overridden to handle possible leaf elements.
+     */
+    protected void loadChildren(ViewFactory f)
+    {
+      Element el = getElement();
+      if (el.isLeaf())
+        {
+          View v = new LabelView(el);
+          append(v);
+        }
+      else
+        super.loadChildren(f);
+    }
+
+    /**
+     * Overridden to reparent the children to this logical view, in case
+     * they have been parented by a row.
+     */
+    protected void forwardUpdateToView(View v, DocumentEvent e, Shape a,
+                                       ViewFactory f)
+    {
+      v.setParent(this);
+      super.forwardUpdateToView(v, e, a, f);
+    }
+
+    /**
+     * Overridden to handle possible leaf element.
+     */
+    protected int getViewIndexAtPosition(int pos)
+    {
+      int index = 0;
+      if (! getElement().isLeaf())
+        index = super.getViewIndexAtPosition(pos);
+      return index;
+    }
   }
 
   /**
    * The shared instance of FlowStrategy.
    */
   static final FlowStrategy sharedStrategy = new FlowStrategy();
 
   /**
    * The span of the <code>FlowView</code> that should be flowed.
    */
   protected int layoutSpan;
 
   /**
    * Represents the logical child elements of this view, encapsulated within
    * one parent view (an instance of a package private <code>LogicalView</code>
@@ -553,33 +665,35 @@
    * Loads the children of this view. The <code>FlowView</code> does not
    * directly load its children. Instead it creates a logical view
    * ([EMAIL PROTECTED] #layoutPool}) which is filled by the logical child views.
    * The real children are created at layout time and each represent one
    * row.
    *
    * This method is called by [EMAIL PROTECTED] View#setParent} in order to initialize
    * the view.
    *
    * @param vf the view factory to use for creating the child views
    */
   protected void loadChildren(ViewFactory vf)
   {
     if (layoutPool == null)
       {
-        layoutPool = new LogicalView(getElement(), getAxis());
+        layoutPool = new LogicalView(getElement());
         layoutPool.setParent(this);
       }
+    // Initialize the flow strategy.
+    strategy.insertUpdate(this, null, null);
   }
 
   /**
    * Performs the layout of this view. If the span along the flow axis changed,
    * this first calls [EMAIL PROTECTED] FlowStrategy#layout} in order to rebuild the
    * rows of this view. Then the superclass's behaviour is called to arrange
    * the rows within the box.
    *
    * @param width the width of the view
    * @param height the height of the view
    */
   protected void layout(int width, int height)
   {
     int flowAxis = getFlowAxis();
     if (flowAxis == X_AXIS)
@@ -710,21 +824,21 @@
    * @param axis the axis that is examined
    * @param r the <code>SizeRequirements</code> object to hold the result,
    *        if <code>null</code>, a new one is created
    *
    * @return the size requirements for this <code>BoxView</code> along
    *         the specified axis
    */
   protected SizeRequirements calculateMinorAxisRequirements(int axis,
                                                             SizeRequirements r)
   {
     SizeRequirements res = r;
     if (res == null)
       res = new SizeRequirements();
     res.minimum = (int) layoutPool.getMinimumSpan(axis);
     res.preferred = Math.max(res.minimum,
-                             (int) layoutPool.getMinimumSpan(axis));
+                             (int) layoutPool.getPreferredSpan(axis));
     res.maximum = Integer.MAX_VALUE;
     res.alignment = 0.5F;
     return res;
   }
 }
Index: javax/swing/text/GlyphView.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/GlyphView.java,v
retrieving revision 1.22
diff -u -1 -5 -r1.22 GlyphView.java
--- javax/swing/text/GlyphView.java	12 Oct 2006 15:12:56 -0000	1.22
+++ javax/swing/text/GlyphView.java	2 Nov 2006 11:19:29 -0000
@@ -421,34 +421,39 @@
     /**
      * Determines the model offset, so that the text between <code>p0</code>
      * and this offset fits within the span starting at <code>x</code> with
      * the length of <code>len</code>. 
      *
      * @param v the glyph view
      * @param p0 the starting offset in the model
      * @param x the start location in the view
      * @param len the length of the span in the view
      */
     public int getBoundedPosition(GlyphView v, int p0, float x, float len)
     {
       TabExpander te = v.getTabExpander();
       Segment txt = v.getText(p0, v.getEndOffset());
       Font font = v.getFont();
-      FontMetrics fm = v.getContainer().getFontMetrics(font);
+      Container c = v.getContainer();
+      FontMetrics fm;
+      if (c != null)
+        fm = c.getFontMetrics(font);
+      else
+        fm = Toolkit.getDefaultToolkit().getFontMetrics(font);
       int pos = Utilities.getTabbedTextOffset(txt, fm, (int) x,
                                               (int) (x + len), te, p0, false);
-      return pos;
+      return pos + p0;
     }
 
     /**
      * Maps a visual position into a document location.
      *
      * @param v the glyph view
      * @param x the X coordinate of the visual position
      * @param y the Y coordinate of the visual position
      * @param a the allocated region
      * @param biasRet filled with the bias of the model location on method exit
      *
      * @return the model location that represents the specified view location
      */
     public int viewToModel(GlyphView v, float x, float y, Shape a,
                            Bias[] biasRet)
@@ -643,33 +648,33 @@
       te = (TabExpander) parent;
     
     return te;
   }
 
   /**
    * Returns the preferred span of this view for tab expansion.
    *
    * @param x the location of the view
    * @param te the tab expander to use
    *
    * @return the preferred span of this view for tab expansion
    */
   public float getTabbedSpan(float x, TabExpander te)
   {
-    Element el = getElement();
-    return getGlyphPainter().getSpan(this, el.getStartOffset(),
-                                     el.getEndOffset(), te, x);
+    checkPainter();
+    return getGlyphPainter().getSpan(this, getStartOffset(),
+                                     getEndOffset(), te, x);
   }
 
   /**
    * Returns the span of a portion of the view. This is used in TAB expansion
    * for fragments that don't contain TABs.
    *
    * @param p0 the start index
    * @param p1 the end index
    *
    * @return the span of the specified portion of the view
    */
   public float getPartialSpan(int p0, int p1)
   {
     Element el = getElement();
     Document doc = el.getDocument();
@@ -893,102 +898,98 @@
    * Tries to break the view near the specified view span <code>len</code>.
    * The glyph view can only be broken in the X direction. For Y direction it
    * returns itself.
    *
    * @param axis the axis for breaking, may be [EMAIL PROTECTED] View#X_AXIS} or
    *        [EMAIL PROTECTED] View#Y_AXIS}
    * @param p0 the model location where the fragment should start
    * @param pos the view position along the axis where the fragment starts
    * @param len the desired length of the fragment view
    *
    * @return the fragment view, or <code>this</code> if breaking was not
    *         possible
    */
   public View breakView(int axis, int p0, float pos, float len)
   {
-    if (axis == Y_AXIS)
-      return this;
-
-    checkPainter();
-
-    // Try to find a suitable line break.
-    Segment txt = new Segment();
-    try
+    View brokenView = this;
+    if (axis == X_AXIS)
       {
-        int start = getStartOffset();
-        int length = getEndOffset() - start;
-        getDocument().getText(start, length, txt);
-      }
-    catch (BadLocationException ex)
-      {
-        AssertionError err = new AssertionError("BadLocationException must not "
-                                                + "be thrown here.");
-        err.initCause(ex);
-        throw err;
+        checkPainter();
+        int end = glyphPainter.getBoundedPosition(this, p0, pos, len);
+        int breakLoc = getBreakLocation(p0, end);
+        if (breakLoc != -1)
+          end = breakLoc;
+        if (p0 != getStartOffset() || end != getEndOffset())
+          {
+            brokenView = createFragment(p0, end);
+          }
       }
-    int breakLocation =
-      Utilities.getBreakLocation(txt, getContainer().getFontMetrics(getFont()),
-                                 (int) pos, (int) (pos + len),
-                                 getTabExpander(), p0);
-    View brokenView = createFragment(p0, breakLocation);
     return brokenView;
   }
 
   /**
    * Determines how well the specified view location is suitable for inserting
    * a line break. If <code>axis</code> is <code>View.Y_AXIS</code>, then
    * this method forwards to the superclass, if <code>axis</code> is
    * <code>View.X_AXIS</code> then this method returns
    * [EMAIL PROTECTED] View#ExcellentBreakWeight} if there is a suitable break location
    * (usually whitespace) within the specified view span, or
    * [EMAIL PROTECTED] View#GoodBreakWeight} if not.
    *
    * @param axis the axis along which the break weight is requested
    * @param pos the starting view location
    * @param len the length of the span at which the view should be broken
    *
    * @return the break weight
    */
   public int getBreakWeight(int axis, float pos, float len)
   {
     int weight;
     if (axis == Y_AXIS)
       weight = super.getBreakWeight(axis, pos, len);
     else
       {
-        // FIXME: Commented out because the Utilities.getBreakLocation method
-        // is still buggy. The GoodBreakWeight is a reasonable workaround for
-        // now.
-//        int startOffset = getStartOffset();
-//        int endOffset = getEndOffset() - 1;
-//        Segment s = getText(startOffset, endOffset);
-//        Container c = getContainer();
-//        FontMetrics fm = c.getFontMetrics(c.getFont());
-//        int x0 = (int) pos;
-//        int x = (int) (pos + len);
-//        int breakLoc = Utilities.getBreakLocation(s, fm, x0, x,
-//                                                  getTabExpander(),
-//                                                  startOffset);
-//        if (breakLoc == startOffset || breakLoc == endOffset)
-//          weight = GoodBreakWeight;
-//        else
-//          weight = ExcellentBreakWeight;
-        weight = GoodBreakWeight;
+        checkPainter();
+        int start = getStartOffset();
+        int end = glyphPainter.getBoundedPosition(this, start, pos, len);
+        if (end == 0)
+          weight = BadBreakWeight;
+        else
+          {
+            if (getBreakLocation(start, end) != -1)
+              weight = ExcellentBreakWeight;
+            else
+              weight = GoodBreakWeight;
+          }
       }
     return weight;
   }
 
+  private int getBreakLocation(int start, int end)
+  {
+    int loc = -1;
+    Segment s = getText(start, end);
+    for (char c = s.last(); c != Segment.DONE && loc == -1; c = s.previous())
+      {
+        if (Character.isWhitespace(c))
+          {
+            loc = s.getIndex() - s.getBeginIndex() + 1 + start;
+          }
+      }
+    return loc;
+  }
+
   /**
    * Receives notification that some text attributes have changed within the
    * text fragment that this view is responsible for. This calls
    * [EMAIL PROTECTED] View#preferenceChanged(View, boolean, boolean)} on the parent for
    * both width and height.
    *
    * @param e the document event describing the change; not used here
    * @param a the view allocation on screen; not used here
    * @param vf the view factory; not used here
    */
   public void changedUpdate(DocumentEvent e, Shape a, ViewFactory vf)
   {
     preferenceChanged(this, true, true);
   }
 
Index: javax/swing/text/Utilities.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/Utilities.java,v
retrieving revision 1.38
diff -u -1 -5 -r1.38 Utilities.java
--- javax/swing/text/Utilities.java	28 Aug 2006 21:41:57 -0000	1.38
+++ javax/swing/text/Utilities.java	2 Nov 2006 11:19:29 -0000
@@ -81,72 +81,71 @@
    * @param startOffset starting offset in the text.
    * @return the x coordinate at the end of the drawn text.
    */
   public static final int drawTabbedText(Segment s, int x, int y, Graphics g,
                                          TabExpander e, int startOffset)
   {
     // This buffers the chars to be drawn.
     char[] buffer = s.array;
 
     // The font metrics of the current selected font.
     FontMetrics metrics = g.getFontMetrics();
     int ascent = metrics.getAscent();
 
     // The current x and y pixel coordinates.
     int pixelX = x;
-    int pixelY = y - ascent;
 
     int pixelWidth = 0;
     int pos = s.offset;
     int len = 0;
     
     int end = s.offset + s.count;
 
     for (int offset = s.offset; offset < end; ++offset)
       {
         char c = buffer[offset];
         if (c == '\t')
           {
             if (len > 0) {
-              g.drawChars(buffer, pos, len, pixelX, pixelY + ascent);            
+              g.drawChars(buffer, pos, len, pixelX, y);
               pixelX += pixelWidth;
               pixelWidth = 0;
             }
             pos = offset+1;
             len = 0;
           }
         
 	switch (c)
 	  {
 	  case '\t':
 	    // In case we have a tab, we just 'jump' over the tab.
 	    // When we have no tab expander we just use the width of ' '.
 	    if (e != null)
 	      pixelX = (int) e.nextTabStop(pixelX,
                                            startOffset + offset - s.offset);
 	    else
 	      pixelX += metrics.charWidth(' ');
 	    break;
 	  default:
             ++len;
 	    pixelWidth += metrics.charWidth(buffer[offset]);
 	    break;
 	  }
       }
 
     if (len > 0)
-      g.drawChars(buffer, pos, len, pixelX, pixelY + ascent);            
+      g.drawChars(buffer, pos, len, pixelX, y);
     
     return pixelX + pixelWidth;
   }
 
   /**
    * Determines the width, that the given text <code>s</code> would take
    * if it was printed with the given [EMAIL PROTECTED] java.awt.FontMetrics} on the
    * specified screen position.
    * @param s the text fragment
    * @param metrics the font metrics of the font to be used
    * @param x the x coordinate of the point at which drawing should be done
    * @param e the [EMAIL PROTECTED] TabExpander} to be used
    * @param startOffset the index in <code>s</code> where to start
    * @returns the width of the given text s. This takes tabs and newlines
    * into account.

Reply via email to