[cairo] [cairo-commit] src/cairo-skia-surface.cpp

[Uli Schlachter]

Hi,

On 14.07.2014 10:36, RAVI NANJUNDAPPA wrote:
> In such a case,"return  _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_INVALID_FORMAT));"  ( the earlier code) 
> makes more sense, as it returns a NULL backend surface with the valid status result.  
> Let me know if I'm missing anything.

The caller will interpret the NULL as "not supported" and will then "do
something else" to try and continue. If you return an error surface, then it
will just stop trying. Look e.g. at cairo_surface_create_similar() or
_cairo_surface_create_scratch().

Also, I just noticed the following:

>From cairo-skia-surface.cpp:

[...]
static const struct _cairo_surface_backend
cairo_skia_surface_backend = {
    CAIRO_SURFACE_TYPE_SKIA,
    _cairo_skia_surface_create_similar,
    _cairo_skia_surface_finish,
    _cairo_skia_surface_acquire_source_image,
    _cairo_skia_surface_release_source_image,
    _cairo_skia_surface_acquire_dest_image,
    _cairo_skia_surface_release_dest_image,
[...]

>From cairo-surface-backend-private.h:

[...]
struct _cairo_surface_backend {
    cairo_surface_type_t type;
    cairo_warn cairo_status_t
    (*finish)			(void			*surface);
    cairo_t *
    (*create_context)		(void			*surface);
    cairo_surface_t *
    (*create_similar)		(void			*surface,
				 cairo_content_t	 content,
				 int			 width,
				 int			 height);
    cairo_surface_t *
    (*create_similar_image)	(void			*surface,
				 cairo_format_t		format,
				 int			 width,
				 int			 height);
    cairo_image_surface_t *
    (*map_to_image)		(void			*surface,
				 const cairo_rectangle_int_t  *extents);
    cairo_int_status_t
    (*unmap_image)		(void			*surface,
				 cairo_image_surface_t	*image);
[...]

Since cairo-skia seems to be unmaintained and no even noticed that it does not
work AT ALL in ages, can't we just drop it? (Does this even compile? I would
have expected the C++ compiler to complain loudly about these implicit casts.)

I will from now on ignore cairo-skia.

Cheers,
Uli

> ------- Original Message -------
> Sender : Uli Schlachter<psychon at znc.in>
> Date : Jul 14, 2014 13:44 (GMT+05:30)
> Title : Re: [cairo] [cairo-commit] src/cairo-skia-surface.cpp
> 
> Hi Ravi,
> 
> On 14.07.2014 08:42, RAVI NANJUNDAPPA wrote:
>> Hi Chris, 
>>
>> Please see if this patch fulfils the need. 
>>
>> Thanks, 
>> N Ravi
>>
>>> -----Original Message-----
>>> From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
>>> Sent: Friday, July 11, 2014 11:28 AM
>>> To: RAVI NANJUNDAPPA
>>> Cc: cairo at cairographics.org; cairo-commit at cairographics.org
>>> Subject: Re: Re: [cairo] [cairo-commit] src/cairo-skia-surface.cpp
>>>
>>> On Fri, Jul 11, 2014 at 04:33:52AM +0000, RAVI NANJUNDAPPA wrote:
>>>> Hi Chris,
>>>>
>>>> I understand your point.
>>>> May be in such a case, where the user has not explicitly passed any
>> format
>>> type, then we can fallback to   " _cairo_image_surface_create_with_content
>>> (content, width, height);" statement,
>>>> which ideally should be the case (I'm using this from
>>> _cairo_gl_surface_create_similar()).
>>>> I hope this is fine ?
>>>
>>> If the create_similar backend returns NULL, it is an indication that the
>>> operation is unsupported for that backend. The caller then decides how to
>>> handle the unsupported operation. In the user facing function, an image
>>> surface is created instead.
>>> -Chris
> 
> The attached effectively reverts your change and makes this return NULL again,
> but it also adds a comment explaining why this shouldn't make a difference.
> 
> What Chris meant is: The old code returned NULL and that is the right thing to
> do. The calling function has to be able to handle NULL and fall back in this
> case. But if you return an error surface, no fallback happens and the error is
> propagated. And implementing the fallback here in cairo-skia isn't necessary and
> would be inconsistent with other backends.
> 
> This isn't a git formatted patch on purpose. Someone else can handle this.
> 
> Uli
> 


-- 
"Every once in a while, declare peace. It confuses the hell out of your enemies"
 - 79th Rule of Acquisition