glow icon indicating copy to clipboard operation
glow copied to clipboard

Adding wrappers around `get_parameter_i32` functions

Open sanbox-irl opened this issue 4 years ago • 4 comments

With the change to NewType patterns in 0.11, the ability to backup OpenGl state in a way generic over webgl and native got more complicated (or rather, revealed that the way we'd been doing it prior wasn't actually correct at all). The problem is that the only way to ask for the current handle on opengl objects is with the get_parameter_x functions. I suspect that this could end up being quite a lot of work, but also will be a much, much nicer abstraction than OpenGl itself provides.

The most sensible solution seems to be to provide wrappers for get_parameter_i32. There are the other get_parameter_x functions, but I do not believe they need newtype wrappers.

I think therefore we would need the following:

get_parameter_texture_x -- all variants, which are 1D, 1D_ARRAY, 2D, 2D_ARRAY, 2D_MULTISAMPLE, 2D_MULTISAMPLE_ARRAY, 3D, BUFFER, CUBE_MAP, CUBE_MAP_ARRAY, RECTANGLE. We can wrap this all in an enum, which would require users to learn of some enum or const to use, or we could use i32 and verify that it is one of those, or write seperate calls for each one (i favor the last option, since the other functions proposed here will not take an argument)
get_parameter_current_program
get_parameter_sampler
get_vertex_array
get_buffer_x -- all variants which are ARRAY, DISPATCH_INDIRECT, ELEMENT_ARRAY, PIXEL_PACK, PIXEL_UNPACK, SHADER_STORAGE, TEXTURE_BUFFER, UNIFORM_BUFFER
(FENCE -- don't think there are any Fence getters)
get_framebuffer_x -- two varaints: DRAW and READ
get_renderbuffer
(QUERY -- don't think there are any query getters, but don't know much about them at all)
(UNFIFORM_LOCATION -- seems to have already been wrapped around, since that's a separate function anyway)
get_transform_feedback -- ths is actually a buffer call in raw opengl but looks like its a separate type here -- not sure if it should be part of the get_buffer_x variants or on its own.

Okay, I think that's all of the calls. This would be a lot of functions! Let me know what approach you think would be good, and I don't mind doing some of the legwork here

sanbox-irl avatar Sep 05 '21 01:09 sanbox-irl

getFramebufferAttachmentParameter would be useful too, for use with FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING (which is already a constant that's exposed).

If we wanted to handle things properly without explicit configuration in the imgui-glow-renderer, we'd need to check something like:

gl.is_enabled(glow::FRAMEBUFFER_SRGB) &&
	glow::SRGB == gl.get_framebuffer_attachment_parameter(
	    glow::FRAMEBUFFER,
	    glow::COLOR_ATTACHMENT0,
	    glow::FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING)

thomcc avatar Oct 07 '21 04:10 thomcc

Current workaround that (experimentally) seems to work (in my case)

fn read_current_framebuffer(gl: &glow::Context) -> glow::NativeFramebuffer {
    unsafe {
        let as_u32 = gl.get_parameter_i32(glow::FRAMEBUFFER_BINDING) as u32;
        std::mem::transmute::<u32, glow::NativeFramebuffer>(as_u32)
    }
}

As far as I can tell, a perfectly adequate solution would be to expose the constructors for the various native types. Maybe this isn't as typesafe, but when working with getParameter, you're always going to have to think about the typing, or the library you're using will have to be high enough level to abstract all of this away (and as far as I can tell, glow isn't trying to fill the role of high level abstractions)

sdfgeoff avatar Nov 06 '21 06:11 sdfgeoff

@sdfgeoff we're hoping to find a good way that works for both native and web, but exposing the constructors won't work on the web backend. I started to experiment with an approach for get_parameter in https://github.com/grovesNL/glow/pull/190 that might be ok

grovesNL avatar Nov 07 '21 00:11 grovesNL

if this is still open .. I was considering changing to this crate and wondered how I'd be able to get the default framebuffer. In my usecases this is all I'd need - other things I bind, I already have .(and for the web you need to eliminate 'gets' anyway). GL_BACK etc are used to bind this (i think glBindFramebuffer( .. ,0) will get it aswell ?)

Perhaps a dedicated call could be added to cover this? context.bind_framebuffer(context.default_framebuffer()) or context.bind_default_framebuffer() then you dont have to expose translating between the gets and user side newtype wrapped objects

dobkeratops avatar Apr 20 '23 22:04 dobkeratops