Skip to content

vo/opengl/formats: use R8 and RG8 formats on GL2.1 and ES2 if available #18264

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.

Sign up for GitHub

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

Open
anarsoul wants to merge 2 commits into
base: master
Choose a base branch
from
+46 −1
Open

vo/opengl/formats: use R8 and RG8 formats on GL2.1 and ES2 if available #18264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7e6c10
vo_gpu: opengl: support GL_EXT_framebuffer_object
anarsoul Jul 13, 2026
5e6c477
vo_gpu: opengl: use R8/RG8 formats on GLES 2.0 and GL 2.1
anarsoul Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions video/out/opengl/common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,26 @@ static const struct gl_functions gl_functions[] = {
.extension = "GL_EXT_unpack_subimage",
.provides = MPGL_CAP_ROW_LENGTH,
},
// Framebuffers, alternative extension name. Some GL 2.1 drivers (e.g.
// Mesa lima) expose only this one. It provides a compatible subset of
// GL_ARB_framebuffer_object for our purposes; the entrypoints loaded
// here are overwritten by the ARB/core ones below if available.
{
.ver_exclude = 300,
.ver_es_exclude = 200, // FBOs are core in ES2
.extension = "GL_EXT_framebuffer_object",
.provides = MPGL_CAP_FB,
.functions = (const struct gl_function[]) {
DEF_FN_NAME(BindFramebuffer, "glBindFramebufferEXT"),
DEF_FN_NAME(GenFramebuffers, "glGenFramebuffersEXT"),
DEF_FN_NAME(DeleteFramebuffers, "glDeleteFramebuffersEXT"),
DEF_FN_NAME(CheckFramebufferStatus, "glCheckFramebufferStatusEXT"),
DEF_FN_NAME(FramebufferTexture2D, "glFramebufferTexture2DEXT"),
DEF_FN_NAME(GetFramebufferAttachmentParameteriv,
"glGetFramebufferAttachmentParameterivEXT"),
{0}
},
},
// Framebuffers, extension in GL 2.x, core in GL 3.x core.
{
.ver_core = 300,
Expand Down
6 changes: 6 additions & 0 deletions video/out/opengl/context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,12 @@ int ra_gl_ctx_color_depth(struct ra_swapchain *sw)
if ((gl->es < 300 && !gl->version) || !(gl->mpgl_caps & MPGL_CAP_FB))
return 0;

// Querying the default framebuffer needs GL_ARB_framebuffer_object or
// GL 3.0+; GL_EXT_framebuffer_object does not allow it.
if (!p->main_fb && gl->version && gl->version < 300 &&

@kasper93 kasper93 Jul 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need !p->main_fb check here? GL_FRAMEBUFFER_ATTACHMENT_GREEN_SIZE is itself a GL 3.0 / ARB_framebuffer_object. So if libmpv api user provides FBO we would still try to query it and fail. So we can early exit. no?

!gl_check_extension(gl->extensions, "GL_ARB_framebuffer_object"))
return 0;

gl->BindFramebuffer(GL_FRAMEBUFFER, p->main_fb);

GLenum obj = gl->version ? GL_BACK_LEFT : GL_BACK;
Expand Down
19 changes: 18 additions & 1 deletion video/out/opengl/formats.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ const struct gl_format gl_formats[] = {
// Specifically not color-renderable.
{"rgb16", GL_RGB16, GL_RGB, T_U16, F_TF | F_EXT16},

// GL 2.1 with GL_ARB/EXT_texture_rg, but without requiring FBOs or float
// textures (unlike F_GL2F). Upload/filtering only.
{"r8", GL_R8, GL_RED, T_U8, F_TF | F_GL2RG},
{"rg8", GL_RG8, GL_RG, T_U8, F_TF | F_GL2RG},

// ES2 with GL_EXT_texture_rg. ES2 requires unsized internal formats.
{"r", GL_RED, GL_RED, T_U8, F_TF | F_ES2RG},
{"rg", GL_RG, GL_RG, T_U8, F_TF | F_ES2RG},

// GL2 legacy. Ignores possibly present FBO extensions (no CF flag set).
{"l8", GL_LUMINANCE8, GL_LUMINANCE, T_U8, F_TF | F_GL2},
{"la8", GL_LUMINANCE8_ALPHA8, GL_LUMINANCE_ALPHA, T_U8, F_TF | F_GL2},
Expand Down Expand Up @@ -104,7 +113,7 @@ const struct gl_format gl_formats[] = {
// Return an or-ed combination of all F_ flags that apply.
int gl_format_feature_flags(GL *gl)
{
return (gl->version == 210 ? F_GL2 : 0)
int flags = (gl->version == 210 ? F_GL2 : 0)
| (gl->version >= 300 ? F_GL3 : 0)
| (gl->es == 200 ? F_ES2 : 0)
| (gl->es >= 300 ? F_ES3 : 0)
Expand All @@ -116,7 +125,15 @@ int gl_format_feature_flags(GL *gl)
(gl->mpgl_caps & MPGL_CAP_ARB_FLOAT) &&
(gl->mpgl_caps & MPGL_CAP_TEX_RG) &&
(gl->mpgl_caps & MPGL_CAP_FB)) ? F_GL2F : 0)
| ((gl->version == 210 &&
(gl->mpgl_caps & MPGL_CAP_TEX_RG)) ? F_GL2RG : 0)
| ((gl->es == 200 &&
(gl->mpgl_caps & MPGL_CAP_TEX_RG)) ? F_ES2RG : 0)
| (gl->mpgl_caps & MPGL_CAP_APPLE_RGB_422 ? F_APPL : 0);
// F_GL2RG is a subset of F_GL2F; avoid duplicate format entries.

@kasper93 kasper93 Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? What is this fixing? F_GL2RG is non-float as I understand so why you bundle it with F_GL2F?

@anarsoul anarsoul Jul 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR adds new entries for r8/rg8 formats (and r/rg for GLES2). Since F_GL2RG is a subset of F_GL2F (which is F_GL2RG + float textures + ARB_framebuffer_object), that would result in two r8/rg8 formats registered twice.

This particular line enforces mutual exclusivity - i.e. full color-renderable will be used with F_GL2F hardware, while filter-only fallback is only used on F_GL2RG

@anarsoul anarsoul Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is "clear F_GL2RG is F_GL2F is set", I hope it makes sense.

@kasper93 kasper93 Jul 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular line enforces mutual exclusivity - i.e. full color-renderable will be used with F_GL2F hardware, while filter-only fallback is only used on F_GL2RG

The format lookup always prefer formats higher on the lists. It's priority list of formats. So there is no issue in having lesser formats down the list. We already have duplication on master between F_GL2 and F_GL2F.

Either way, it's fine, just something that is rather not focused on main change.

if (flags & F_GL2F)
flags &= ~F_GL2RG;
return flags;
}

int gl_format_type(const struct gl_format *format)
Expand Down
2 changes: 2 additions & 0 deletions video/out/opengl/formats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ enum {
F_EXTF16 = 1 << 6, // GL_EXT_color_buffer_half_float
F_GL2F = 1 << 7, // GL2.1-only with texture_rg + texture_float + FBOs
F_APPL = 1 << 8, // GL_APPLE_rgb_422
F_GL2RG = 1 << 9, // GL2.1-only with texture_rg (no float/FBO requirement)
F_ES2RG = 1 << 10, // ES2-only with GL_EXT_texture_rg

// Feature flags. They are additional and signal presence of features.
F_CR = 1 << 16, // color-renderable
Expand Down
Loading