On Fri, Mar 1, 2013 at 10:01 PM, Ian Romanick <i...@freedesktop.org> wrote: > On 02/27/2013 10:19 PM, John Kåre Alsaker wrote: >> >> On Mon, Feb 25, 2013 at 8:55 PM, Ian Romanick <i...@freedesktop.org> wrote: >>> >>> Also... are there piglit tests coming? >> >> Not unless you convince me otherwise. I don't think I'll be able to >> verify that said tests work however. >> >>> More recent versions of the spec template include a section for >>> describing >>> conformance tests. This would be a good place to document piglit tests. >>> It >>> also helps app developers know how things should work. >> >> Where is this template? > > > I don't think the template is officially available anywhere, but the > ARB_internalformat_query spec is a good example: > > http://www.opengl.org/registry/specs/ARB/internalformat_query.txt > > I'm uncomfortable adding new functionality that has zero tests. Every time > we've done this, we've gotten bitten by it. At the minimum it should be > possible to have tests that verify: > > - Calling eglCreateImageKHR with {EGL_GAMMA_MESA, EGL_DEFAULT_MESA} always > works. > > - Calling eglCreateImageKHR with {EGL_GAMMA_MESA, <some invalid value>} > always generates EGL_BAD_VIEW_MESA. That sounds possible.
>>> What if the underlying driver doesn't support sRGB? >> >> It should just return EGL_BAD_VIEW_MESA then. > > > Isn't it better to not expose the extension at all in that case? If the > only operations you ever want to do with the extension will always fail, > what's the point? :) Otherwise, I suspect we'll get spurious bug reports. I'm not sure how to tell if the driver supports sRGB without actually trying it, but I should at least make it depend on the DRIimage version it needs. >>> Shouldn't attrs sill be const? >> >> If Mesa does const by default, then sure. > > > The parameters were const before your patch. That's why I was asking. It's a new parameter, the old one is gone. > > >>> Why can't I create an sRGB view of a pixmap? It's just a reinterpretation >>> of the bits. >> >> Because it's not implemented. I prefer not to touch code I won't even >> manually test after (no pixmaps in Wayland). > > > In that case, since this is likely the only implementation, the spec should > just disallow it. Principle of least surprise and all that. Looking over I think it's probably best to move the code into dri2_create_image and expose that to the platform_*.c files. Then it will work for all EGL images. > > >>> The spec doesn't give much guidance about why the >>> implementation may not "support creating the specified gamma view". At >>> least mentioning something in the issues section would be helpful. >> >> I could probably add something. >> >>> I know the ifdef stuff is used other places, but since this comes from >>> our >>> own header file, I don't think it's necessary. >> >> Surrounding code had it. > > > I understand that. I don't think it's necessary in this case, and it's > probably not necessary in the other cases either. I'll remove it then. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev