[cairo] 1.6: a8-mask: Making non-aligned strides illegal
behdad at behdad.org
Mon Jan 28 17:07:27 PST 2008
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
> 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
> * 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
#define STRIDE_ALIGNMENT (sizeof (uint32_t))
if (stride & (STRIDE_ALIGNMENT-1) != 0)
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.
"Those who would give up Essential Liberty to purchase a little
Temporary Safety, deserve neither Liberty nor Safety."
-- Benjamin Franklin, 1759
More information about the cairo