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

api: move get_proc_address to GlDisplay #1460

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

kchibisov
Copy link
Member

This should made it clear that you should call this only once and not on each context creation.

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality

@MarijnS95
Copy link
Member

Thanks! That was a blocker for #1417 to function in a clean way :)

@MarijnS95 MarijnS95 mentioned this pull request Sep 8, 2022
@kchibisov
Copy link
Member Author

Uhm, there's still a problem with WGL, I think we have nothing better than document that...

This should made it clear that you should call this only once
and not on each context creation.
@kchibisov
Copy link
Member Author

kchibisov commented Sep 8, 2022

I've added a comment wrt that, since having function on the context is missleading in the first place, because you'll try to load it each time you create the context.

@MarijnS95
Copy link
Member

Yeah you mentioned in #1417 but it's probably worth mentioning it explicitly in the PR here too to investigate.

Can you add an optional current context argument to get_proc_addr() and assert it's not-None on WGL?

glutin/src/display.rs Outdated Show resolved Hide resolved
Co-authored-by: Marijn Suijten <marijns95@gmail.com>
@kchibisov
Copy link
Member Author

Can you add an optional current context argument to get_proc_addr() and assert it's not-None on WGL?

The thing is that it'll still load, it will be just a limited set of functions. You won't have shaders, but you'll have glClear for example. It's not like it won't work at all, that's the point. I don't want to have a custom argument here, since you can't crash due to that, you'll just load not everything and that's sort of clear that way anyway.

I'll see whether it'll raise issues and adjust if folks will struggle with that.

@MarijnS95
Copy link
Member

The thing is that it'll still load, it will be just a limited set of functions.

I did not know that.

Then indeed, I agree that we shouldn't crash, documenting this properly is all we can do. Let's see if any issues arise.

@kchibisov kchibisov merged commit 5c121bc into rust-windowing:master Sep 8, 2022
@kchibisov kchibisov deleted the display-proc-address branch September 8, 2022 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants