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

Support RedrawRequested event #3

Closed
parasyte opened this issue Oct 22, 2019 · 9 comments
Closed

Support RedrawRequested event #3

parasyte opened this issue Oct 22, 2019 · 9 comments

Comments

@parasyte
Copy link
Contributor

Looks like this event is not currently supported: https://docs.rs/winit/0.20.0-alpha4/winit/event/enum.WindowEvent.html#variant.RedrawRequested

The crate supports everything else I need, just this one piece is out of place. See it in context, here:

https://github.com/parasyte/pixels/blob/d33c30dd08d2561cd1643001bea912a77b198c53/examples/invaders/main.rs#L51-L60

@rukai
Copy link
Owner

rukai commented Oct 22, 2019

e08cac1 I implemented it here.
Does this work for you?
Let me know if you want a crates.io release.

@parasyte
Copy link
Contributor Author

I’m sure that will function correctly.

Seems like I will have to use an if-else to either draw or update my game state. I also don’t know what kind of timing implications this will have, deferring RedrawRequested until after EventsCleared.

According to rust-windowing/winit#1041 (comment) RedrawRequested should be handled immediately.

@rukai
Copy link
Owner

rukai commented Oct 22, 2019

Eventscleared is not coming from your OS but from winit itself, just meaning that it has no more events queued up at the moment. So it should be fine to just wait for all queue d events to process before acting on the redrawrequested.

Games will want to redraw every frame anyway so games don't usually bother with redraw requested.

@parasyte
Copy link
Contributor Author

I was going to get back to this sooner, but while I was testing on Windows, my GPU decided to completely die.

Anyway, The winit issue I linked has some very important details about exactly why they have decided to implement RedrawRequested as a separate event. From what I can tell, the need arises from the way Windows handles resizes. Specifically, Ossipial provided examples like these demonstrating how poor timing will cause ugly artifacts, e.g. while resizing.

I wasn't able to verify any of this when I tested, before my GPU died. All I was able to get out of it was my game "freezing" while the window is resized. And this is with using the recommended strategy. I can't tell if these are just bugs or if it doesn't really matter.

@rukai
Copy link
Owner

rukai commented Oct 23, 2019

Well, if you want to process the event immediately why do you want winit_input_helper to be involved? It sounds like your current code is fine.

@parasyte
Copy link
Contributor Author

Just trying to reduce line noise in the source, TBH. You're right, the current code is fine functionally.

@rukai
Copy link
Owner

rukai commented Oct 23, 2019

Maybe it would be neater if it wasn't on so many lines ;)

if let Event::WindowEvent { event: WindowEvent::RedrawRequested, .. } = event {
    pixels.render(invaders.draw());
}

Unless you have any objections I might revert the changes I did for this issue because, as you have pointed out, its incorrect.

@parasyte
Copy link
Contributor Author

Yeah, rustfmt is picky. 🤷‍♀

Feel free to revert the change. I was hoping this was something simple. But might end up being a headache that isn't worth it. Returning an enum instead of a bool or whatever. Sounds like Pandora's box.

@rukai
Copy link
Owner

rukai commented Oct 24, 2019

Changes are reverted

@rukai rukai closed this as completed Oct 24, 2019
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

No branches or pull requests

2 participants