use cfg aliases throught the code base

[amrbashir]

• Tested on all platforms changed
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

[maroider]

I'd prefer if we kept things as close to 1:1 as possible, so I'd prefer if x11 and wayland were only aliases for their respective features, without also implying linux. Some cfg guards were also re-ordered, and I'd prefer if we kept the existing order (this isn't a huge deal, tbh, just a preference).

On the whole, though, I think this is a good change, and it makes our cfg guards a lot less noisy.

[amrbashir]

Some cfg guards were also re-ordered, and I'd prefer if we kept the existing order (this isn't a huge deal, tbh, just a preference)

Reverted!

[maroider]

So, I checked out your changes locally and grepped through for cfg guards, and found that you haven't converted some cfg guards in the following files:

• src/platform_impl/android/mod.rs
• src/platform_impl/ios/mod.rs
• src/platform_impl/linux/wayland/window/shim.rs

One issue that did occur to me just now is that os_linux makes it seem like it is only set when targeting Linux, which isn't true. I wouldn't consider it a hard blocker, but we might want to come up with a name that better conveys what os_linux actually means. wasm is also already a built-in cfg option, so we may want to use a cfg alias with a different name.

[amrbashir]

So, I checked out your changes locally and grepped through for cfg guards, and found that you haven't converted some cfg guards in the following files:

• src/platform_impl/android/mod.rs
• src/platform_impl/ios/mod.rs
• src/platform_impl/linux/wayland/window/shim.rs

Thanks, will update them too.

One issue that did occur to me just now is that os_linux makes it seem like it is only set when targeting Linux, which isn't true. I wouldn't consider it a hard blocker, but we might want to come up with a name that better conveys what os_linux actually means.

I wanted to name it os_unix but that would convey support for macOS, ios and android also.

wasm is also already a built-in cfg option, so we may want to use a cfg alias with a different name.

Can't I just remove the wasm custom alias and use the built-in cfg alias?

[amrbashir]

Can't I just remove the wasm custom alias and use the built-in cfg alias?

Looks like I can't, so I went ahead and changed it arch_wasm

[maroider]

I wanted to name it os_unix but that would convey support for macOS, ios and android also.

My ideas here are not great (mostly just os_linuxy).

Can't I just remove the wasm custom alias and use the built-in cfg alias?

That's probably fine. As far as I'm aware, there's only really wasm32 targets for rustc. If I were to hazard a guess, wasm64 support will likely arrive some day, but I don't think we directly rely on anything that would prevent Winit from "just working" on wasm64 (plus or minus some changes to Cargo.toml and support in web-sys and friends).

[maroider]

Looks like I can't, so I went ahead and changed it arch_wasm

Uuh, ok. Why didn't it work?

[amrbashir]

I don't know what the built-in cfg(wasm) expands to and couldn't find it out from searching through google results but seems like it doesn't satisfy the check here on Windows
https://github.com/rust-windowing/winit/blob/89eea64a4aa952ddb6a4612f61075f8ad622e1a9/src/platform_impl/mod.rs

see this run https://github.com/rust-windowing/winit/actions/runs/3664441578/jobs/6194851823

[amrbashir]

My ideas here are not great (mostly just os_linuxy).

Sounds acceptable to me for now until a better name is found.

[maroider]

I can't think of any further issues beyond what's already been mentioned.

[madsmtm]

Apart from os_linuxy I don't think we really gain much from this (other than an increase in compile-times, and an extra indirection where people have to go see what os_ios actually means ;) ).

But I'm not that against it, and if you think it would make it easier to integrate GTK, then sure, go for it (though yeah, I still think #2430 is the most "correct" way to go, but eh, that may also just be sunk costs speaking).

[amrbashir]

Apart from os_linuxy I don't think we really gain much from this (other than an increase in compile-times, and an extra indirection where people have to go see what os_ios actually means ;) ).

But I'm not that against it, and if you think it would make it easier to integrate GTK, then sure, go for it (though yeah, I still think #2430 is the most "correct" way to go, but eh, that may also just be sunk costs speaking).

TBH, I feel neutral about it, I think we can integrate GTK without it, but it was requested by @kchibisov

[maroider]

I don't think we really gain much from this (other than an increase in compile-times

Oof, I forgot about compile-times. I don't think this will add more than a couple of seconds, but any regression is unfortunate.

TBH, I feel neutral about it, I think we can integrate GTK without it, but it was requested by @kchibisov

Can you link that request, just for posterity?

[amrbashir]

https://matrix.to/#/!DGpLzJTRzBDTwZiogk:matrix.org/$9t77LEXJ2t3jIs2FAoa0oGUZZQfq1l1un2i6QhJi67w?via=libera.chat&via=matrix.org&via=matrix.fedibird.com

[kchibisov]

The idea is to structure around the displays, like, so in the end we'll have

x11_platform
wayland_platform
macos_appkit
windows
wasm
android
ios

The OS should be hidden where it makes sense, like for x11/wayland, and if something will try to use Wayland on e.g. macOS, porting will be done by tweaking build.rs to achieve mutually exclusivity and slight updating the inner backend.

The backends are expected to move to a static dispatch marcos based on the displays and not the platforms.

The GTK part could be entirely encapsulated from that, but more about it will be once we come to a point where our backends layout will be adjusted.

For an example about aliases you can take a look at https://github.com/rust-windowing/glutin/blob/master/glutin/build.rs .

[kchibisov]

Then, @amrbashir, since you've asked what's needs to be done to bring the GTK in the tree. The following layout from that PR should be implemented. It looks kind of ok and @madsmtm could likely help you with that, but for a working concept of such a thing you can look again at glutin.

#2430

https://github.com/rust-windowing/glutin/blob/master/glutin/src/lib.rs#L42 - that's how dispatch is done in glutin, but @madsmtm has slightly better version of this macro in their work.

So the cfg you're adding right now will be hidden inside the dispatch macros and not even seen in the code other than in module inclusion!.

[madsmtm]

Oh... I had understood previously that you were against #2430, @kchibisov, which is why I haven't worked on it more?

(Though these two PRs are not exclusive, so still 👍 from me on this one)

[kchibisov]

x11 or windows.

[amrbashir]

clippy will complain about unused functions on Wayland, changed it to x11, wayland or Windows

[kchibisov]

Thanks!