Testing output from each backend (was: [cairo] State of Win32 backend?)

[Owen Taylor]

On Fri, 2005-05-06 at 16:49 +0300, Tor Lillqvist wrote:
> Carl Worth writes:
>  > One thing that would help those people who want to work on the win32
>  > (or almost any other backend) is to tweak the test suite so that it
>  > will test these backends. 
> 
> OK, here is the (trivial) patch that adds support for the Win32
> backend to cairo-test.c. BTW, if I used CAIRO_FORMAT_RGB24, the tests
> all fail, the win32 pngs were mostly black. Apparently there is some
> mismatch between Cairo's RGB24 format pixmaps and those in Windows, or
> something.

The test are expected to fail in this case ... the reference images
are four-channel. But if the images don't look right at all when
using RGB24, then that might indicate a problem with 
cairo_surface_write_png().

> (I also had to rework the stderr acrobatics. stderr isn't an lvalue in
> Microsoft's C library. Instead I manipulate fileno(stderr), which
> hopefully is more portable?)

Looks equally (or more) unportable to me. The only correct way to
do this operation under Unix is to use dup2(), so unless that works
under Windows (seems unlikely) it probably needs conditionalization.

Or just abandon the stderr redirection ... the only benefit is if they
are testing things that are documented as programmer error ... 
Cairo must be silent in all other cases.

> @@ -168,5 +169,19 @@
>  set_win32_target (cairo_t *cr, int width, int height, void **closure)
>  {
> -#error Not yet implemented
> +    cairo_surface_t *surface;
> +    cairo_surface_t *_cairo_win32_surface_create_dib (cairo_format_t format,
> +						      int	     width,
> +						      int	     height);
> +
> +    if (width == 0)
> +	width = 1;
> +    if (height == 0)
> +	height = 1;
> +
> +    *closure = surface = _cairo_win32_surface_create_dib (CAIRO_FORMAT_ARGB32, width, height);
> +
> +    cairo_set_target_surface (cr, surface);
> +
> +    return CAIRO_TEST_SUCCESS;
>  }

I don't like this much. The stuff in test/ should only be using public
API ... the above isn't even going to work if we start properly
controlling exports to the DLL, right?

Now the basic conceptual problem is that there aren't hardware
accelerated ARGB32 destination surfaces in the win32 backend ....
so you aren't going to test any of the "hard parts" of the win32
backend. All rendering to ARGB32 destination surfaces is done
with libpixman.

This is why there is no public API for creating ARGB32 destinations
for the win32 backend other than create_similar() [ and create
similar creates image surfaces in some cases ] ... the public
API is cairo_image_surface_create().

I don't really have an idea ... perhaps win32 test cases here just
aren't useful. While they found one bug (in the below) they are
only testing a very small fraction of the code, and adding more
test cases doesn't test anything new.
 
> Index: src/cairo-win32-surface.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo-win32-surface.c,v
> retrieving revision 1.19
> diff -u -2 -r1.19 cairo-win32-surface.c
> --- src/cairo-win32-surface.c   26 Apr 2005 02:38:44 -0000      1.19
> +++ src/cairo-win32-surface.c   6 May 2005 13:47:33 -0000
> @@ -195,5 +195,5 @@
> 
>      if (rowstride_out) {
> -       /* Windows bitmaps are padded to 16-bit (word) boundaries */
> +       /* DIB section rows are padded to 32-bit (DWORD) boundaries */
>         switch (format) {
>         case CAIRO_FORMAT_ARGB32:

Hmm, the docs actually say LONG rather than DWORD, but I guess those
two are always the same.

> @@ -203,9 +203,9 @@
> 
>         case CAIRO_FORMAT_A8:
> -           *rowstride_out = (width + 1) & -2;
> +           *rowstride_out = ((width - 1)/4 + 1) * 4;
>             break;
> 
>         case CAIRO_FORMAT_A1:
> -           *rowstride_out = ((width + 15) & -16) / 8;
> +           *rowstride_out = ((width - 1)/32 + 1) * 4;
>             break;
>         }

These gives bad results for width == 0... while that may not 
matter much, I'd rather see correct expressions. It's easy
to adapt the original to the dword alignment case.

+ 3 ) & -4 .... + 15) & -32) / 8;

Regards,
					Owen

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050509/9140c367/attachment.pgp