[cairo] [PATCH]: gl: Return surface in error when creating oversized texture surfaces
mrobinson at igalia.com
Mon Jul 29 14:02:34 PDT 2013
On 07/29/2013 01:35 PM, Chris Wilson wrote:
> (Note there is already a function _cairo_gl_surface_size_valid, maybe
> extend that to handle read-only/write-only/read-write surfaces?)
Oof. Can't believe I missed that. Thanks for pointing it out.
> The majority of its callers do seem to be internal and only expect a gl
> surface to be created.
It's true that this is mainly aimed at cairo_gl_surface_create, which is
> I feel a little uneasy about changing the user API though to error out
> if asked to handle a surface too large for hw. In most cases it is
> better to be upfront about when they are going to encounter pain, but on
> the other hand we have a long history of hiding such hardware details.
> (Though I thought we did expose the limits through queries on the
> cairo_gl_context_t device.) The problem is that though the hardware may
> only support 8k coordinates, it may be be sample from and render to
> textures 64k x 64k in size (and technically even larger), for instance
> Intel hw. Whether or not OpenGL is ever capable of expressing such
> operations and their limits sanely is another question. But imposing
> this limit feels like we are artificially constraining the hw and
> preventing common use cases.
If you are more comfortable with it, I can simply replicate the check at
the beginning of _cairo_gl_surface_create_similar and the associated
error surface. I think that's sufficient for what we need in WebKit.
> This patch also needs to extend (create!) the documentation for
> cairo_gl_surface_create() explaining the various limits and how to
> implement a fallback, such as
> surface = cairo_gl_surface_create(device, content, width, height);
> if (cairo_surface_status(surface) == CAIRO_STATUS_INVALID_SIZE)
> surface = cairo_image_surface_create(choose_format_from_content(content),
> width, height);
> and perhaps remind the reader that cairo_surface_create_similar() will
> do a similar substitution automatically.
>> + oversized_surface = cairo_gl_surface_create (device, CAIRO_CONTENT_COLOR_ALPHA, INT_MAX, INT_MAX);
>> + assert (cairo_surface_status (oversized_surface) == CAIRO_STATUS_INVALID_SIZE);
> This should just set the test_status = CAIRO_TEST_FAILURE and keep going.
> So that we don't rely on the test process being forked (i.e. under
> gdb and valgrind etc).
I've spent too much time writing tests for other harnesses. :) I'll fix
More information about the cairo