Re: patch [1/1] invert image in _cairo_gl_surrface_map_to_image

Hey, Andrea

I looked at your negative stride image code, 
The effect of calling cairo_image_surface_create_for_data (data + stride * (height - 1), format, width, height, -stride) is same as calling
_cairo_image_surface_create_with_pixman_format (data, format, width, height, -1), the stride parameter is tested and ignored in cairo-image-surface.c 

I don't see invert image comes out.

Thanks

Henry


-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Andrea Canciani
Sent: Wednesday, January 04, 2012 11:08 AM
To: Henry Song
Cc: [email protected]
Subject: Re: [cairo] patch [1/1] invert image in _cairo_gl_surrface_map_to_image

On Wed, Jan 4, 2012 at 4:57 PM, Henry Song <[email protected]> wrote:
> Hi, Andrea
>
> The patch creates a dst of inv_image and the source is image,  they are not
> same, aren't they?

Sorry, you're right, I misread the pattern creation.
Andrea

>
> Thanks
>
> Henry
>
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of
> Andrea Canciani
> Sent: Wednesday, January 04, 2012 12:43 AM
> To: Henry (Yu) Song - SISA
> Cc: [email protected]
> Subject: Re: [cairo] patch [1/1] invert image in
> _cairo_gl_surrface_map_to_image
>
> On Wed, Jan 4, 2012 at 1:56 AM, Henry (Yu) Song - SISA
> <[email protected]> wrote:
>> Date:   Fri Dec 30 11:24:54 2011 -0800
>>
>>    gl: Add support to invert non texture gl surface image that does not
> have
>>    GL_MESA_pack_invert when map from gl surface to image surface
>>
>> diff --git a/src/cairo-gl-surface.c b/src/cairo-gl-surface.c
>> index 8f69033..8b8f196 100644
>>
>> --- a/src/cairo-gl-surface.c
>> +++ b/src/cairo-gl-surface.c
>> @@ -1001,6 +1001,10 @@ _cairo_gl_surface_map_to_image (void
>  *abstract_surface,
>>     unsigned int cpp;
>>     cairo_bool_t invert;
>>     cairo_status_t status;
>> +    cairo_t *inv_cr = NULL;
>> +    cairo_image_surface_t *inv_image = NULL;
>> +    cairo_pattern_t *inv_pattern = NULL;
>> +    cairo_matrix_t inv_matrix;
>>
>>     /* Want to use a switch statement here but the compiler gets whiny. */
>>     if (surface->base.content == CAIRO_CONTENT_COLOR_ALPHA) {
>> @@ -1093,6 +1097,50 @@ _cairo_gl_surface_map_to_image (void
>  *abstract_surface,
>>        image = (cairo_image_surface_t *)
>>            _cairo_surface_create_in_error (status);
>>     }
>> +
>> +    /*
>> +     * FIXME: we must invert the image if it is non texture surface,
>> +     * and it does not have GL_MESA_pack_invert.
>> +     * Is there more efficient way to invert image?
>
> Using negative stride might be more efficient, but some backends might
> not (yet) support them correctly.
>
> Side note:
> It is already possible to create negative stride images.
> Should we add a test trying to use them and support them?
> Unless I get feedback, I'll try to write the test this weekend.
>
>> +     */
>> +
>> +    if (! _cairo_gl_surface_is_texture (surface) && ! invert) {
>> +        inv_image = (cairo_image_surface_t*)
>> +       _cairo_image_surface_create_with_pixman_format (NULL,
>> +                                                       pixman_format,
>> +                                                       extents->width,
>> +                                                       extents->height,
>> +                                                       -1);
>> +       if (unlikely (inv_image->base.status)) {
>> +           goto CLEAR_IMAGE;
>> +       }
>> +
>> +       image->base.is_clear = FALSE;
>> +       inv_pattern = cairo_pattern_create_for_surface (&image->base);
>> +       if (unlikely (inv_pattern->status)) {
>> +           goto CLEAR_PATTERN;
>> +       }
>> +       cairo_matrix_init_scale (&inv_matrix, 1.0, -1.0);
>> +       cairo_matrix_translate (&inv_matrix, 0, -(extents->height));
>> +       cairo_pattern_set_matrix (inv_pattern, &inv_matrix);
>> +
>> +       inv_cr = cairo_create (&inv_image->base);
>> +       if (unlikely (inv_cr->status)) {
>> +           goto CLEAR_CAIRO;
>> +       }
>> +       cairo_set_source (inv_cr, inv_pattern);
>> +       cairo_set_operator (inv_cr, CAIRO_OPERATOR_SOURCE);
>> +       cairo_paint (inv_cr);
>
> You're using the same surface as source and as destination.
> This results in undefined behavior.
> If you want to flip the image like this, you should use two independent
> surfaces
>
>
>> +       cairo_surface_destroy (&image->base);
>> +       image = (cairo_image_surface_t *)cairo_surface_reference
> (&inv_image->base);
>> +    }
>> +
>> +CLEAR_CAIRO:
>> +    cairo_destroy (inv_cr);
>> +CLEAR_PATTERN:
>> +    cairo_pattern_destroy (inv_pattern);
>> +CLEAR_IMAGE:
>> +    cairo_surface_destroy (&inv_image->base);
>>
>>     return &image->base;
>>  }
>> --
>> cairo mailing list
>> [email protected]
>> http://lists.cairographics.org/mailman/listinfo/cairo
> --
> cairo mailing list
> [email protected]
> http://lists.cairographics.org/mailman/listinfo/cairo
>
--
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo
--
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo