Can we remove most of the conditional compilation stuff?

[Lokathor]

The conditional compilation stuff is an unnecessary complication to the system.

Basically, every library that consumes a HasRawWindowHandle value already needs to have a default, catch-all case where they get some unknown handle type and then error out. We even want the enum to be tagged as non-exhaustive so that the library can be "future proof" and force people to have a default cause so that we don't break semver when new window systems are eventually added.

So since your consuming code's match statement already needs to say

  unknown => return Err(format!("unknown window handle type: {:?}", unknown)),

at the end of it, then having the IOS handle conditionally compiled to not exist on windows doesn't actually add anything. Either way you can't have exhaustive matching.

And since we're just giving all of our handle definitions in terms of libc types and core::ffi::c_void and so forth there's not really a problem with pulling in all sorts of platform dependencies if we just have every variant compiled in on all platforms.

So I think that never doing any conditional compilation is probably better in the long term. Makes things simpler to reason about.

[hecrj]

Have we considered removing the non-exhaustive RawWindowHandle enum and HasRawWindowHandle trait altogether and adding platform-specific traits instead? These traits could also potentially be used to remove the current non-exhaustive structs too.

What is the purpose of giving up guarantees by wrapping the handle in an non-exhaustive enum when our users have to write platform-specific code anyway?

[Lokathor]

No the point is that an end user of this communication point sees it as just one thing

let surface = instance.make_surface_for_window(&window).unwrap();

Since some systems potentially have more than one window system available, for this to move all the complexity away from the end user there cannot be window specific methods.

[hecrj]

@Lokathor The same way we currently force our users to pattern match using conditional compilation, we can force them to declare a function using conditional compilation too (they probably have to do this anyway):

#[cfg(target_os = "ios")]
fn make_surface_for_window<W: HasIOSHandle>(&self, window: &W) -> Result<Surface, Error> {
    // window.ui_window()
    // window.ui_view()
    // ...
}

The main difference is that there won't be any need for non-exhaustive enums or fields, and there won't be misleading APIs taking an unsupported window handle and erroring at runtime.

[Lokathor]

No, graphics libs aren't the end users. Graphics libs and window libs can both have a hard time and I don't much care because you sort it out a few times and then it's done.

I mean actual end users that want to make a window and then tell a backend to work on that window. That is the situation that will be written out over and over in countless new projects and demos, and that's the case that we want to optimize simplicity for.

[hecrj]

@Lokathor I don't think my proposal would affect end users.

In fact, I think it would help them. With my approach, if our end users try to compile a graphics library on an unsupported platform, they will get a compilation error instead of a runtime error.

[Osspial]

@Lokathor Reading through your argument, I don't see how removing the conditional compilation stuff makes things any easier for downstream users. I'd argue that it actually makes it harder to understand - making IOSHandle available on Windows implies that it's possible to get a handle to an iOS window when running in Windows, which is clearly nonsense. Conditionally compiling the variant out makes sure you hit the error messages as early as possible when writing your platform matching code, and prevents HasRawWindowHandle implementors from returning garbage values.

@hecrj We actually had a platform-specific trait design originally, although it admittedly wasn't exactly how you're proposing. That was removed because the people involved in the discussion didn't like the design (#1 #4 @kvark @Ralith), so we switched to the enum system. Looks like you can't please everyone 😅.

That being said, switching to a design where you had a different top-level trait for each platform would complicate this crate's usage substantially, since you'd either need to have different functions for each platform that takes each different trait, or conditionally compile the different platform trait into a single function. I don't like either of those solutions. Believe me, I've done a lot of work with compile-time trait matching, and I don't think the complexity of that solution is worth the benefits here.

[hecrj]

@Osspial What about exposing HasRawWindowHandle as the end-user-facing trait, conditionally extending a platform-specific trait?

#[cfg(target_os = "linux")]
pub trait HasRawWindowHandle: crate::unix::HasUnixHandle {}

#[cfg(target_os = "windows")]
pub trait HasRawWindowHandle: crate::windows::HasWindowsHandle {}

Graphics libraries could then write a single function:

impl Instance {
    pub fn create_surface<W: HasRawWindowHandle>(&mut self, window: &W) -> Result<Surface, Error> {
        #[cfg(target_os = "linux")]
        let surface = self.create_surface_for_unix(window)?;

        #[cfg(target_os = "windows")]
        let surface = self.create_surface_for_windows(window)?;

        // ...

        Ok(surface)
    }
}

Would this work? This is very similar to what wgpu-native is already doing, but you won't be forced to fail at runtime on an unsupported platform. Additionally, traits are more flexible than enums and structs when trying to avoid breaking changes.

[Osspial]

@hecrj ...huh! I can't see why that wouldn't work. Could you throw together a prototype implementing that? One small addition to that design is that HasRawWindowHandle should get auto-implemented if the type implements the platform trait, but other than that I think you've got a nice solution here.

It looks like we'll have to do a breaking change to differentiate between XLib and XCB anyway, so this design works I'm tempted to roll both changes into a single release.

[Osspial]

Closing this issue, since there doesn't seem to be a compelling reason to do this and the discussion in this issue has moved to a different topic. We can continue discussion of the trait prototype in #23.