Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make types generic over a Backend trait #908

Closed
nvzqz opened this issue Jun 10, 2019 · 2 comments
Closed

Make types generic over a Backend trait #908

nvzqz opened this issue Jun 10, 2019 · 2 comments
Labels
C - needs discussion Direction must be ironed out

Comments

@nvzqz
Copy link
Contributor

nvzqz commented Jun 10, 2019

Because some platforms are capable of using multiple different backends, it may be worthwhile to create a Backend trait that's implemented on concrete types for each backend. The types would only be exposed on the platforms that support them.

For example, Unix (excluding macOS) supports X11 and Wayland.

The following are the proposed types that would go in a backend module:

  • Unix: X11, Wayland
  • Windows: Windows (or Win32?)
  • macOS: AppKit
  • iOS: UiKit
  • Android: Android

Other platforms will inevitably have more than one backend in the future. For example:

  • UIKit is coming to macOS, and so a UiKit backend type can then be used on iOS or macOS.
  • If we added Flutter support, it would be used on Fuchsia, iOS, and Android. On top of that, it is going to be available for macOS, Linux, and Windows.

Implementation

The following presents a basic idea of how this would be implemented:

mod backend {
    pub trait Backend {
        type NewEventLoopError;
        type EventLoopOptions;

        type NewWindowError;
        type WindowOptions;

        fn new() -> Self;

        fn event_loop<T>(self) -> Result<EventLoop<Self>, Self::NewEventLoopError>;

        fn event_loop_with<T>(self, options: Self::EventLoopOptions) -> Result<EventLoop<Self>, Self::NewEventLoopError>;

        fn create_window<T>(event_loop: &EventLoop<Self, T>) -> Result<Window<Self>, NewWindowError>;

        fn create_window_with<T>(
            event_loop: &EventLoop<Self, T>,
            options: Self::WindowOptions,
        ) -> Result<Window<Self>, NewWindowError>;

        // ...
    }

    #[cfg(any(
        target_os = "linux",
        target_os = "dragonfly",
        target_os = "freebsd",
        target_os = "netbsd",
        target_os = "openbsd"
    ))]
    pub struct Wayland {
        // ...
    }

    #[cfg(...)]
    impl Backend for Wayland {
        // ...
    }

    #[cfg(target_os = "macos")]
    pub struct AppKit {
        // ...
    }

    #[cfg(...)]
    impl Backend for AppKit {
        // ...
    }

    pub enum AnyBackend {
        #[cfg(...)]
        Wayland(Wayland),

        #[cfg(...)]
        AppKit(AppKit),

        // ...
    }

    impl Backend for AnyBackend {
        // ...
    }
}

pub struct EventLoop<B: Backend, T> {
    // ...
}

impl<B: Backend, T> EventLoop<B, T> {
    fn new_user_event() -> Result<Self, B::NewEventLoopError> {
        B::new().event_loop()
    }

    fn new_user_event_with(options: B::EventLoopOptions) -> Result<Self, B::NewEventLoopError> {
        B::new().event_loop_with(options)
    }

    // ...
}

pub struct Window<B: Backend> {
    // ...
}

impl<B: Backend> Window<B> {
    fn new<T>(event_loop: &EventLoop<T>) -> Result<Self, B::NewWindowError> {
        B::create_window(event_loop)
    }

    fn new_with<T>(
        event_loop: &EventLoop<Self, T>,
        options: Self::WindowOptions,
    ) -> Result<Window<Self>, NewWindowError> {
        B::create_window_with(event_loop, options)
    }
}

pub struct WindowBuilder<B: Backend>(B::WindowOptions);

impl<B: Backend> WindowBuilder<B> {
    // ...

    fn build<T>(self, event_loop: &EventLoop<T>) -> Result<Window<B>, B::NewWindowError> {
        B::create_window_with(event_loop, self.0)
    }
}

AnyBackend

The following type would be available for all supported platforms:

pub enum AnyBackend {
    #[cfg(...)]
    Wayland(Wayland),

    #[cfg(...)]
    AppKit(AppKit),

    // ...
}

This is somewhat similar to how platform_impl::linux::EventLoop is implemented.

There are 3 cases:

  • 0 backends available for the current platform. This should be a compile error like how it is now.
  • 1 backend available. This would have no more runtime cost than using the internal backend directly.
  • n backends available. The Backend::new implementation would pick the preferred option available on the current system.

Pros

  • Type safety. Functionality specific to Wayland vs X11 doesn't get mixed into the same type.

  • Enforces error handling. std::convert::Infallible can be used as the error type when operations will always succeed. This would eliminate any branches associated with Result::Err.

  • May allow for getting rid of platform_impl.

Cons

  • You'd need to create a window via:

    use winit::backend::Wayland;
    use winit::{EventLoop, Window};
    
    let el = EventLoop::<Wayland>::new();
    let win = Window::new(&el);

    The user may not want to be concerned about about the actual backend being used.

  • Using winit becomes slightly more verbose.

  • There's a lot of boilerplate when implementing Backend functionality on the concrete types. However, this can perhaps be mitigated with macros although I'm not certain.

@Osspial
Copy link
Contributor

Osspial commented Jun 10, 2019

This introduces a significant amount of complexity to the cross-platform API for benefits that are really only felt on one platform. Even then, I'm not convinced this provides much in the way of benefits on the platforms it affects - you'd need to do some serious type system contortions to use the statically-checked backend while maintaining cross-platform and cross-backend compatibility (I can elaborate on what those contortions would be if you'd like, but I don't feel that's necessary right now).

If you want to separate types for the X11 and Wayland backends, you can do that without touching the public cross-platform API:

pub trait WindowExtUnix {
    fn get_window_ext(&self) -> WindowExtUnixEnum<'_>;
}

pub enum WindowExtUnixEnum<'a> {
    Wayland(WindowExtWayland<'a>),
    X11(WindowExtX11<'a>),
}

pub struct WindowExtWayland<'_> {
    /* functions */
}

pub struct WindowExtX11<'_> {
    /* functions */
}

Why is a generics-based solution better than that?


Enforces error handling. std::convert::Infallible can be used as the error type when operations will always succeed. This would eliminate any branches associated with Result::Err.

Why is this a benefit? You could do that, but:

  1. If downstream users rely on an error never happening, that kills cross-platform compatibility with platforms that can error.
  2. Consistently untaken branches have pretty much no cost on modern systems because of branch prediction. If the error handling interaction with your windowing library is a noticeable performance bottleneck in your code, there are far more serious problems that need to be addressed.

May allow for getting rid of platform_impl.

platform_impl is an implementation detail that nobody outside of Winit's codebase sees. Why does this matter?

@Osspial Osspial added the C - needs discussion Direction must be ironed out label Jun 13, 2019
@madsmtm
Copy link
Member

madsmtm commented Jan 23, 2023

I've opened #2430 for a proposal that solves some of the same problems.

For this one in particular, I think @Osspial hits it right on the nail; there really isn't that big of a benefit to doing it, so I'll go ahead and close this issue. Thanks for the suggestion (though it was a while back ;) )

@madsmtm madsmtm closed this as completed Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out
Development

No branches or pull requests

3 participants