On Tuesday, 2019-01-22 15:29:44 +0000, Eric Engestrom wrote: > On Tuesday, 2019-01-22 13:43:07 +0000, Emil Velikov wrote: > > Hi Eric, > > > > Thanks for writing this up. > > > > On Tue, 22 Jan 2019 at 12:43, Eric Engestrom <eric.engest...@intel.com> > > wrote: > > > > > > Cc: Veluri Mithun <velurimithu...@gmail.com> > > > Cc: Emil Velikov <emil.l.veli...@gmail.com> > > > Cc: Rob Clark <robdcl...@gmail.com> > > > Cc: Nicolai Hähnle <nicolai.haeh...@amd.com> > > > Signed-off-by: Eric Engestrom <eric.engest...@intel.com> > > > --- > > > The extension is currently in development in this MR: > > > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47 > > > > > > Veluri will send updated versions of this test if the spec changes. > > > > > + > > > + piglit_require_egl_extension(EGL_NO_DISPLAY, > > > "EGL_MESA_query_driver"); > > > + > > AFAICT we need an valid/initialized display here. EGL_NO_DISPLAY is > > for _client_ EGL extensions and EGL_MESA_query_driver is not one. > > Good point, indeed. > > > > > > > > + egl_error = eglGetError(); > > > + if (driver_name || egl_error != EGL_BAD_DISPLAY) { > > > + printf("eglGetDisplayDriverName() should have failed with > > > EGL_BAD_DISPLAY\n"); > > > + printf("Instead, it returned %s and with error %s\n", > > > + driver_name, piglit_get_egl_error_name(egl_error)); > > > + piglit_report_result(PIGLIT_FAIL); > > > + } > > > + > > This hunk seems to be an open-coded piglit_check_egl_error(), use the > > helper instead? > > Suggestion applies for the whole file. > > Yup, didn't know about the helper :) > > > > > > + printf("Driver name: %s\n", driver_name); > > > + printf("Driver config: %s\n", driver_config); > > > + free(driver_config); > > > + > > I'd add a "TODO: add basic xml config validation/printing based on the > > example in the spec." > > That would be good in theory, but: > - we'd need to write a proper DTD, which we don't have right now > - xml parsing is not trivial (we'd need a library, it's not something > one should do by hand), and I'm not sure we want to add that kind of > beast to piglit. > > If we decide to do this, I'm happy to write the DTD.
Actually, after thinking about this a bit more, I think we really should have a DTD, and it should be part of the extension spec. I've started writing it, but I don't have much time left today. I'll send it once it's done. _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
