I paid a little attention to the conditions determining the type of
ColorSpace, and came up with some questions:

GraphicsSetProcessColor(Color color): Would it be possible to
implement and use ColorSpace.getNumComponents() instead of hard coding
the number of components here?

GraphicsSetProcessColor.writeToStream and
PtocaBuilder.setExtendedTextColor: Ideally the data written out are
contained in the ColorSpace class instead of being contained in this
method for all types of ColorSpace; are they not?

These are only minor points. Apart from the junit problems signalled
in my earlier email, I am in favour of merging this work into trunk:
+1.

Thanks for this work.

Simon

On Sat, Jan 29, 2011 at 04:14:41PM +0100, Simon Pepping wrote:
> I get three failures on the color branch junit tests:
> 
> java version "1.6.0_18"
> OpenJDK Runtime Environment (IcedTea6 1.8.3) (6b18-1.8.3-2)
> OpenJDK Server VM (build 16.0-b13, mixed mode)
> OS: GNU/Linux (Debian testing)
> 
> [echo] Apache Ant version 1.8.0 compiled on March 11 2010
> [echo] VM: 16.0-b13, Sun Microsystems Inc.
> 
> [junit] Testcase: testSeparationColor(org.apache.fop.util.ColorUtilTestCase): 
> FAILED
> [junit] expected:<255.0> but was:<250.0>
> 
> [junit] Testcase: 
> testNamedColorProfile(org.apache.fop.util.ColorUtilTestCase):       FAILED
> [junit] expected:<255.0> but was:<253.0>
> 
> [junit] Testcase: 
> color_1.xml(org.apache.fop.intermediate.IntermediateFormatTestSuite$1):     
> FAILED
> [junit] org.custommonkey.xmlunit.Diff
> [junit] [different] Expected number of child nodes '14' but was '13' - 
> comparing <viewport...> at 
> /document[1]/page-sequence[1]/page[1]/content[1]/viewport[1] to <viewport...> 
> at /document[1]/page-sequence[1]/page[1]/content[1]/viewport[1]
> 
> Simon
> 
> On Wed, Jan 19, 2011 at 11:56:34AM +0100, Jeremias Maerki wrote:
> > I've cleaned up the color branch, tweaked a few things and did some more
> > testing. I'm happy with the current state, so I'm calling for a vote to
> > merge the current FOP color branch into trunk.
> > 
> > https://svn.apache.org/repos/asf/xmlgraphics/fop/branches/Temp_Color
> > 
> > +1 from me, obviously.
> > 
> > Jeremias Maerki
> > 

Reply via email to