Add 'use-x-over-wayland' feature

[alexheretic]

As the wayland code is currently not working well enough under a gnome wayland session (Ie currently the window example renders nothing), I've added a feature to allow XWayland usage, ie simply use X when in a wayland session. Now in a gnome wayland session I can run the window example with cargo run --example window --features use-x-over-wayland

It is debatable whether this should be default until the wayland code is in a better state. In any case this feature will allow me to use this lib in wayland.

[alexheretic]

This can serve as a general work around for wayland-window issues ie #163 #109

[mitchmindtree]

Ie currently the window example renders nothing

#188 should have solved this particular issue. I'm also in the middle of trying to fix this issue with @vberger's help, along with a couple other smaller winit issues, so hopefully the situation will have improved a little over the next few days/weeks.

That said, I agree that this would be a useful feature, even if only for the fact that the wayland backend won't be able to provide native looking window decorations for the forseeable future. It would probably also make testing the x11 backend much easier for wayland users.

I'd be happy to see this merged, but would prefer to wait for @tomaka to give the sign-off as this adds a new feature.

[tomaka]

My opinion is that this is a very wrong usage of Cargo features, and that this thing should be decided at runtime.

[mitchmindtree]

Perhaps this option could be better provided via the os::unix module somehow?

[alexheretic]

It probably would be more natural to choose at runtime by the library consumer, however the current code has it baked in to prefer wayland over x via static initialisation. Indeed the current API design implies exactly one backend chosen at compile time configured in the Cargo.toml file (using the target os).

Here we have something new, 2 different backends on the same target os.

Since winit chooses it's backend at compile time, and the API relies on there being only one, I see the following choices:

• Redesign the API for all platforms to allow backend choice.
• Fix Wayland backend so it is on par with X.
• Allow unix backend preference at compile time

Obviously we want the second one, in the meantime with this PR I'm suggesting the last option. It has no effect on the API, will break nothing and is a small code change. It simply allows use of XWayland, and such usage is currently very desirable and will be useful as long as there are positive differences between X backend and Wayland.

[alexheretic]

I suppose we could use an environment variable instead. Would that be preferred?

[tomaka]

My opinion on this is that in an ideal world you would have Cargo features to enable/disable each backend individually, and backends would be enabled by default on their corresponding platform.

For example, the x11 Cargo feature would enable/disable the X11 backend. It would be enabled by default on Linux and disabled by default on Windows, but you could enable it and use it on Windows if you wanted to.

Then you could decide at runtime in which order the backends should be tried, or create a window with a specific backend.

When it comes to environment variables, I think it's a bad design when a library changes its behavior depending on an environment variable. I know the SDL does that for example, but in my opinion it is better when a library has a precise, controllable behavior from the point of view of the application that uses it.

[alexheretic]

Yes I agree that the top level app dev should be able to choose between x and wayland. If we did it at runtime the switch would need to be done before any call to winit functions.

So a functional configuration could look like this:

fn main() {
    winit::preconfig::use_x11_linux_backend().unwrap(); // or some equivalent fn call
    // panics if backend already initialised

    let events_loop = winit::EventsLoop::new(); // call statically initialises backend
    winit::WindowBuilder::new().build(&events_loop).unwrap();
}

Or we could use an env variable USE_X11_LINUX_BACKEND that affects the static initialisation.
I don't think I agree than env vars are bad. In a way they are more natural as you set them before running by default (unlike the function that will panic, or just not work if used after winit-init). They also mean the end user can decide "wayland isn't working for me on my distro for whatever reason" and try to use XWayland.

Are there other better ways to introduce runtime configuration?

[mitchmindtree]

If we were to go with a function call that sets some global like in your example, it would probably be better to use the existing os module (which handles OS-specific code), e.g. winit::os::unix::prefer_x11_over_wayland or something like this. However, I imagine it would probably be more suitable to add this as an alternative EventsLoop constructor (impld in the os::unix module), as a user shouldn't be able to call this after an EventsLoop has already been created.

Alternatively, winit could provide an enum which enumerated the available backends of the platform along with an EventsLoop builder that allowed specifying the order of preference, e.g. EventsLoop::with_backend_preferences(&[Backend::X11, Backend::Wayland]) or something along these lines.

[alexheretic]

All the winit functions initialise the backend though, not just EventLoop construction. Ie winit::get_available_monitors(). But I suppose EventsLoop::with_backend_preferences could just panic if the already initialised backend is invalid to that selection.

By tethering the choice to EventsLoop you make EventsLoop the definitive first call to winit. I'm not sure that's best with the other public functions being around. (Although I'm not fully aware of the general design directions of winit).

Maybe static init is bad. In general I prefer explicit instance initialisation without static happenings.

fn main() {
    let winit_context = winit::Context::builder()
        .prefer_x11_over_wayland()
        .build();
    // winit_context.get_available_monitors();
    // winit_context.get_primary_monitor();
    // winit_context.new_events_loop() // or maybe EventsLoop::new(&winit_context)

    winit::WindowBuilder::new().build(&winit_context).unwrap();
}

At the moment it seems like you're using EventsLoop (shouldn't it be 'EventLoop' anyway?), as both a handle on events for the user, and a context for the window. So maybe the separation would be a good thing.

You'd also be able to build two differently configured contexts in the same program.

But we're moving into the whole 'API redesign' thing I mentioned earlier. I'm sure you guys don't particularly care how I prefer to design APIs, but I do care about being able to use winit in a wayland session this year. Can I?

[mitchmindtree]

In generally I prefer explicit instance initialisation without static happenings.

I agree with this, I'd be happy to see removal of the global stuff. Though I think in your example the Window should probably still be built with the EventsLoop (it could get a handle to the context via the EventsLoop while registering with it for events).

shouldn't it be 'EventLoop' anyway?

I agree but this should be discussed in its own issue (or in a PR that fixes it).

I'm sure you guys don't particularly care how I prefer to design APIs, but I do care about being able to use winit in a wayland session this year.

I personally do care and would prefer to see winit's API improved. Whether or not we should add a temp hack for this in the meantime is probably up to tomaka.

[alexheretic]

Well in any case it looks like this PR is the wrong approach. So how about we:

• Work on removing the static init assumptions of the API and allow the flexibility to do stuff like select backend preference at runtime (a bit like above example, to be discussed in a new issue)
• Add an environment variable WINIT_PREFER_BACKEND=X11 (or equivalent) logic now, to be removed when this desired functionality can be done properly in the API

If we get non-static init to work, the PR would naturally remove the env variable logic around the statically initialised backend. @mitchmindtree @tomaka would you agree that this is an ok compromise?

Perhaps I can help with both. In any case thank you for taking the time to look at this, with your answers I think we can close this PR.

[alexheretic]

Closed in favour of #199