Re: [PATCH] text layout implementation

[graydon hoare <graydon at redhat dot com>]

Sascha Brawer <brawer@dandelis.ch> writes:

> But why do you need to create the string at all? Could't you set up a
> CharacterIterator so it points to a single font run, and use the method
> java.awt.Font.createGlyphVector(FontRenderContext, CharacterIterator)?
> This should be much more efficient, because you'd save all those object
> allocations. A local class could implement CharacterIterator, but also
> have setter methods (or accessible fields), so only a single instance
> needs to be allocated.

ok, changed to do this.

> Also, does the proposed implementation work if there are changes in any
> attributes other than font, such as TextAttribute.UNDERLINE? It seems
> that all these attributes are lost by passing Strings.

well, the CharacterIteratorProxy I've implemented (modified patch
below) doesn't do the whole AttributedCharacterIterator interface, but
there's no method on Font to create a glyph vector with an
AttributedCharacterIterator anyways. I think we'd have to interpret
extra attributes as calls to deriveFont or something, no?

> Please don't feel offended if I'm a bit picky here; I'm just trying to
> ensure that we have efficient and good code.

not at all, this is precisely the sort of polishing that code review
fosters, I'm glad to get the suggestions (as I was, belatedly, with
the graphics2d suggestions).

-graydon

--- java/awt/font/TextLayout.java	17 Feb 2003 08:05:50 -0000	1.1
+++ java/awt/font/TextLayout.java	19 Nov 2003 20:34:57 -0000
@@ -43,8 +43,12 @@
 import java.awt.Shape;
 import java.awt.geom.AffineTransform;
 import java.awt.geom.Rectangle2D;
+import java.text.CharacterIterator;
 import java.text.AttributedCharacterIterator;
+import java.text.AttributedString;
 import java.util.Map;
+import java.awt.font.TextAttribute;
+
 
 /**
  * @author Michael Koch
@@ -67,24 +71,26 @@
     }
   }
 
+  private AttributedString attributedString;
   private FontRenderContext fontRenderContext;
   
   public TextLayout (AttributedCharacterIterator text, FontRenderContext frc)
   {
-    // FIXME
-    this.fontRenderContext = frc;
+    attributedString = new AttributedString (text);
+    fontRenderContext = frc;
   }
 
   public TextLayout (String string, Font font, FontRenderContext frc) 
   {
-    // FIXME
-    this.fontRenderContext = frc;
+    attributedString = new AttributedString (string);
+    attributedString.addAttribute (TextAttribute.FONT, font);
+    fontRenderContext = frc;
   }
 
   public TextLayout (String string, Map attributes, FontRenderContext frc) 
   {
-    // FIXME
-    this.fontRenderContext = frc;
+    attributedString = new AttributedString (string, attributes);
+    fontRenderContext = frc;
   }
 
   protected Object clone ()
@@ -100,9 +106,139 @@
       }
   }
 
+
+  protected class CharacterIteratorProxy 
+    implements CharacterIterator
+  {
+    public CharacterIterator target;
+    public int begin;
+    public int limit;
+    public int index;
+
+    public CharacterIteratorProxy (CharacterIterator ci)
+    {
+      target = ci;
+    }
+
+    public int getBeginIndex ()
+    {
+      return begin;
+    }
+
+    public int getEndIndex ()
+    {
+      return limit;
+    }
+
+    public int getIndex ()
+    {
+      return index;
+    }
+
+    public char setIndex (int idx) 
+      throws IllegalArgumentException
+    {
+      if (idx < begin || idx >= limit)
+        throw new IllegalArgumentException ();
+      char ch = target.setIndex (idx);
+      index = idx;
+      return ch;
+    }
+
+    public char first ()
+    {
+      int save = target.getIndex ();
+      char ch = target.setIndex (begin);
+      target.setIndex (save);
+      return ch;
+    }
+
+    public char last ()
+    {
+      if (begin == limit)
+        return this.first ();
+
+      int save = target.getIndex ();
+      char ch = target.setIndex (limit - 1);
+      target.setIndex (save);
+      return ch;
+    }
+
+    public char current ()
+    {
+      return target.current();
+    }
+
+    public char next ()
+    {
+      if (index >= limit - 1)
+        return CharacterIterator.DONE;
+      else
+        {
+          index++;
+          return target.next();
+        }
+    }
+
+    public char previous ()
+    {
+      if (index <= begin)
+        return CharacterIterator.DONE;
+      else
+        {
+          index--;
+          return target.previous ();
+        }
+    }
+
+    public Object clone ()
+    {
+      CharacterIteratorProxy cip = new CharacterIteratorProxy (this.target);
+      cip.begin = this.begin;
+      cip.limit = this.limit;
+      cip.index = this.index;
+      return cip;
+    }
+    
+  }
+
+
   public void draw (Graphics2D g2, float x, float y) 
   {
-    throw new Error ("not implemented");
+    AttributedCharacterIterator ci = attributedString.getIterator ();
+    CharacterIteratorProxy proxy = new CharacterIteratorProxy (ci);
+    Font defFont = g2.getFont ();
+
+    for (char c = ci.first ();
+         c != CharacterIterator.DONE;
+         c = ci.next ())
+      {                
+        proxy.begin = ci.getIndex ();
+        proxy.limit = ci.getRunLimit(TextAttribute.FONT);
+        if (proxy.limit <= proxy.begin)
+          continue;
+
+        proxy.index = proxy.begin;
+
+        Object fnt = ci.getAttribute(TextAttribute.FONT);
+        GlyphVector gv;
+        if (fnt != null && fnt instanceof Font)          
+          gv = ((Font)fnt).createGlyphVector (fontRenderContext, proxy);
+        else
+          gv = defFont.createGlyphVector (fontRenderContext, proxy);
+
+        g2.drawGlyphVector (gv, x, y);
+
+        int n = gv.getNumGlyphs ();
+        for (int i = 0; i < n; ++i)
+          {
+            GlyphMetrics gm = gv.getGlyphMetrics (i);
+            if (gm.getAdvanceX() == gm.getAdvance ())
+              x += gm.getAdvanceX ();
+            else
+              y += gm.getAdvanceY ();
+          }
+      }
   }
 
   public boolean equals (Object obj)