Re: [PATCH 3/3] W32: Add a win32 boilerplate that uses a real window

On Sat, Apr 04, 2015 at 07:21:20PM +0300, LRN wrote:
> See the discussion in "[PATCH 2/3] Support a different pixel format for HDC"
> for background information and the previous 2 patches.
> 
> This patch adds a boilerplate code for win32 that uses a real window (instead
> of a device-independent bitmap) as a backend for the surface.
> 
> There is a summary of testsuite results in the commit message (obtained by
> counting the number of '*.fail.png' and '*.pass.png' for a particular target.

Thanks for checking that.

> IMO, the results prove that patch 1/3 (the one that adds
> cairo_win32_surface_create_with_alpha()) does not make things worse.
> 
> -- 
> O< ascii ribbon - stop html email! - www.asciiribbon.org

> From 5e8811893688966cd29b0f63432e3ee65644d10b Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?=
>  =?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= <[email protected]>
> Date: Sat, 4 Apr 2015 15:58:53 +0000
> Subject: [PATCH 3/3] W32: Add a win32 boilerplate that uses a real window
> 
> This way it uses the codepath for cairo_win32_surface_create()
> and cairo_win32_surface_create_with_alpha(), instead of the
> cairo_win32_surface_create_with_dib().
> 
> Without the recording tests (which crash the testsuite hard)
> the testsuite results for win32 are:
> 
> win32.rgb24:
> 169 fails
> 104 passes
> 
> win32.argb32:
> 82 fails
> 173 passes
> 
> win32-window-color.rgb24:
> 162 fails
> 111 passes
> 
> win32-window-coloralpha.argb32:
> 80 fails
> 175 passes
> ---
>  boilerplate/cairo-boilerplate-win32.c | 168 ++++++++++++++++++++++++++++++++++
>  1 file changed, 168 insertions(+)
> 
> diff --git a/boilerplate/cairo-boilerplate-win32.c b/boilerplate/cairo-boilerplate-win32.c
> index 7469cc7..bda4f2d 100644
> --- a/boilerplate/cairo-boilerplate-win32.c
> +++ b/boilerplate/cairo-boilerplate-win32.c
> @@ -28,6 +28,144 @@
>  
>  #include <cairo-win32.h>
>  
> +static const cairo_user_data_key_t win32_closure_key;
> +
> +typedef struct _win32_target_closure {
> +    HWND wnd;
> +    HDC dc;
> +    cairo_surface_t *surface;
> +} win32_target_closure_t;
> +
> +static void
> +_cairo_boilerplate_win32_cleanup (void *closure)
> +{
> +    win32_target_closure_t *win32tc = closure;
> +
> +    if (win32tc)
> +    {
> +        ReleaseDC(win32tc->wnd, win32tc->dc);
> +        DestroyWindow (win32tc->wnd);
> +
> +        free (win32tc);
> +    }
> +}

It may not matter since this is cleanup, but both ReleaseDC and
DestroyWindow return error codes.

> +static void
> +_cairo_boilerplate_win32_create_window (int			width,
> +					int			height,
> +					win32_target_closure_t *win32tc)
> +{
> +    WNDCLASSEXA wincl;
> +    int format;
> +
> +    PIXELFORMATDESCRIPTOR pfd = {
> +      sizeof(PIXELFORMATDESCRIPTOR),
> +      1,                                /* Version Number */
> +      PFD_DRAW_TO_WINDOW      |         /* Format Must Support Window */
> +      PFD_SUPPORT_COMPOSITION |         /* Format Must Support Composition */
> +      PFD_DOUBLEBUFFER,                 /* Must Support Double Buffering */
> +      PFD_TYPE_RGBA,                    /* Request An RGBA Format */
> +      32,                               /* Select Our Color Depth */
> +      0, 0, 0, 0, 0, 0,                 /* Color Bits Ignored */
> +      8,                                /* An Alpha Buffer */
> +      0,                                /* Shift Bit Ignored */
> +      0,                                /* No Accumulation Buffer */
> +      0, 0, 0, 0,                       /* Accumulation Bits Ignored */
> +      24,                               /* 24Bit Z-Buffer (Depth Buffer) */
> +      8,                                /* Some Stencil Buffer */
> +      0,                                /* No Auxiliary Buffer */
> +      PFD_MAIN_PLANE,                   /* Main Drawing Layer */
> +      0,                                /* Reserved */
> +      0, 0, 0                           /* Layer Masks Ignored */
> +    };
> +
> +    ZeroMemory (&wincl, sizeof (WNDCLASSEXA));
> +    wincl.cbSize = sizeof (WNDCLASSEXA);
> +    wincl.hInstance = GetModuleHandle (0);
> +    wincl.lpszClassName = "cairo_boilerplate_win32_dummy";
> +    wincl.lpfnWndProc = DefWindowProcA;
> +    wincl.style = CS_OWNDC;
> +
> +    RegisterClassExA (&wincl);

Check for return value 0; extended error info can be obtained via
GetLastError().

> +    win32tc->wnd = CreateWindowEx (WS_EX_TOOLWINDOW,
> +				   "cairo_boilerplate_win32_dummy",
> +				   0,
> +				   WS_POPUP,
> +				   0,
> +				   0,
> +				   width,
> +				   height,
> +				   0,
> +				   0,
> +				   0,
> +				   0);

Check ->wnd for NULL; GetLastError() for error.

> +    win32tc->dc = GetDC (win32tc->wnd);

Check NULL

> +    format = ChoosePixelFormat (win32tc->dc, &pfd);

Check if format == 0; then GetLastError()

> +    SetPixelFormat (win32tc->dc, format, &pfd);

Returns FALSE on error

> +    ShowWindow (win32tc->wnd, SW_SHOWNOACTIVATE);

Safe to ignore return value here.

> +}


> +static cairo_surface_t *
> +_cairo_boilerplate_win32_for_create_window (const char		       *name,

This is an odd name for a routine.  Maybe a better name would be
something like _cairo_boilerplate_win32_create_window_surface() ?

> +					    cairo_content_t		content,
> +					    double			width,
> +					    double			height,
> +					    double			max_width,
> +					    double			max_height,
> +					    cairo_boilerplate_mode_t    mode,
> +					    void		      **closure)
> +{
> +    win32_target_closure_t *win32tc;
> +    cairo_surface_t *surface;
> +    cairo_format_t format;
> +
> +    win32tc = calloc (1, sizeof (win32_target_closure_t));

Missing NULL check

> +    *closure = win32tc;
> +
> +     _cairo_boilerplate_win32_create_window(width, height, win32tc);

Actually, why not move the calloc into *create_window, and have
*create_window return the constructed win32tc instead of taking it as a
parameter?


> +    format = cairo_boilerplate_format_from_content (content);
> +
> +    if (format == CAIRO_FORMAT_ARGB32)
> +      surface = cairo_win32_surface_create_with_alpha (win32tc->dc);
> +    else
> +      surface = cairo_win32_surface_create (win32tc->dc);

If you use my suggestion to just make an API that takes a format
parameter, you can replace this if chain with it.

> +    win32tc->surface = surface;
> +
> +    if (cairo_surface_status (surface)) {
> +	_cairo_boilerplate_win32_cleanup (win32tc);
> +	return NULL;
> +    }
> +
> +    return surface;
> +}
> +

> +static cairo_status_t
> +_cairo_boilerplate_win32_finish_window (cairo_surface_t *surface)
> +{

Would probably be a good idea to document this routine, and mention that
the surface parameter can get changed.

> +    win32_target_closure_t *win32tc = cairo_surface_get_user_data (surface,
> +								   &win32_closure_key);
> +
> +    if (win32tc != NULL && win32tc->surface != NULL) {
> +	cairo_t *cr;
> +
> +	cr = cairo_create (win32tc->surface);
> +	cairo_set_source_surface (cr, surface, 0, 0);
> +	cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
> +	cairo_paint (cr);
> +	cairo_destroy (cr);

Error checks needed in some of the above...

> +	surface = win32tc->surface;
> +    }
> +
> +    return CAIRO_STATUS_SUCCESS;
> +}
> +
>  static cairo_surface_t *
>  _cairo_boilerplate_win32_create_surface (const char		   *name,
>  					 cairo_content_t	    content,
> @@ -73,5 +211,35 @@ static const cairo_boilerplate_target_t targets[] = {
>  	cairo_surface_write_to_png,
>  	NULL, NULL, NULL, FALSE, FALSE, FALSE
>      },
> +    {
> +	"win32-window-coloralpha", "win32", NULL, NULL,
> +	CAIRO_SURFACE_TYPE_WIN32, CAIRO_CONTENT_COLOR_ALPHA, 1,
> +	"cairo_win32_surface_create_with_alpha",
> +	_cairo_boilerplate_win32_for_create_window,
> +	cairo_surface_create_similar,
> +	NULL,
> +	_cairo_boilerplate_win32_finish_window,
> +	_cairo_boilerplate_get_image_surface,
> +	cairo_surface_write_to_png,
> +	_cairo_boilerplate_win32_cleanup,
> +	NULL,
> +        NULL,
> +	FALSE, FALSE, FALSE
> +    },
> +    {
> +	"win32-window-color", "win32", NULL, NULL,
> +	CAIRO_SURFACE_TYPE_WIN32, CAIRO_CONTENT_COLOR, 1,
> +	"cairo_win32_surface_create",
> +	_cairo_boilerplate_win32_for_create_window,
> +	cairo_surface_create_similar,
> +	NULL,
> +	_cairo_boilerplate_win32_finish_window,
> +	_cairo_boilerplate_get_image_surface,
> +	cairo_surface_write_to_png,
> +	_cairo_boilerplate_win32_cleanup,
> +	NULL,
> +        NULL,
> +	FALSE, FALSE, FALSE
> +    },
>  };
>  CAIRO_BOILERPLATE (win32, targets)
> -- 
> 1.8.5.3
> 

> pub  2048R/922360B0 2011-08-05 LRN <[email protected]>
> sub  2048R/1CFF4F8C 2011-08-05 [expires: 2016-08-03]


> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (MingW32)
> 
> iQEcBAEBAgAGBQJVIA+EAAoJEOs4Jb6SI2CwiAcIAJL7+ndpWb/TEIS7RK6cW2hy
> LjG0z9vKuzYCeCgNskFPSftGXwxqc3fvzwHXE/0bqRtwRfjY6TF49Ejr83UYPUag
> eO4fAiTvlphB5Q3j9OW0J258WuV96a+pXKlNHQAtlsUWXEUL5+i/foNofgUW44vb
> fZtT8rfkpIlK4ysb6zzi31pe4tRn5sXpeTvFnLiV2GTIdLX732FrbNGVHK1OoXUc
> e/YT9ldlNP2/Qv7DgJq+s/BqY3oi1sTPnLylfczpb0VaNptRopCvh6k9SoFdyFbl
> F1b+Vr02hqmfYIzlOz5kOx0Mdm7iiGubPhY+Xn6YCRtw/OmtiRAK1fOOLiaMOnU=
> =ukX8
> -----END PGP SIGNATURE-----


> -- 
> cairo mailing list
> [email protected]
> http://lists.cairographics.org/mailman/listinfo/cairo

Bryce
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo