Minor API improvements to PText

[GoogleCodeExporter]

When considering the code on the phtml branch, I noticed a few things that
could be improved with PText.

textPaint is not implemented as a bound property.

The default textPaint (Color.BLACK) should be public, or package-private at
a minimum so that it is accessible to unit tests.

justification should be called horizontalAlignment.

constrainWidthToTextWidth and constrainHeightToTextHeight are poorly named.
 I am open to suggestions for these.

Should justification/constrain... be implemented as bound properties?

Original issue reported on code.google.com by heue...@gmail.com on 29 Jul 2009 at 9:02

[GoogleCodeExporter]

More bad smells:

font property can be set to null, but then DEFAULT_FONT is used.  Would be 
better if
it were set to DEFAULT_FONT on construction and if setFont(null) threw IAE or 
NPE.

no validation is performed on setting justification values.

no provision is made for setting text rendering hints (KEY_TEXT_ANTIALIASING,
KEY_TEXT_LCD_CONTRAST, KEY_FRACTIONALMETRICS) other than what is provided by
PPaintContext.RENDER_QUALITY_HIGH_FRC

Original comment by heue...@gmail.com on 29 Jul 2009 at 9:22

[GoogleCodeExporter]

Patch for some of the above:

Index: PText.java
===================================================================
--- PText.java  (revision 622)
+++ PText.java  (working copy)
@@ -75,12 +75,30 @@
     public static final String PROPERTY_FONT = "font";
     public static final int PROPERTY_CODE_FONT = 1 << 20;

-    public static Font DEFAULT_FONT = new Font("Helvetica", Font.PLAIN, 12);
-    public static double DEFAULT_GREEK_THRESHOLD = 5.5;
+    /**
+     * The property name that identifies a change of this node's text paint 
(see
+     * {@link #getTextPaint getTextPaint}). Both old and new value will be set 
in any
+     * property change event.
+     */
+    public static final String PROPERTY_TEXT_PAINT = "text  paint";
+    public static final int PROPERTY_CODE_TEXT_PAINT = 1 << 21;

-    private String text;
-    private Paint textPaint;
-    private Font font;
+    /** Default font, 12 point <code>"SansSerif"</code>. */
+    //public static final Font DEFAULT_FONT = new Font(Font.SANS_SERIF, 
Font.PLAIN,
12); jdk 1.6+
+    public static final Font DEFAULT_FONT = new Font("SansSerif", Font.PLAIN, 
12);
+
+    /** Default text paint, <code>Color.BLACK</code>. */
+    public static final Paint DEFAULT_TEXT_PAINT = Color.BLACK;
+
+    /** Default greek threshold, <code>5.5d</code>. */
+    public static double DEFAULT_GREEK_THRESHOLD = 5.5d;
+
+    /** Default text, <code>""</code>. */
+    public static final String DEFAULT_TEXT = "";
+
+    private String text = DEFAULT_TEXT;
+    private Paint textPaint = DEFAULT_TEXT_PAINT;
+    private Font font = DEFAULT_FONT;
     protected double greekThreshold = DEFAULT_GREEK_THRESHOLD;
     private float justification = Component.LEFT_ALIGNMENT;
     private boolean constrainHeightToTextHeight = true;
@@ -89,13 +107,11 @@

     public PText() {
         super();
-        setTextPaint(Color.BLACK);
-        text = "";
     }

