Further cleanup/refinement of FixedLength:

* improve consistency in use of the PropertyCache
  remove public constructor ->  replaced by getInstance() in a handful of classes
* added a sourceResolution parameter to getInstance() and the private
  constructor to allow better handling of device-dependent units (px)


git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@617812 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 91 insertions, 86 deletions

diff --git a/src/java/org/apache/fop/fo/flow/ExternalGraphic.java b/src/java/org/apache/fop/fo/flow/ExternalGraphic.java
index dd45f585c..03229e4c2 100644
--- a/src/java/org/apache/fop/fo/flow/ExternalGraphic.java
+++ b/src/java/org/apache/fop/fo/flow/ExternalGraphic.java
@@ -37,7 +37,8 @@ import org.apache.fop.fo.ValidationException;
 import org.apache.fop.fo.properties.FixedLength;
 
 /**
- * Class modelling the fo:external-graphic object.
+ * Class modelling the <a href="http://www.w3.org/TR/xsl/#fo_external-graphic">
+ * <code>fo:external-graphic</code></a> object.
  * This FO node handles the external graphic. It creates an image
  * inline area that can be added to the area tree.
  */
@@ -63,9 +64,7 @@ public class ExternalGraphic extends AbstractGraphics {
         super(parent);
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public void bind(PropertyList pList) throws FOPException {
         super.bind(pList);
         src = pList.get(PR_SRC).getString();
@@ -87,15 +86,14 @@ public class ExternalGraphic extends AbstractGraphics {
             this.intrinsicHeight = info.getSize().getHeightMpt();
             int baseline = info.getSize().getBaselinePositionFromBottom();
             if (baseline != 0) {
-                this.intrinsicAlignmentAdjust = new FixedLength(-baseline);
+                this.intrinsicAlignmentAdjust
+                    = FixedLength.getInstance(-baseline);
             }
         }
         //TODO Report to caller so he can decide to throw an exception
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().image(this);
@@ -103,23 +101,19 @@ public class ExternalGraphic extends AbstractGraphics {
 
     /**
      * {@inheritDoc}
-     * XSL Content Model: empty
+     * <br>XSL Content Model: empty
      */
     protected void validateChildNode(Locator loc, String nsURI, String localName) 
         throws ValidationException {
             invalidChildError(loc, nsURI, localName);
     }
 
-    /**
-     * @return the "src" property.
-     */
+    /** @return the "src" property */
     public String getSrc() {
         return src;
     }
 
-    /**
-     * @return Get the resulting URL based on the src property.
-     */
+    /** @return Get the resulting URL based on the src property */
     public String getURL() {
         return url;
     }
@@ -129,30 +123,22 @@ public class ExternalGraphic extends AbstractGraphics {
         return "external-graphic";
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public int getNameId() {
         return FO_EXTERNAL_GRAPHIC;
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public int getIntrinsicWidth() {
         return this.intrinsicWidth;
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public int getIntrinsicHeight() {
         return this.intrinsicHeight;
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public Length getIntrinsicAlignmentAdjust() {
         return this.intrinsicAlignmentAdjust;
     }
diff --git a/src/java/org/apache/fop/fo/properties/BorderWidthPropertyMaker.java b/src/java/org/apache/fop/fo/properties/BorderWidthPropertyMaker.java
index b828de285..86986bca5 100644
--- a/src/java/org/apache/fop/fo/properties/BorderWidthPropertyMaker.java
+++ b/src/java/org/apache/fop/fo/properties/BorderWidthPropertyMaker.java
@@ -63,7 +63,7 @@ public class BorderWidthPropertyMaker extends LengthProperty.Maker {
         // Calculate the values as described in 7.7.20.
         Property style = propertyList.get(borderStyleId);
         if (style.getEnum() == Constants.EN_NONE) {
-            return new FixedLength(0);
+            return FixedLength.getInstance(0);
         }
         return p;
     }
diff --git a/src/java/org/apache/fop/fo/properties/FixedLength.java b/src/java/org/apache/fop/fo/properties/FixedLength.java
index 28206a12a..fbd235b30 100644
--- a/src/java/org/apache/fop/fo/properties/FixedLength.java
+++ b/src/java/org/apache/fop/fo/properties/FixedLength.java
@@ -32,18 +32,40 @@ public final class FixedLength extends LengthProperty {
     private int millipoints;
 
     /**
-     * Set the length given a number of units and a unit name.
+     * Set the length given a number of units, a unit name and
+     * an assumed resolution (used in case the units are pixels)
      * 
-     * @param numUnits quantity of input units
-     * @param units input unit specifier (in, cm, etc.)
+     * @param numUnits  quantity of input units
+     * @param units     input unit specifier
+     * @param res       input/source resolution
      */
-    private FixedLength(double numUnits, String units) {
-        convert(numUnits, units);
+    private FixedLength(double numUnits, String units, float res) {
+        this.millipoints = convert(numUnits, units, res);
     }
     
     /**
-     * Return the cached FixedLength instance corresponding
+     * Return the cached {@link FixedLength} instance corresponding
+     * to the computed value in base-units (millipoints).
+     * 
+     * @param numUnits  quantity of input units
+     * @param units     input unit specifier
+     * @param sourceResolution input/source resolution (= ratio of pixels per pt)
+     * @return  the canonical FixedLength instance corresponding
+     *          to the given number of units and unit specifier
+     *          in the given resolution
+     */
+    public static FixedLength getInstance(double numUnits, 
+                                          String units,
+                                          float sourceResolution) {
+        return (FixedLength)cache.fetch(
+                new FixedLength(numUnits, units, sourceResolution));
+        
+    }
+    
+    /**
+     * Return the cached {@link FixedLength} instance corresponding
      * to the computed value
+     * This method assumes a source-resolution of 1 (1px = 1pt)
      * 
      * @param numUnits  input units
      * @param units     unit specifier
@@ -52,59 +74,56 @@ public final class FixedLength extends LengthProperty {
      */
     public static FixedLength getInstance(double numUnits, 
                                           String units) {
-        return (FixedLength) cache.fetch(new FixedLength(numUnits, units));
+        return getInstance(numUnits, units, 1.0f);
         
     }
     
     /**
-     * @param baseUnits the length as a number of base units (millipoints)
+     * Return the cached {@link FixedLength} instance corresponding
+     * to the computed value.
+     * This method assumes 'millipoints' (non-standard) as units, 
+     * and an implied source-resolution of 1 (1px = 1pt).
+     * 
+     * @param numUnits  input units
+     * @return  the canonical FixedLength instance corresponding
+     *          to the given number of units and unit specifier
      */
-    public FixedLength(int baseUnits) {
-        millipoints = baseUnits;
+    public static FixedLength getInstance(double numUnits) {
+        return getInstance(numUnits, "mpt", 1.0f);
+        
     }
-
+    
     /**
      * Convert the given length to a dimensionless integer representing
      * a whole number of base units (milli-points).
+     * 
      * @param dvalue quantity of input units
      * @param unit input unit specifier (in, cm, etc.)
+     * @param res   the input/source resolution (in case the unit spec is "px")
      */
-    private void convert(double dvalue, String unit) {
-        // TODO: the whole routine smells fishy.
+    private static int convert(double dvalue, String unit, float res) {
+        // TODO: Maybe this method has a better place in org.apache.fop.util.UnitConv?.
 
-        int assumedResolution = 1;    // points/pixel = 72dpi
-
-        if (unit.equals("in")) {
-            dvalue = dvalue * 72;
-        } else if (unit.equals("cm")) {
-            dvalue = dvalue * 28.3464567;
-        } else if (unit.equals("mm")) {
-            dvalue = dvalue * 2.83464567;
-        } else if (unit.equals("pt")) {
-            // Do nothing.
-            // dvalue = dvalue;
-        } else if (unit.equals("mpt")) { //mpt is non-standard!!! mpt=millipoints
-            // TODO: this seems to be wrong.
-            // Do nothing.
-            // dvalue = dvalue;
-        } else if (unit.equals("pc")) {
-            dvalue = dvalue * 12;
-            /*
-             * } else if (unit.equals("em")) {
-             * dvalue = dvalue * fontsize;
-             */
-        } else if (unit.equals("px")) {
-            // TODO: get resolution from user agent?
-            dvalue = dvalue * assumedResolution;
-        } else {
-            dvalue = 0;
-            log.error("Unknown length unit '" + unit + "'");
-        }
-        if (unit.equals("mpt")) {
-            millipoints = (int)dvalue;
+        if ("px".equals(unit)) {
+            //device-dependent units, take the resolution into account
+            dvalue *= (res * 1000);
         } else {
-            millipoints = (int)(dvalue * 1000);
+            if ("in".equals(unit)) {
+                dvalue *= 72000;
+            } else if ("cm".equals(unit)) {
+                dvalue *= 28346.4567;
+            } else if ("mm".equals(unit)) {
+                dvalue *= 2834.64567;
+            } else if ("pt".equals(unit)) {
+                dvalue *= 1000;
+            } else if ("pc".equals(unit)) {
+                dvalue *= 12000;
+            } else if (!"mpt".equals(unit)) {
+                dvalue = 0;
+                log.error("Unknown length unit '" + unit + "'");
+            }
         }
+        return (int)dvalue;
     }
 
     /** {@inheritDoc} */
@@ -128,7 +147,7 @@ public final class FixedLength extends LengthProperty {
     }
 
     /**
-     * Return true since FixedLength are always absolute.
+     * Return true since a FixedLength is always absolute.
      * {@inheritDoc}
      */
     public boolean isAbsolute() {
@@ -142,11 +161,13 @@ public final class FixedLength extends LengthProperty {
 
     /** {@inheritDoc} */
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof FixedLength) {
             return (((FixedLength)obj).millipoints == this.millipoints);
-        } else {
-            return false;
         }
+        return false;
     }
 
     /** {@inheritDoc} */
diff --git a/src/java/org/apache/fop/fo/properties/FontSizePropertyMaker.java b/src/java/org/apache/fop/fo/properties/FontSizePropertyMaker.java
index acc02dc13..b641b2abf 100644
--- a/src/java/org/apache/fop/fo/properties/FontSizePropertyMaker.java
+++ b/src/java/org/apache/fop/fo/properties/FontSizePropertyMaker.java
@@ -55,7 +55,7 @@ public class FontSizePropertyMaker
         if (p instanceof PercentLength) {
             Property pp = propertyList.getFromParent(this.propId);
             p = FixedLength.getInstance(
-                    pp.getLength().getValue() * ((PercentLength)p).getPercentage() / 100, "mpt");
+                    pp.getLength().getValue() * ((PercentLength)p).getPercentage() / 100);
         }
         return p;
     }
@@ -74,10 +74,10 @@ public class FontSizePropertyMaker
             int baseFontSize = computeClosestAbsoluteFontSize(pp.getLength().getValue());
             if (p.getEnum() == EN_LARGER) {
                 return FixedLength.getInstance(
-                        Math.round(baseFontSize * FONT_SIZE_GROWTH_FACTOR), "mpt");
+                        Math.round(baseFontSize * FONT_SIZE_GROWTH_FACTOR));
             } else {
                 return FixedLength.getInstance(
-                        Math.round(baseFontSize / FONT_SIZE_GROWTH_FACTOR), "mpt");
+                        Math.round(baseFontSize / FONT_SIZE_GROWTH_FACTOR));
             }
         }
         return super.convertProperty(p, propertyList, fo);
diff --git a/src/java/org/apache/fop/fo/properties/IndentPropertyMaker.java b/src/java/org/apache/fop/fo/properties/IndentPropertyMaker.java
index ebc1b80f0..b29caba04 100644
--- a/src/java/org/apache/fop/fo/properties/IndentPropertyMaker.java
+++ b/src/java/org/apache/fop/fo/properties/IndentPropertyMaker.java
@@ -108,7 +108,7 @@ public class IndentPropertyMaker extends CorrespondingPropertyMaker {
             //Margin is used
             Numeric margin = propertyList.get(marginProp).getNumeric();
             
-            Numeric v = new FixedLength(0);
+            Numeric v = FixedLength.getInstance(0, "mpt");
             if (!propertyList.getFObj().generatesReferenceAreas()) {
                 // The inherited_value_of([start|end]-indent)
                 v = NumericOp.addition(v, propertyList.getInherited(baseMaker.propId).getNumeric());
@@ -173,7 +173,7 @@ public class IndentPropertyMaker extends CorrespondingPropertyMaker {
                 if (isInherited(propertyList) || !marginNearest) {
                     return null;
                 } else {
-                    return new FixedLength(0);
+                    return FixedLength.getInstance(0);
                 }
             } else {
                 return indent;
@@ -182,7 +182,7 @@ public class IndentPropertyMaker extends CorrespondingPropertyMaker {
             //Margin is used
             Numeric margin = propertyList.get(marginProp).getNumeric();
             
-            Numeric v = new FixedLength(0);
+            Numeric v = FixedLength.getInstance(0);
             if (isInherited(propertyList)) {
                 // The inherited_value_of([start|end]-indent)
                 v = NumericOp.addition(v, propertyList.getInherited(baseMaker.propId).getNumeric());
diff --git a/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java b/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java
index acce6f564..966f51a51 100644
--- a/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java
+++ b/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java
@@ -65,9 +65,7 @@ public class LengthRangeProperty extends Property implements CompoundDatatype {
                     || (len.isAbsolute() && len.getValue() < 0));
         }
         
-        /**
-         * {@inheritDoc}
-         */        
+        /** {@inheritDoc} */        
         public Property convertProperty(Property p, 
                                 PropertyList propertyList, FObj fo)
                         throws PropertyException {
@@ -84,7 +82,7 @@ public class LengthRangeProperty extends Property implements CompoundDatatype {
                         log.warn(FObj.decorateWithContextInfo(
                                 "Replaced negative value (" + len + ") for " + getName()
                                 + " with 0mpt", fo));
-                        p = new FixedLength(0);
+                        p = FixedLength.getInstance(0);
                     }
                 }
             }
@@ -107,7 +105,7 @@ public class LengthRangeProperty extends Property implements CompoundDatatype {
                         log.warn("Replaced negative value (" + len + ") for " + getName()
                                 + " with 0mpt");
                         val.setComponent(subpropertyId,
-                                new FixedLength(0), false);
+                                FixedLength.getInstance(0), false);
                         return baseProperty;
                     }
                 }