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

Use glow instead of gl_generator for GL/GLES bindings #321

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sagudev
Copy link
Member

@sagudev sagudev commented Oct 17, 2024

See servo/servo#33539.

Note that this PR will expose glow types in surfmans public API.

@mrobinson mrobinson changed the title use glow instead of gl_generator for GL/GLES bindings Use glow instead of gl_generator for GL/GLES bindings Nov 20, 2024
@mrobinson mrobinson changed the title Use glow instead of gl_generator for GL/GLES bindings Use glow instead of gl_generator for GL/GLES bindings Nov 20, 2024
@sagudev sagudev force-pushed the no_gl_gen branch 3 times, most recently from dce071e to 3ae3cf7 Compare November 20, 2024 17:14
@sagudev
Copy link
Member Author

sagudev commented Nov 29, 2024

So the problem is that:

thread_local! {
    pub static GL_FUNCTIONS: Gl = unsafe {Gl::from_loader_function(context::get_proc_address)};
}

does not work because glow requires for context to be current, because it does some querying (version, extension). My idea is to simply replace all usages of GL_FUNCTIONS with let gl = unsafe {Gl::from_loader_function(context::get_proc_address)}; as most of the time we are already current in the calls. This would make querying every time, but functions are not hot AFAIK, so this should be ok. Alternative would be to have Gl stored on context, but this needs to be lazy value, as context creation does not make context current.

@sagudev
Copy link
Member Author

sagudev commented Jan 4, 2025

I am wondering do we really need newly-created contexts that not immediately made current, or can we just made them current on creation? Linked issue on test is about something different. THis was introduced in dae72e4. cc @pcwalton @jdm

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.

1 participant