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

priotirize osmesa over glutin conext builder #379

Merged
merged 9 commits into from
Jul 25, 2023
Merged

Conversation

adhami3310
Copy link
Contributor

@adhami3310 adhami3310 commented Jul 14, 2023

From manual testing on a machine without a display backend, attempting to create EventLoop will cause a panic, thus preventing it from attempting to create an osmesa instance which could work without a display backend. Simply reordering the lines (and removing unnecessary passing of the event loop back to the caller) fixes the issue and it does attempt osmesa first.

@adhami3310
Copy link
Contributor Author

there's some unneeded space in headless but there seems to be other formatting issues?

@asny
Copy link
Owner

asny commented Jul 16, 2023

First of all, thanks for the suggestion, it sure makes sense. Did you test that it works when not using osmesa? I've had a lot of issues earlier, so I'm not too fond of changing this 😬

About the formatting issues, I think you just need to run cargo fmt 🙂

@adhami3310
Copy link
Contributor Author

Did you test that it works when not using osmesa?

So I cannot confirm it works when not having osmesa installed as my system and docker images have osmesa enabled. If you don't have osmesa installed you can test this by running the headless example with cargo run --example headless --features="headless"

What I can confirm, is that it works perfectly fine when having osmesa enabled. I can find someone with no osmesa to test this.

@asny
Copy link
Owner

asny commented Jul 25, 2023

Sorry for the spam, tried to copy some old code, but it wasn't working anymore. I added an action that tests that headless is working on linux. I'm not on linux so I can't test it locally. It seems to work in the action, so I guess it's ok.

@asny asny merged commit 4b7600c into asny:master Jul 25, 2023
5 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

2 participants