-    public PText(final String aText) {
+    public PText(final String text) {
         this();
-        setText(aText);
+        setText(text);
     }

     /**
@@ -132,8 +148,13 @@
      * @param textPaint
      */
     public void setTextPaint(final Paint textPaint) {
+        if (textPaint == this.textPaint) {
+            return;
+        }
+        final Paint oldTextPaint = this.textPaint;
         this.textPaint = textPaint;
         invalidatePaint();
+        firePropertyChange(PROPERTY_CODE_TEXT_PAINT, PROPERTY_TEXT_PAINT,
oldTextPaint, this.textPaint);
     }

     public boolean isConstrainWidthToTextWidth() {
@@ -191,13 +212,16 @@
      * Set the text for this node. The text will be broken up into multiple
      * lines based on the size of the text and the bounds width of this node.
      */
-    public void setText(final String aText) {
-        final String old = text;
-        text = aText == null ? "" : aText;
+    public void setText(final String text) {
+        if (text == this.text) {
+            return;
+        }
+        final String oldText = this.text;
+        this.text = text == null ? DEFAULT_TEXT : text;
         lines = null;
         recomputeLayout();
         invalidatePaint();
-        firePropertyChange(PROPERTY_CODE_TEXT, PROPERTY_TEXT, old, text);
+        firePropertyChange(PROPERTY_CODE_TEXT, PROPERTY_TEXT, oldText, 
this.text);
     }

     /**
@@ -206,9 +230,6 @@
      * @return the font of this PText.
      */
     public Font getFont() {
-        if (font == null) {
-            font = DEFAULT_FONT;
-        }
         return font;
     }

@@ -218,13 +239,16 @@
      * node instead of changing the font size to get that same effect. Using
      * very large font sizes can slow performance.
      */
-    public void setFont(final Font aFont) {
-        final Font old = font;
-        font = aFont;
+    public void setFont(final Font font) {
+        if (font == this.font) {
+            return;
+        }
+        final Font oldFont = this.font;
+        this.font = font == null ? DEFAULT_FONT : font;
         lines = null;
         recomputeLayout();
         invalidatePaint();
-        firePropertyChange(PROPERTY_CODE_FONT, PROPERTY_FONT, old, font);
+        firePropertyChange(PROPERTY_CODE_FONT, PROPERTY_FONT, oldFont, 
this.font);
     }

     private static final TextLayout[] EMPTY_TEXT_LAYOUT_ARRAY = new TextLayout[0];

Original comment by heue...@gmail.com on 29 Jul 2009 at 9:46

[GoogleCodeExporter]

And patch for justification/horizontalAlignment:

Index: PText.java
===================================================================
--- PText.java  (revision 622)
+++ PText.java  (working copy)
@@ -77,12 +77,14 @@

     public static Font DEFAULT_FONT = new Font("Helvetica", Font.PLAIN, 12);
     public static double DEFAULT_GREEK_THRESHOLD = 5.5;
+    /** Default horizontal alignment, <code>Component.LEFT_ALIGNMENT</code>. */
+    public static final float DEFAULT_HORIZONTAL_ALIGNMENT = 
Component.LEFT_ALIGNMENT;

     private String text;
     private Paint textPaint;
     private Font font;
     protected double greekThreshold = DEFAULT_GREEK_THRESHOLD;
-    private float justification = Component.LEFT_ALIGNMENT;
+    private float horizontalAlignment = DEFAULT_HORIZONTAL_ALIGNMENT;
     private boolean constrainHeightToTextHeight = true;
     private boolean constrainWidthToTextWidth = true;
     private transient TextLayout[] lines;
@@ -98,26 +100,58 @@
         setText(aText);
     }

+    /** @deprecated by getHorizontalAlignment() */
+    public float getJustification() {
+        return getHorizontalAlignment();
+    }
+
+    /** @deprecated by setHorizontalAlignment(float) */
+    public void setJustification(final float justification) {
+        setHorizontalAlignment(justification);
+    }
+
     /**
-     * Return the justificaiton of the text in the bounds.
-     * 
-     * @return float
+     * Return the horizontal alignment for this text node.  The horizontal 
alignment
will be one of 
+     * <code>Component.LEFT_ALIGNMENT</code>, 
<code>Component.CENTER_ALIGNMENT</code>,
+     * or <code>Component.RIGHT_ALIGNMENT</code>.
+     *
+     * @see #DEFAULT_HORIZONTAL_ALIGNMENT
+     * @return the horizontal alignment for this text node
      */
-    public float getJustification() {
-        return justification;
+    public float getHorizontalAlignment() {
+        return horizontalAlignment;
     }

     /**
-     * Sets the justificaiton of the text in the bounds.
-     * 
-     * @param just
+     * Set the horizontal alignment for this text node to
<code>horizontalAlignment</code>
+     *
+     * @param horizontalAlignment horizontal alignment, must be one of
+     *    <code>Component.LEFT_ALIGNMENT</code>,
<code>Component.CENTER_ALIGNMENT</code>,
+     *    or <code>Component.RIGHT_ALIGNMENT</code>
      */
-    public void setJustification(final float just) {
-        justification = just;
-        recomputeLayout();
+    public void setHorizontalAlignment(final float horizontalAlignment) {
+        if (!validHorizontalAlignment(horizontalAlignment)) {
+            throw new IllegalArgumentException("horizontalAlignment must be 
one of
Component.LEFT_ALIGNMENT, "
+                    + "Component.CENTER_ALIGNMENT, or 
Component.RIGHT_ALIGNMENT");
+        }
+        this.horizontalAlignment = horizontalAlignment;
     }

     /**
+     * Return true if the specified horizontal alignment is one of
<code>Component.LEFT_ALIGNMENT</code>,
+     * <code>Component.CENTER_ALIGNMENT</code>, or
<code>Component.RIGHT_ALIGNMENT</code>.
+     *
+     * @param horizontalAlignment horizontal alignment
+     * @return true if the specified horizontal alignment is one of
<code>Component.LEFT_ALIGNMENT</code>,
+     *    <code>Component.CENTER_ALIGNMENT</code>, or
<code>Component.RIGHT_ALIGNMENT</code>
+     */
+    private static boolean validHorizontalAlignment(final float 
horizontalAlignment) {
+        return Component.LEFT_ALIGNMENT == horizontalAlignment
+                || Component.CENTER_ALIGNMENT == horizontalAlignment
+                || Component.RIGHT_ALIGNMENT == horizontalAlignment;
+    }
+
+    /**
      * Get the paint used to paint this nodes text.
      * 
      * @return Paint
@@ -328,7 +362,7 @@
                 return;
             }

-            final float offset = (float) (getWidth() - tl.getAdvance()) * 
justification;
+            final float offset = (float) (getWidth() - tl.getAdvance()) *
horizontalAlignment;
             tl.draw(g2, x + offset, y);

             y += tl.getDescent() + tl.getLeading();

FindBugs may complain about the float comparisons in validHorizontalAlignment.

Original comment by heue...@gmail.com on 29 Jul 2009 at 10:04

[GoogleCodeExporter]

In addition to those issues described above, javadoc has been improved and 
protected
paint(PPaintContext) has been split into protected paintGreek(PPaintContext) and
protected paintText(PPaintContext) to facilitate overriding paint styles in 
subclasses.

I've left constrainWidthToTextWidth and constrainHeightToTextHeight as is for 
now.

$ commit -m "Issue 114 ; minor API improvements to PText"
  Sending        ...
  Transmitting file data ...
  Committed revision 623.

Original comment by heue...@gmail.com on 30 Jul 2009 at 2:59

• Changed state: Started

[GoogleCodeExporter]

Original comment by heue...@gmail.com on 30 Jul 2009 at 5:58

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by allain.lalonde on 30 Oct 2009 at 3:53

• Changed state: Verified