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.pohjolai...@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. _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit