godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Expose OpenGL API through wrapper object #8380

Open BastiaanOlij opened 12 months ago

BastiaanOlij commented 12 months ago

Describe the project you are working on

Working on improving the compatibility renderer.

Describe the problem or limitation you are having in your project

When we wrote the compatibility renderer this was mostly porting code from Godot 3 but wrapping it in a Godot 4 jacket. One thing we did not look into is giving access to OpenGL features from GDScript, plugins, etc, limiting our ability to extend the compatibility renderer. Another was the inability to track objects and dependencies, which is something we partially solved in the GLES3::Utilities class. Finally there is the issue of dealing with extensions and with small syntax differences between OpenGL and OpenGLES, especially where we rely on GLAD on desktop, but limit ourselves to core GLES on Android and Web platforms and handle extensions ourselves. This is partially handled by our GLES3::Config class.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The change I want to make is introduce a GLES3::OpenGL helper/wrapper class through which we access OpenGL. This object will be responsible for tracking state, tracking allocation, etc. for all OpenGL calls. This class will be a singleton that can be requested from GDScript/GDExtensions/etc if the compatibility renderer is used. The functionality in GLES3::Config will move into this new class, and the memory tracking added to GLES3::Utilities should move into this class.

We should add additional logic on every call so we can check status and on a debug build output issues (as they often remain hidden). For instance:

void OpenGL::glRenderbufferStorageMultisample(GLenum target, GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height) {
    ERR_FAIL_NULL(glRenderbufferStorageMultisamplePtr);
    glRenderbufferStorageMultisamplePtr(target, samples, internalformat, width, height);
    opengl_error = glGetError();
    if (opengl_error== GL_NO_ERROR) {
        GLuint buffer_id = render_buffer_targets[target];
        _update_render_buffer_size(buffer_id, samples * width * height * _get_size_for_format(internalformat));
#ifdef DEBUG_ENABLED
    } else {
        ERR_FAIL_MSG("Couldn't allocate multisample storage: " + _get_error_msg(opengl_error)
#endif
    }
}

This is a potential implementation for an API call wrapper, this checks if we have a valid function pointer, then calls the actual OpenGL function, and as we need to track memory here checks for success and updates our total memory. On a debug build, on failure we have extra code that outputs the error.

Note also that we assume here that we've made our own wrapper for glGenRenderbuffers which keeps track of all render buffers and their size, and glBindRenderbuffer which records which buffer is currently bound, so we can do our memory tracking.

Most functions will likely be 1:1 pass throughs, but we do need to make a choice if we stick to OpenGL IDs or, seeing that we're already tracking objects, whether we'll use RIDs here and whether we want to combine some functions that are likely to be used in combination like convenient "create framebuffer" or "create texture" functions. Obviously we can start with ensuring all OpenGL functions are exposed as is, and we add such convenience methods as an addition.

Finally as we're already handling our own extension loading for OpenGLES, there is also a choice whether we want to keep relying on GLAD or whether we'll embed our own extension loading for OpenGL as well. That may be overkill however

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

See above.

If this enhancement will not be used often, can it be worked around with a few lines of script?

This is core rendering functionality.

Is there a reason why this should be core and not an add-on in the asset library?

This is core rendering functionality.

bruvzg commented 12 months ago

Finally there is the issue of dealing with extensions and with small syntax differences between OpenGL and OpenGLES, especially where we rely on GLAD on desktop, but limit ourselves to core GLES on Android and Web platforms and handle extensions ourselves.

RasterizerGLES3::is_gles_over_gl can be used to check if it's GL or GLES (both are supported on macOS, Linux and Windows after ANGLE addition, so it can't be determined only based on platform).

The change I want to make is introduce a GLES3::OpenGL helper/wrapper class through which we access OpenGL.

How often something like this will be used? For the GDExtensions it's probably enough to expose glGetProcAddress used by the engine, the rest can be acquired using it and used without any unnecessary overhead from the wrappers. And for GDScript, implementing a generic FFI and using the same might be a better idea as well.

BastiaanOlij commented 12 months ago

Hey @bruvzg,

RasterizerGLES3::is_gles_over_gl can be used to check if it's GL or GLES (both are supported on macOS, Linux and Windows after ANGLE addition, so it can't be determined only based on platform).

That's a good one because I have that limitation currently also with https://github.com/godotengine/godot/pull/83976, where I'm pulling in GLES only extensions.

How often something like this will be used? For the GDExtensions it's probably enough to expose glGetProcAddress used by the engine, the rest can be acquired using it and used without any unnecessary overhead from the wrappers. And for GDScript, implementing a generic FFI and using the same might be a better idea as well.

Very hard to say, it's come up in a few discussions in the last couple of months, especially around the functionality I was adding to the RD renderer around rendering/compositor effects and the wish to be able to do the same thing in compatibility. That will require exposing parts of the compositor implementation through classDB.

In the past I've indeed just used OpenGL directly through GDExtensions and prayed, it mostly works (only some issues when threading is involved).

The biggest problem though is the tracking we want to introduce especially when extensions are directly calling the OpenGL API and bypassing our tracking. There is so much global state that can change on you, things break easily

BastiaanOlij commented 10 months ago

Had further discussions about this with @clayjohn on the contributors chat and during the rendering meeting. This came up again because we were trying to fix a number of bugs in the OpenGL renderer caused by mismatched state changes. We also again discussed the requirement at some point to expose the API to GDScript/GDExtension so we can implement compositor effects in OpenGL as well.

Even if we keep most of the core code calling OpenGL directly, we could expose most through an OpenGL class to GDScript, and embed more complex logic into this.

So worth further discussion.