On Feb 4, 2008, at 22:15, Andreas Delmelle wrote:

Small *grin* update:

Looking into a fix now. The issue arises in ColorUtil.parseColorString(). After my changes, percentages get passed into fop-rgb-icc() as percentages instead of decimals, which leads to a rather peculiar error message:

[junit] SEVERE: Ignoring property: color="rgb-icc(100%,50%,0%, sRGB, 1, 0.5, 0)" (Arguments to rgb-icc() must be [0..255] or [0%.. 100%]; property:'color')

Investigated further, and the problem was yet slightly different: one of my changes was the addition of getPercentBase() to the ICCColorFunction to make sure the RGBColorFunction correctly gets passed the percentage values of the first three arguments. A small correction, it seemed: the PropertyParser now, correctly, recognizes the percentage and parses it into a resolved NumberProperty.

Somehow, what bothered me about making that change was that I could only set the percent-base for all arguments at the same time. Maybe it does not matter so much here, but I could imagine that someone would want to interpret percentages using a different base for the latter three arguments (?) Currently, it's all or nothing, but that seems to be only a minor issue. I've now at least separated it from the default RGBPercentBase, so we may revisit this later...

Now, while the message shows percentages, after the change the actual String value that get passed to ColorUtil contains resolved percentage values (between 0 and 255, as doubles...).

On the other hand, I still have some doubts on the color-parsing process if you look at it like this: Somewhere in an XML stream, an attribute is encountered with the name 'color' and the value 'rgb-icc(100%,50%,0%,ColorProfileX,1.0,0.5,1.0)'. This attribute makes its way into PropertyList.convertAttributeToProperty(), which results in the PropertyParser neatly breaking the expression down into a function call and a set of arguments. The arguments are in their turn nicely resolved to a StringProperty or NumberProperty. Then, the function call is resolved, which currently means: dump the properties/arguments back into a StringBuffer and parse that somewhere else. There is the inherently dangerous assumption that toString() will always be implemented to provide 'parseable' values.

While tracing further, I stumbled upon the following dubious construction...

try {
  float f = Float.parseFloat(someString); //(*)

  if (f < 0.0 || f > 1.0) {
    throw new PropertyException("Out of range!");
  }
  ...
  String s = anotherString;
  if (s == null || "".equals(s)) {
    throw new PropertyException("Missing argument!"); //(*)
  }

} catch (Exception e) {
  throw new PropertyException("Out of range!");
}


If one of the lines marked with (*) causes an error, the real reason why the try-block fails will always remains a mystery to the regular user. I happened to notice such an exception in the debugger. Since it is converted into a log message higher up, a stack trace will never appear, and even if it would, I doubt this would clarify much... :-/

Making it

...
} catch (PropertyException pe) {
  //simply re-throw
  throw pe;
} catch (Exception e) {
  //wrap in a PropertyException
  throw new PropertyException(e);
}

helps a lot already.


On another note, I've converted ColorUtil to use String.split() instead of StringTokenizer. I picked up that Sun actually discourages the use of StringTokenizer, unless you really need its three-argument constructor with the boolean value to get the separators as well. Since in this context we don't and we're on 1.4, we might as well take advantage of this feature.

Hope this sits right with everyone.


Cheers

Andreas

Reply via email to