On 13 December 2016 at 16:35, Ilia Mirkin <imir...@alum.mit.edu> wrote: > On Tue, Dec 13, 2016 at 11:30 AM, Emil Velikov <emil.l.veli...@gmail.com> > wrote: >> On 13 December 2016 at 16:19, Ilia Mirkin <imir...@alum.mit.edu> wrote: >>> On Tue, Dec 13, 2016 at 11:14 AM, Emil Velikov <emil.l.veli...@gmail.com> >>> wrote: >>>> On 13 December 2016 at 16:09, Ilia Mirkin <imir...@alum.mit.edu> wrote: >>>>> On Tue, Dec 13, 2016 at 11:08 AM, Emil Velikov <emil.l.veli...@gmail.com> >>>>> wrote: >>>>>> On 13 December 2016 at 15:25, Ilia Mirkin <imir...@alum.mit.edu> wrote: >>>>>>> On Thu, Dec 8, 2016 at 2:21 PM, Emil Velikov <emil.l.veli...@gmail.com> >>>>>>> wrote: >>>>>>>> From: Emil Velikov <emil.veli...@collabora.com> >>>>>>>> >>>>>>>> Since day 1, Vulkan has depended on --with-egl-platforms to select the >>>>>>>> platforms build. >>>>>>>> >>>>>>>> With earlier commits, we've attributed for that internally by renaming >>>>>>>> the [internal] conditionals in our build. At the same time having the >>>>>>>> --enable-egl and --with-egl-platforms dependency for Vulkan makes no >>>>>>>> sense. >>>>>>>> >>>>>>>> Since where' on the bridge we want to make the conditional generic, >>>>>>>> thus >>>>>>>> others (such as the gallium VL targets) can honour the respective >>>>>>>> platforms. >>>>>>>> >>>>>>>> Cc: mesa-maintain...@lists.freedesktop.org >>>>>>>> Signed-off-by: Emil Velikov <emil.veli...@collabora.com> >>>>>>>> --- >>>>>>>> Should we keep the old configure and error out when anyone provides a >>>>>>>> non-default value ? This way they'll have direct feedback how to move >>>>>>>> forward. Otherwise they'll likely miss the warning message. >>>>>>> >>>>>>> I can't tell what problem this is solving. OTOH this is creating the >>>>>>> problem that everyone's configure lines will need fixing. >>>>>> This need fixing (builder intervention) either way, I'm afraid. Question >>>>>> is >>>>>> a) should we expect people to read the release notes, or >>>>>> b) get their attention with an error message, providing an example >>>>>> in order to do the right thing. >>>>> >>>>> (c) leave things alone if this isn't fixing anything other than >>>>> aesthetics. >>>> It's not aesthetics... atm if you want just Vulkan you still need >>>> --enable-egl --with-egl-platforms=x11,wayland*. >>>> Otherwise it won't have any winsys integration... Not sure if it'll >>>> build/link properly even :-( >>> >>> So error the build and say you don't support that configuration if you >>> enable vulkan and don't enable any winsys stuff. Or make it so that >>> --enable-egl sets the default value of --with-egl-platforms to >>> whatever. Don't break everyone's builds, bisects, etc. >>> >> Sounds like you've not looked at the lovely stuff we have in there. I >> welcome you to do so. >> >> That aside: >> Your suggestions contradict each other - on one hand you're saying >> "error out" in a what is buggy yet valid usecase. On the other hand >> you're against AC_MSG_ERROR since it will break X/Y usecase. > > I thought you said it didn't currently work. If it doesn't work, or is > unsupportable, error out. If it currently works, and is supportable - > keep it working. > If it works is by sheer magic. Personally I would stay away from calling it "supported". Furthermore close to nobody has any knowledge of the
>> >> Also "everyone" is a mild overstatement, but I guess we all know that ;-) > > Perhaps I'm misunderstanding. This will break everyone that has > --with-egl-platforms in their configure line, no? If not, disregard my > comments. > > I imagine that nearly everyone that does development on mesa has that > flag in their configure line. That means any bisect will require > changing the configure line as you cross this commit on the bisect. > This is very annoying. Sometimes it's required, but it shouldn't be > done lightly. > Agreed we shouldn't do these lightly and as mentioned a few times months ago "if we have a flag day we can fix the world" sadly we cannot. > So I'll ask again because it's still unclear to me - what *problem* is > this fixing? I.e. what currently doesn't work that you are trying to > make work? > Ok let me try again, starting with the mandatory backstory: [Note: a fair bit of this is in the cover letter... still] Currently there's no clear way to select which platform you want to use for Vulkan - one "inherits" it from the EGL platform selection. At the same time you do _not_ need EGL to have Vulkan. Thus as you disable EGL (and therefore its platform selection) you'll end up with Vulkan driver(s) without _any_ winsys support - not something that's intuitive and/or you'd would want most of the time. Side note: Vulkan requires DRI3 with X11, yet it's missing the check or any of the dependency tracking/management. Thus it fails at build or link time - depending on how lucky you get. This has been fixed as standalone patch(es) which should be ok for stable. Now combine that with the fact that we want a similar logic for the VL targets (omx, vaapi and maybe vdpau/xvmc/other one day) you'll end up with the following options a) provide a --with-foo-platforms=... Here we'll end up with [even greather] combinatoric explosion, hundreds of lines of build glue and nearly impossible to test/ensure that mesa isn't constantly broken. There is the part of "building EGL w/o wayland while Vulkan with wayland" is the style of thing that we do _not_ support [for example gallium drivers building/using graw/gallivm], nor something people [normally] go for. b) or, have a single --with-platforms= Giving us a uniform way to control the winsys/platforms. It is _very_ annoying about the rename, but it handles a lot more nuisances in a brief and easy to understand manner. I doubt many builders will be excited about reading and managing another 3-4 new configure toggles... A workaround which just make to mind: - keep --with-egl-platforms around and it's heuristics (which will also be applied for --with-platforms). - keep the --with-egl-platforms decisions for EGL only, and let one override it via --with-platforms. Warn as the latter happens. - warn for the deprecation of --with-egl-platforms This gives us: - old (consistent) behaviour for EGL - definition of the current undefined behaviour for Vulkan - definition for VL targets and any future ones. - everybody's scripts keep working... on the premise that they'll see the warning and update before things are removed (2...20 releases?) How does this sound ? Thanks Emil P.S. For the future please point out "what do you mean with 'some quoted text'" rather than saying "what's the problem". _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
