Skip to content

api: move get_proc_address to GlDisplay#1460

Merged
kchibisov merged 2 commits intorust-windowing:masterfrom
kchibisov:display-proc-address
Sep 8, 2022
Merged

api: move get_proc_address to GlDisplay#1460
kchibisov merged 2 commits intorust-windowing:masterfrom
kchibisov:display-proc-address

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 kchibisov force-pushed the display-proc-address branch from 8b3688e to ca96b59 Compare September 8, 2022 12:37
@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?

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