On Fri, Mar 02, 2018 at 12:28:02PM +0000, Daniel Stone wrote: > Hi, > > On 2 March 2018 at 12:06, Thierry Reding <thierry.red...@gmail.com> wrote: > > On Thu, Mar 01, 2018 at 09:37:28AM -0500, Ilia Mirkin wrote: > >> > +static void > >> > +nvc0_query_dmabuf_modifiers(struct pipe_screen *screen, > >> > + enum pipe_format format, int max, > >> > >> Maybe change this to "unsigned max"? That would avoid unnecessary > >> complications below to check if it's negative. > > > > I would like that very much, but I'm afraid it's probably too late to > > change this now because the signedness is handed down directly from the > > EGL_EXT_image_dma_buf_import_modifiers extension: > > > > > > https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt > > > > Adding Pekka and Daniel for visibility, maybe there were reasons for why > > these are explicitly signed. I'm not familiar with the process of > > getting an extension changed, but, even though a very minor change, this > > would be changing the API of a completed extension, which doesn't seem > > like something that you're supposed to do. > > The main reason was the precedent of, e.g., eglChooseConfig using > EGLint. It's not ideal, but then again every time I type 'int i' these > days I get a momentary shiver not knowing if it should be an unsigned > int or not.
It also seems like EGL doesn't even define EGLuint, which would be a good reason why everything is signed in the EGL API. > > That said, the revision history of the extension mentions that revision > > 4 introduced a new type for the modifiers, so perhaps there is some > > leeway in what we can do. > > r4 was made before the extension was actually finalised/used anywhere, > so it's not really precedent. > > > I guess we could always force a cast in dri2_query_dma_buf_modifiers() > > if changing the extension is not possible. That way we could at least > > internally not worry about the signedness. > > If you think that'd make for a better Gallium interface, by all means > please go for it, especially if it makes sense in the context of the > rest of the Gallium API. Yeah, I think it'd make sense. Gallium is already fairly consistently using unsigned for values that can never be negative. I was already halfway through the conversion when I noticed that this comes originally from the EGL extension, so I'll wrap all of it into a patch and send it out for review. Thanks, Thierry
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
