Re: JPEG Support For Libcairo

On Wednesday 30 December 2015 14:48:53 Uli Schlachter wrote:
> Hi,
> 
> Am 28.12.2015 um 15:12 schrieb Bernhard Fischer:
> > I implemented two small functions which read/write JPEG into/from a Cairo
> > image surface.
> > 
> > Please find more info here:
> > https://www.cypherpunk.at/2015/12/jpeg-support-for-libcairo/
> 
> Let's look at some of the code:
> 
>    // check valid input format
>    if (cairo_surface_get_type(sfc) != CAIRO_SURFACE_TYPE_IMAGE &&
>          cairo_image_surface_get_format(sfc) != CAIRO_FORMAT_ARGB32 &&
>          cairo_image_surface_get_format(sfc) != CAIRO_FORMAT_RGB24)
>       return CAIRO_STATUS_INVALID_FORMAT;
> 
> If I call this code with an image surface in format A8, the individual
> comparisons evaluate to (true && false && false), so the whole if still
> evaluates to false. You want to do:
> 
>    if (get_type() != IMAGE || (get_format() != ARGB32 && get_format() !=
> RGB24)) return;
> 
> For readability, I'd suggest:
> 
>   cairo_format_t format;
> 
>   if (cairo_surface_get_type(sfc) != CAIRO_SURFACE_TYPE_IMAGE)
>      return CAIRO_STATUS_INVALID_FORMAT;
> 
>   format = cairo_image_surface_get_format(sfc)
>   if (format != CAIRO_FORMAT_ARGB32 && format != CAIRO_FORMAT_RGB24)
>      return CAIRO_STATUS_INVALID_FORMAT;
> 
> Cheers,
> Uli
> 
> P.S.: Actually... Why can't this code just copy other surfaces to an image
> surface, if needed? Thanks to cairo_surface_get_content() and
> cairo_surface_create_similar_image(), this should be relatively easy to do.
> The only complication is to get the size of the surface, but for that you
> could use cairo_create() and cairo_clip_extents() (which would still fail
> for unbounded recording surfaces...).


Thank you very much for your audit.
In the meanwhile I incorporated your suggestions into the code.

(* incorrect boolean operators)
(* surface format conversion)


Bernhard

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