On 11 September 2017 18:33:27 BST, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 8 September 2017 at 18:36, Eric Engestrom > <eric.engest...@imgtec.com> wrote: > > On Friday, 2017-09-08 08:13:54 -0700, Kenneth Graunke wrote: > >> On Friday, September 8, 2017 6:33:44 AM PDT Emil Velikov wrote: > >> > Hi Eric, > >> > > >> > On 8 September 2017 at 13:40, Eric Engestrom > <eric.engest...@imgtec.com> wrote: > >> > > Instead of setting based on set/unset, allow users to use > boolean values. > >> > > In the docs, use `ALWAYS=true` instead of `ALWAYS=1` as it's > clearer IMO. > >> > > > >> > I'm a bit weary about this series. In the past we had cases where > >> > people will set the variable to empty string and/or 0 and expect > >> > things to work. > >> > > >> > IIRC the older Intel QA team was using it, not sure about now. > Either > >> > way it's great to confirm with a few people before pushing this. > >> > >> That team is gone. I like the consistency of using > env_var_as_boolean > >> across the codebase - it might break a few things, but I think > that's > >> okay. Most people don't use this, and most of those that do use > =1. > > > > Agreed. > > > >> > >> > That aside, env_var_as_boolean() treats 'yes', '1' and 'true' in > >> > identical manner. Perhaps we can state what a "boolean variable" > is > >> > and use that reference through the documentation? > >> > > >> > -Emil > >> > >> That seems like a good idea. > > > > Yes indeed, and I'll do that, but as a follow up patch. > Sure thing. > > > I'm thinking of adding something like this at the top of > envvar.html: > > > > There are different kind of environment variables: > > - BOOL: expects 1/true/yes or 0/false/no. > > - INT: expects an integer value. > > - ENUM: expects an string, but only a limited set of > values is accepted. > > - PATH: expects a file or folder path. > > - NAME: expects a string name (like a driver name), but > not a full path. > > - NON-NULL: any value enables it (including empty string); > use `unset` to disable. > > Note: some variables expect more complicated values, like > > MESA_EXTENSION_OVERRIDE. > > > > and then each env var in the doc would get an annotation: > > LIBGL_ALWAYS_SOFTWARE (BOOL) > > LIBGL_SHOW_FPS (INT) > This should really be a bool, but that's for another time.
If memory serves, this is the number of frames to average the FPS over, but we could fix it and turn this env into a bool. > > > EGL_LOG_LEVEL (ENUM: debug, info, warning, fatal) > > EGL_DRIVERS_PATH (PATH) > > EGL_DRIVER (PATH or NAME) > > EGL_PLATFORM (NAME) > > ... etc. > > > > Hopefully, this should be clear enough. The last option (NON-NULL) > would > > be applied to the old-style toggles. > > > Seems pretty clear over here. Although damn man... that's really nice > and comprehensive. > > > For now, there seems to be another issue; I sent this series to > travis, > > and EGL is failing to link [1] because every exported symbol in > mesautil > > appears twice; some sort of double-link issue? > > I don't know if I'm tired or thick, but I can't figure it out :] > > > > [1] https://travis-ci.org/1ace/mesa/jobs/273302410#L6797 > > > > I'd appreciate any help :) > > > I've spotted it - libEGL_common conditionally uses libmesautil.la (for > the wayland code). > I'd just make that unconditionally and omit it from > libEGL/libEGL_mesa. > > With the above fix, the patch is > Reviewed-by: Emil Velikov <emil.veli...@collabora.com> > > -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
