[Piglit] [RFC 1/3] framework: introduce interface for external buffers

[Pohjolainen, Topi]

On Fri, Mar 01, 2013 at 11:25:50AM -0800, Chad Versace wrote:
> On 03/01/2013 10:57 AM, Chad Versace wrote:
> > On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> >> In order to test the OES_EGL_image_external with planar formats such
> >> as YUV420 or NV12, one needs a way for creating buffers that can be
> >> passed to EGL and filling them with YUV-data for the GL-stack to
> >> sample.
> >> By the nature the extension in question deals with platform specific
> >> buffers and hence the idea here is to push the details down into the
> >> platform specific logic leaving the tests themselves platform
> >> independent.
> >>
> >> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> >> ---
> >>  tests/util/piglit-framework-gl.c                   |   29 ++++++++++++++++++++
> >>  tests/util/piglit-framework-gl.h                   |   21 ++++++++++++++
> >>  .../util/piglit-framework-gl/piglit_gl_framework.h |   23 ++++++++++++++++
> >>  3 files changed, 73 insertions(+)
> 
> 
> >> +void *
> >> +piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
> >> +			const void *y, const void *u, const void *v)
> >> +{
> >> +	if (!gl_fw->create_external_420_buffer)
> >> +		return NULL;
> >> +
> >> +	return gl_fw->create_external_420_buffer(gl_fw, w, h, swap_vu, y, u, v);
> >> +}
> > 
> > Later, in patch 3, you write the test so that, if piglit_create_ext_420_buf()
> > fails then the test reports PIGLIT_FAIL. But the convention in piglit is to report
> > PIGLIT_SKIP when the platform is incapable of running the test.
> > 
> > There are two choices to handle this:
> >   1. Here, in piglit_create_ext_420_buf(), report PIGLIT_SKIP if it fails
> >      to create the buffer.
> > 
> >   2. In the testfile itself, report PIGLIT_SKIP if piglit_create_ext_420()
> >      fails.
> 
> 
> My opinion here has changed after I thought about it more. I think that the
> piglit_gbm_framework_create() should set gl_fw->create_external_420_buffer
> only if the platform is capable of creating a yuv420 buffer.
> 
> If a test calls piglit_create_ext_420_buf(), but the function is unable to create
> the buffer because the platform doesn't support it (that is, because
> gl_fw->create_external_420_buffer is NULL), then the test should report PIGLIT_SKIP.
> That is the convention in piglit; if a test attempts to test something that the
> platform is incapable of doing, then it reports PIGLIT_SKIP. In this case,
> the best place to report PIGLIT_SKIP is here inside piglit_create_ext_420_buf().

Thanks Chad, I'll change it accordingly.