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. > 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. Cheers, Daniel _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
