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

SimpleWindowBuilder.build should be generic over EventLoop<T> #2093

Merged
merged 1 commit into from Jan 1, 2024

Conversation

waynr
Copy link
Contributor

@waynr waynr commented Jan 1, 2024

This allows me to add my own events to the event loop via
winit::event::Event, prior to this change I was seeing errors that looked
like the following:

error[E0308]: mismatched types
   --> src/main.rs:62:16
    |
62  |         .build(&el);
    |          ----- ^^^ expected `&EventLoop<()>`, found `&EventLoop<MyEvent>`
    |          |
    |          arguments to this method are incorrect
    |
    = note: expected reference `&EventLoop<()>`
               found reference `&EventLoop<MyEvent>`

I haven't tried running any tests, just thought I would get feedback from
maintainers before doing any more.

@est31
Copy link
Collaborator

est31 commented Jan 1, 2024

LGTM, I think it's good to have this change.

@notgull
Copy link
Contributor

notgull commented Jan 1, 2024

I think a better solution would be to work a no-generic clonable owned display handle into winit, as proposed here.

@waynr
Copy link
Contributor Author

waynr commented Jan 1, 2024

I think a better solution would be to work a no-generic clonable owned display handle into winit, as proposed rust-windowing/glutin#1582 (comment).

@notgull I'm still fairly new to graphics programming in general let alone glium/glutin/winit...but I don't understand how that solves the problem. Does obviate the need for SimpleWindowBuilder somehow such that I can use winit::window::WindowBuilder with its already generic build method?

Not disputing that your proposed solution is better, but is there anything particularly wrong with this approach that should prevent it being merged before the better-in-the-long-term solution is available? Is it possible someone is actually depending on SimpleWindowBuilder taking a non-generic EventLoop<()>?

@notgull
Copy link
Contributor

notgull commented Jan 1, 2024

Ah my bad, misunderstood the problem. Yeah that wouldn't work

@est31 est31 merged commit ce5c3a0 into glium:master Jan 1, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants