Re: [PATCH 1/3] Enlarge fallback surface

On 17.04.2014 11:06, LRN wrote:
> Enlarges the fallback surface created of the amount
> necessary not to write past the end of the DIB. It assumes that
> Clip applied to an HDC are clamped to (0,0,width,height) of the HDC.
> 
> Fixes bug 53121

How hard would it be to write a new test case for the test suite that catches
this bug? This would mean it would need to be a test not using any
(cairo-)win32-specific APIs.

> Signed-off-by: Massimo Valentini <[email protected]>
>
> ---
>  src/win32/cairo-win32-display-surface.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/win32/cairo-win32-display-surface.c b/src/win32/cairo-win32-display-surface.c
> index 5ecdbee..965f2c4 100644
> --- a/src/win32/cairo-win32-display-surface.c
> +++ b/src/win32/cairo-win32-display-surface.c
> @@ -455,17 +455,17 @@ _cairo_win32_display_surface_map_to_image (void                    *abstract_sur
>  	surface->fallback =
>  	    _cairo_win32_display_surface_create_for_dc (surface->win32.dc,
>  							surface->win32.format,
> -							surface->win32.extents.width,
> -							surface->win32.extents.height);
> +							surface->win32.extents.x + surface->win32.extents.width,
> +							surface->win32.extents.y + surface->win32.extents.height);

Can you explain (in the commit message) *why* this is necessary?

This function is asked to create a surface covering the extents "extents".
Apparently some caller is mis-using the returned surface and uses it outside of
these extents. That sounds like a bug elsewhere.

You are fixing this by just handling back a bigger surface which means we get
more memory pressure and lots of these memory is actually unused because it
contains parts of an image which will just be discarded later on.

(Feel free to correct me. Everyone else may feel free to ignore me and just
merge this. I don't have any clue about cairo-win32 and I have even less clue
about win32 in general. And I don't plan on touching code that I have no clue
about and which I cannot possibly test, sorry.)

>  	if (unlikely (status = surface->fallback->status))
>  	    goto err;
> 
>  	if (!BitBlt (to_win32_surface(surface->fallback)->dc,
> -		     0, 0,
> +		     surface->win32.extents.x, surface->win32.extents.y,
>  		     surface->win32.extents.width,
>  		     surface->win32.extents.height,
>  		     surface->win32.dc,
> -		     0, 0,
> +		     surface->win32.extents.x, surface->win32.extents.y,
>  		     SRCCOPY)) {
>  	    status = _cairo_error (CAIRO_STATUS_DEVICE_ERROR);
>  	    goto err;

Uli
-- 
A normal person is just someone you don't know well enough yet.
 - Nettie Wiebe
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo