Bugzilla #38618:

cleanup of rgb() and implementation of system-color()
Submitted by: Max Berger <max.at.berger.name>

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

10 files changed, 143 insertions, 59 deletions

diff --git a/src/documentation/content/xdocs/compliance.ihtml b/src/documentation/content/xdocs/compliance.ihtml
index 2507f4411..d77cd2faf 100644
--- a/src/documentation/content/xdocs/compliance.ihtml
+++ b/src/documentation/content/xdocs/compliance.ihtml
@@ -7101,7 +7101,7 @@
         no
       </td>
       <td align="center">
-        &nbsp;
+        &nbsp; <!-- TODO Implemented in Trunk. Update me on the next release! -->
       </td>
     </tr>
     <tr>
diff --git a/src/java/org/apache/fop/fo/FOPropertyMapping.java b/src/java/org/apache/fop/fo/FOPropertyMapping.java
index 2b6803547..fd9c9a1c5 100644
--- a/src/java/org/apache/fop/fo/FOPropertyMapping.java
+++ b/src/java/org/apache/fop/fo/FOPropertyMapping.java
@@ -688,7 +688,7 @@ public class FOPropertyMapping implements Constants {
         // background-color
         m  = new ColorTypeProperty.Maker(PR_BACKGROUND_COLOR) {
             protected Property convertPropertyDatatype(
-                    Property p, PropertyList propertyList, FObj fo) {
+                    Property p, PropertyList propertyList, FObj fo) throws PropertyException {
                 String nameval = p.getNCname();
                 if (nameval != null) {
                     return new ColorTypeProperty(nameval);
diff --git a/src/java/org/apache/fop/fo/expr/PropertyParser.java b/src/java/org/apache/fop/fo/expr/PropertyParser.java
index d08f7631c..67cee2d0c 100644
--- a/src/java/org/apache/fop/fo/expr/PropertyParser.java
+++ b/src/java/org/apache/fop/fo/expr/PropertyParser.java
@@ -50,6 +50,7 @@ public final class PropertyParser extends PropertyTokenizer {
         FUNCTION_TABLE.put("max", new MaxFunction());
         FUNCTION_TABLE.put("abs", new AbsFunction());
         FUNCTION_TABLE.put("rgb", new RGBColorFunction());
+        FUNCTION_TABLE.put("system-color", new SystemColorFunction());
         FUNCTION_TABLE.put("from-table-column", new FromTableColumnFunction());
         FUNCTION_TABLE.put("inherited-property-value",
                           new InheritedPropFunction());
@@ -66,7 +67,6 @@ public final class PropertyParser extends PropertyTokenizer {
         /**
          * * NOT YET IMPLEMENTED!!!
          * FUNCTION_TABLE.put("icc-color", new ICCcolorFunction());
-         * FUNCTION_TABLE.put("system-color", new SystemColorFunction());
          * FUNCTION_TABLE.put("system-font", new SystemFontFunction());
          *
          * FUNCTION_TABLE.put("merge-property-values", new MergePropsFunction());
diff --git a/src/java/org/apache/fop/fo/expr/RGBColorFunction.java b/src/java/org/apache/fop/fo/expr/RGBColorFunction.java
index 4e050bd08..976933d97 100644
--- a/src/java/org/apache/fop/fo/expr/RGBColorFunction.java
+++ b/src/java/org/apache/fop/fo/expr/RGBColorFunction.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 1999-2004 The Apache Software Foundation.
+ * Copyright 1999-2006 The Apache Software Foundation.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,19 +18,23 @@
  
 package org.apache.fop.fo.expr;
 
-
 import org.apache.fop.datatypes.PercentBaseContext;
 import org.apache.fop.datatypes.PercentBase;
 import org.apache.fop.fo.properties.ColorTypeProperty;
 import org.apache.fop.fo.properties.Property;
 
+/**
+ * Implements the rgb() function.
+ */
 class RGBColorFunction extends FunctionBase {
+    
+    /** @see org.apache.fop.fo.expr.Function#nbArgs() */
     public int nbArgs() {
         return 3;
     }
 
     /**
-     * Return an object which implements the PercentBase interface.
+     * @return an object which implements the PercentBase interface.
      * Percents in arguments to this function are interpreted relative
      * to 255.
      */
@@ -38,25 +42,10 @@ class RGBColorFunction extends FunctionBase {
         return new RGBPercentBase();
     }
 
+    /** @see org.apache.fop.fo.expr.Function */
     public Property eval(Property[] args,
                          PropertyInfo pInfo) throws PropertyException {
-        // Using CSS rules, numbers are either supposed to be 0-255
-        // or percentage values. If percentages value, they've already
-        // been converted to reals.
-        float[] cfvals = new float[3];    // RGB
-        for (int i = 0; i < 3; i++) {
-            Number cval = args[i].getNumber();
-            if (cval == null) {
-                throw new PropertyException("Argument to rgb() must be a Number");
-            }
-            float colorVal = cval.floatValue() / 255f;
-            if (colorVal < 0.0 || colorVal > 255.0) {
-                throw new PropertyException(
-                        "Arguments to rgb() must normalize to the range 0 to 1");
-            }
-            cfvals[i] = colorVal;
-        }
-        return new ColorTypeProperty(cfvals[0], cfvals[1], cfvals[2]);
+        return new ColorTypeProperty("rgb(" + args[0] + "," + args[1] + "," + args[2] + ")");
 
     }
 
diff --git a/src/java/org/apache/fop/fo/expr/SystemColorFunction.java b/src/java/org/apache/fop/fo/expr/SystemColorFunction.java
new file mode 100644
index 000000000..bae217165
--- /dev/null
+++ b/src/java/org/apache/fop/fo/expr/SystemColorFunction.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2006 The Apache Software Foundation.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* $Id$ */
+ 
+package org.apache.fop.fo.expr;
+
+import org.apache.fop.fo.properties.ColorTypeProperty;
+import org.apache.fop.fo.properties.Property;
+
+/**
+ * Implements the system-color() function.
+ */
+class SystemColorFunction extends FunctionBase {
+    
+    /** @see org.apache.fop.fo.expr.Function#nbArgs() */
+    public int nbArgs() {
+        return 1;
+    }
+
+    /** @see org.apache.fop.fo.expr.Function */
+    public Property eval(Property[] args,
+                         PropertyInfo pInfo) throws PropertyException {
+        return new ColorTypeProperty("system-color(" + args[0] + ")");
+
+    }
+
+}
diff --git a/src/java/org/apache/fop/fo/properties/ColorTypeProperty.java b/src/java/org/apache/fop/fo/properties/ColorTypeProperty.java
index 6d3fc6b73..836d3d974 100644
--- a/src/java/org/apache/fop/fo/properties/ColorTypeProperty.java
+++ b/src/java/org/apache/fop/fo/properties/ColorTypeProperty.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 1999-2004 The Apache Software Foundation.
+ * Copyright 1999-2006 The Apache Software Foundation.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@ import java.util.StringTokenizer;
 import org.apache.fop.datatypes.ColorType;
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.PropertyList;
+import org.apache.fop.fo.expr.PropertyException;
 
 /**
  * Superclass for properties that wrap ColorType values
@@ -63,8 +64,10 @@ public class ColorTypeProperty extends Property implements ColorType {
             super(propId);
         }
 
+        /** @see org.apache.fop.fo.properties.PropertyMaker */
         public Property convertProperty(Property p,
-                                        PropertyList propertyList, FObj fo) {
+                                        PropertyList propertyList, FObj fo) 
+                    throws PropertyException {
             if (p instanceof ColorTypeProperty) {
                 return p;
             }
@@ -93,8 +96,9 @@ public class ColorTypeProperty extends Property implements ColorType {
      * Set the colour given a particular String specifying either a
      * colour name or #RGB or #RRGGBB
      * @param value RGB value as String to be parsed
+     * @throws PropertyException if the value can't be parsed
      */
-    public ColorTypeProperty(String value) {
+    public ColorTypeProperty(String value) throws PropertyException {
         if (value.startsWith("#")) {
             try {
                 if (value.length() == 4) {
@@ -114,16 +118,12 @@ public class ColorTypeProperty extends Property implements ColorType {
                     this.blue = Integer.parseInt(value.substring(5), 16)
                     / 255f;
                 } else {
-                    this.red = 0;
-                    this.green = 0;
-                    this.blue = 0;
-                    //log.error("unknown colour format. Must be #RGB or #RRGGBB");
+                    throw new PropertyException("Unknown color format: "
+                            + value + ". Must be #RGB or #RRGGBB");
                 }
-            } catch (Exception e) {
-                this.red = 0;
-                this.green = 0;
-                this.blue = 0;
-                //log.error("unknown colour format. Must be #RGB or #RRGGBB");
+            } catch (NumberFormatException e) {
+                throw new PropertyException("Unknown color format: " + value
+                        + ". Must be #RGB or #RRGGBB");
             }
         } else if (value.startsWith("rgb(")) {
             int poss = value.indexOf("(");
@@ -135,43 +135,52 @@ public class ColorTypeProperty extends Property implements ColorType {
                     if (st.hasMoreTokens()) {
                         String str = st.nextToken().trim();
                         if (str.endsWith("%")) {
-                            this.red =
-                            Integer.parseInt(str.substring(0, str.length() - 1))
-                            * 2.55f;
+                            this.red = Float.parseFloat(str.substring(0, str
+                                    .length() - 1)) / 100.0f;
                         } else {
-                            this.red = Integer.parseInt(str) / 255f;
+                            this.red = Float.parseFloat(str) / 255f;
                         }
                     }
                     if (st.hasMoreTokens()) {
                         String str = st.nextToken().trim();
                         if (str.endsWith("%")) {
-                            this.green =
-                            Integer.parseInt(str.substring(0, str.length() - 1))
-                            * 2.55f;
+                            this.green = Float.parseFloat(str.substring(0, str
+                                    .length() - 1)) / 100.0f;
                         } else {
-                            this.green = Integer.parseInt(str) / 255f;
+                            this.green = Float.parseFloat(str) / 255f;
                         }
                     }
                     if (st.hasMoreTokens()) {
                         String str = st.nextToken().trim();
                         if (str.endsWith("%")) {
-                            this.blue =
-                            Integer.parseInt(str.substring(0, str.length() - 1))
-                            * 2.55f;
+                            this.blue = Float.parseFloat(str.substring(0, str
+                                    .length() - 1)) / 100.0f;
                         } else {
-                            this.blue = Integer.parseInt(str) / 255f;
+                            this.blue = Float.parseFloat(str) / 255f;
                         }
                     }
                 } catch (Exception e) {
-                    this.red = 0;
-                    this.green = 0;
-                    this.blue = 0;
-                    //log.error("unknown colour format. Must be #RGB or #RRGGBB");
+                    throw new PropertyException(
+                            "Arguments to rgb() must be [0..255] or [0%..100%]");
                 }
+            } else {
+                throw new PropertyException("Unknown color format: " + value
+                        + ". Must be rgb(r,g,b)");
             }
         } else if (value.startsWith("url(")) {
-            // refers to a gradient
+            throw new PropertyException(
+                    "Colors starting with url( are not yet supported!");
         } else {
+            if (value.startsWith("system-color(")) {
+                int poss = value.indexOf("(");
+                int pose = value.indexOf(")");
+                if (poss != -1 && pose != -1) {
+                    value = value.substring(poss + 1, pose);
+                } else {
+                    throw new PropertyException("Unknown color format: "
+                            + value + ". Must be system-color(x)");
+                }
+            }
             if (value.toLowerCase().equals("transparent")) {
                 this.red = 0;
                 this.green = 0;
@@ -189,14 +198,16 @@ public class ColorTypeProperty extends Property implements ColorType {
                     }
                 }
                 if (!found) {
-                    this.red = 0;
-                    this.green = 0;
-                    this.blue = 0;
-                    //log.error("unknown colour name: "
-                    //                       + value);
+                    throw new PropertyException("Unknown color name: " + value);
                 }
             }
         }
+        if ((this.red < 0.0 || this.red > 1.0)
+                || (this.green < 0.0 || this.green > 1.0)
+                || (this.blue < 0.0 || this.blue > 1.0)
+                || (this.alpha < 0.0 || this.alpha > 1.0)) {
+            throw new PropertyException("Color values out of range");
+        }
     }
 
     /**
diff --git a/src/java/org/apache/fop/fo/properties/NumberProperty.java b/src/java/org/apache/fop/fo/properties/NumberProperty.java
index 6ed092110..c69c7affd 100644
--- a/src/java/org/apache/fop/fo/properties/NumberProperty.java
+++ b/src/java/org/apache/fop/fo/properties/NumberProperty.java
@@ -22,6 +22,7 @@ import org.apache.fop.datatypes.PercentBaseContext;
 import org.apache.fop.datatypes.Numeric;
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.PropertyList;
+import org.apache.fop.fo.expr.PropertyException;
 
 /**
  * Class for handling numeric properties
@@ -42,10 +43,12 @@ public class NumberProperty extends Property implements Numeric {
         }
 
         /**
+         * @throws PropertyException 
          * @see PropertyMaker#convertProperty
          */
         public Property convertProperty(Property p,
-                                        PropertyList propertyList, FObj fo) {
+                                        PropertyList propertyList, FObj fo) 
+                    throws PropertyException {
             if (p instanceof NumberProperty) {
                 return p;
             }
diff --git a/src/java/org/apache/fop/fo/properties/PropertyMaker.java b/src/java/org/apache/fop/fo/properties/PropertyMaker.java
index 40e9b7b98..a1dcea7e9 100644
--- a/src/java/org/apache/fop/fo/properties/PropertyMaker.java
+++ b/src/java/org/apache/fop/fo/properties/PropertyMaker.java
@@ -558,10 +558,11 @@ public class PropertyMaker implements Cloneable {
      * @param fo The parent FO for the FO whose property is being made.
      * why this is needed, or remove it from the signature).
      * @return an Property with the appropriate datatype used
+     * @throws PropertyException for invalid or inconsistent input
      */
     protected Property convertPropertyDatatype(Property p,
                                                PropertyList propertyList,
-                                               FObj fo) {
+                                               FObj fo) throws PropertyException {
         return null;
     }
 
diff --git a/status.xml b/status.xml
index 22beced41..5e198c4b5 100644
--- a/status.xml
+++ b/status.xml
@@ -27,6 +27,9 @@
 
   <changes>
     <release version="FOP Trunk">
+      <action context="Code" dev="JM" type="add" fixes-bug="38618" due-to="Max Berger">
+        Added support for system-color() function.
+      </action>
       <action context="Code" dev="JM" type="fix">
         Bugfix: Fixed two causes for ClassCastExceptions in BlockContainerLayoutManager.
       </action>
diff --git a/test/fotree/testcases/color-functions.fo b/test/fotree/testcases/color-functions.fo
new file mode 100644
index 000000000..9f87a1994
--- /dev/null
+++ b/test/fotree/testcases/color-functions.fo
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Copyright 2006 The Apache Software Foundation
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<!-- $Id$ -->
+<!-- This test verifies all the different method for specifying a color. -->
+<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format" xmlns:test="http://xmlgraphics.apache.org/fop/test">
+  <fo:layout-master-set>
+    <fo:simple-page-master page-width="5in" page-height="5in" master-name="normal">
+      <fo:region-body/>
+    </fo:simple-page-master>
+  </fo:layout-master-set>
+  <fo:page-sequence master-reference="normal">
+    <fo:flow flow-name="xsl-region-body">
+      <fo:block>All the next blocks (other than this one) should be in blue</fo:block>
+      <fo:block color="#0000FF"><test:assert property="color" expected="#0000ff"/>#0000FF</fo:block>
+      <fo:block color="#00F"><test:assert property="color" expected="#0000ff"/>#00F</fo:block>
+      <fo:block color="rgb(0,0,255)"><test:assert property="color" expected="#0000ff"/>rgb(0,0,255)</fo:block>
+      <fo:block color="rgb(0%,0%,100%)"><test:assert property="color" expected="#0000ff"/>rgb(0%,0%,100%)</fo:block>
+      <fo:block color="system-color(blue)"><test:assert property="color" expected="#0000ff"/>system-color(blue)</fo:block>
+      <fo:block color="blue"><test:assert property="color" expected="#0000ff"/>blue</fo:block>
+    </fo:flow>
+  </fo:page-sequence>
+</fo:root>