[cairo] 1.6: a8-mask: Making non-aligned strides illegal

[Behdad Esfahbod]

On Mon, 2008-01-28 at 15:18 -0800, Carl Worth wrote:
> On Mon, 28 Jan 2008 16:59:05 -0500, Behdad Esfahbod wrote:
> > Right.  Subsurfacing is only one use case for using a greater width
> > value for stride computation than the actual width.
> >
> > I still prefer at least allowing "any width" for stride.
> 
> Yes, it should still be legal to use a smaller width as long as the
> stride matches what was allocated and the stride came from
> cairo_format_stride_for_width.
>
> I've rewritten the documentation again to clarify it, and this is what
> I've come up with:
> 
>  * @stride: the number of bytes between the start of rows in the
>  *     buffer as allocated. This value should always be computed by
>  *     cairo_format_stride_for_width() before allocating the data
>  *     buffer.

Other question: is a negative stride supported/allowed?  FreeType uses
them for example.  In new libraries I write I always use unsigned int
where negative values are not allowed.  It makes things so much
easier...

...
>  * Note that the stride will often be larger than
>  * width*bytes_per_pixel to provide proper alignment for each

I'd replace "will often" by "may be".  It isn't for ARGB32 after all.

Also this text ignores the fact that RGB24 is 3 bytes per pixel worth of
data but takes 4.  But doesn't matter.

>  * row. This alignment is required to allow high-performance rendering
>  * within cairo. The correct way to obtain a legal stride value is to
>  * call cairo_format_stride_for_width() with the desired format and
>  * maximum image width value, and the use the resulting stride value
>  * to allocate the data and to create the image surface. See
>  * cairo_format_stride_for_width() for example code.
> ...
>  * This function always returns a valid pointer, but it will return a
>  * pointer to a "nil" surface in the case of an error such as out of
>  * memory or an invalid stride value. You can use
>  * cairo_surface_status() to check for this.
> 
> I think that's the best I'm going to come up with for now, so I'll
> just push that. But feel free to tweak it further if you see any ways
> to improve it.

Is quite good actually other than the minor nits above.

> > Right.  Also, in the patch I'd define a macro and use that instead of
> > hardcoding the 32-bit requirement in more than one place.
> 
> I can throw in a LEGAL_STRIDE macro for (stride % sizeof (uint32_t) ==
> 0), but I'm only seeing that in one place
> (cairo_image_surface_create_for_data). Am I missing something? It's
> actually quite different what we need in
> cairo_format_stride_for_width.

#define STRIDE_ALIGNMENT (sizeof (uint32_t))

in cairo_image_surface_create_for_data:

  if (stride & (STRIDE_ALIGNMENT-1) != 0)
     err...

in cairo_format_stride_for_width:

    return ((bpp*width+7)/8 + STRIDE_ALIGNMENT-1) & ~(STRIDE_ALIGNMENT-1)


> Or shall I do the macro and assert that the stride I compute in
> stride_for_width satisfies it?
> 
> > Sounds fine.  I marginally prefer requiring int alignment than any-type
> > alignment.  But who knows what we'll require in the future...
> 
> Right. Documenting things as safer than strictly necessary is a nice
> place to be. With that, we still have the freedom to loosen things up
> in the future if needed.

True.  But you don't want to document so strictly that people keep
relying on documented-as-undefined behavior.

> -Carl
-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759