[cairo] [PATCH][pixman] Test program for stressing the use of different formats and operators

Soeren Sandmann sandmann at daimi.au.dk
Mon Jul 13 23:22:17 PDT 2009


Siarhei Siamashka <siarhei.siamashka at gmail.com> writes:

> The code and overall method is mostly based on scaling-test. This one
> focuses on trying to stress as many different color format and types
> off composition operations as possible.
> 
> This is an initial implementation which may need more tuning. Also
> not all color format and operator combinations are actually used.
> The ones which crashed pixman or caused other problems even when
> compiled without any cpu specific optimizations are enclosed
> in '#if 0' blocks. Preferably they should be enabled later, once
> the bugs in pixman core are fixed.

This is clearly a very useful test. I fixed the 64 bit issues and the
issues with CONJOINT_DST/DISJOINT_DST before releasing 0.15.16. (And I
just realized that we need to compile with -fno-strict-aliasing; we
haven't had that so far). I'm not sure what's up with the
component-alpha ones.

The reason it crashes for the g4, c4, etc. formats is that those
require a palette to be set with 

        pixman_image_set_indexed().

That call sets a pointer to a data type that maps palette entries to
rgba pixels, and either rgb555 pixels (for c* formats) or y15 (for g*
formats) to palette entries.

Some specific comments on the code:

- The CRC32 and the random number generator could be shared with the
  scaling-test. I'd suggest renaming the existing utils.[ch] to
  something like show-image.[ch] and then make a new utils.[ch] that
  contains code shared between tests. Maybe there is other code that
  could be shared, like the endian swapping?

- It has a list of valid mask formats, but in fact all formats can be
  used as masks.

- It should follow the coding style and naming convention. (The coding
  style is similar to cairo/HACKING; the main difference is that
  braces always go on their own line). See the changes I made to
  scaling-test.c.

Some more general comments about test suites:

I'm not convinced that basing the test suite on CRC32 computations and
reference binaries is ideal. The things I'd be looking for are:

- Make it really easy to check if all tests pass.

- Make it easy to update when pixman behavior changes. 

  With the CRC based checks, if you commit something that changes
  pixman behavior, you have no easy way to verify that the only things
  that change are those you expect to change.

- Make it easy to generate regression tests.

  The idea with that is to have a separate test program that ensures
  that earlier bugs never happen again. So it would be useful if the
  test suite could print out sufficient information to reproduce a
  bug.

- Make it easy to pinpoint the failing test.

  The procedure to get to the failing test is pretty cumbersome
  because it involves compiling pixman twice and manually copying a
  reference binary around, then running a script. It seems this should
  be automatable. It also seems like it should be possible to compare
  the various implementations directly if it were possible to turn
  them on and off at runtime

- Make it possible to *see* what is going wrong.

  Quite often, you can tell from looking at the output what is going
  on. Also when you expect behavior changes, visual output allows you
  to verify that the new results look correct.

> The following optimized pixman functions have been found to produce results, 
> at least different from C variant:
> 
> ARM NEON: neon_composite_over_n_8_0565, neon_composite_over_n_0565,
> neon_composite_over_8888_0565
> 
> MMX: mmx_composite_over_n_8_0565, mmx_composite_over_n_8_8888,
> mmx_composite_over_n_8888, mmx_composite_over_n_0565,
> mmx_composite_src_n_8_8888, mmx_composite_in_n_8_8
> 
> VMX: vmx_combine_atop_u, vmx_combine_atop_reverse_u,
> vmx_combine_xor_u

Very cool. We should get these fixed before 0.16.0. I'm a bit
surprised that things like mmx_composite_over_n_8_8888() fails because
this is the 'text' operation, and it was used very heavily before
Andre wrote the sse2 backend.


Thanks,
Soren


More information about the cairo mailing list