Skip to content

Fix: skip legacy ARB-extension name check on core-profile GL contexts - #3022

Open
tomjn wants to merge 1 commit into
beyond-all-reason:masterfrom
tomjn:fix/gl-core-profile-ext-check
Open

Fix: skip legacy ARB-extension name check on core-profile GL contexts#3022
tomjn wants to merge 1 commit into
beyond-all-reason:masterfrom
tomjn:fix/gl-core-profile-ext-check

Conversation

@tomjn

@tomjn tomjn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

What

CGlobalRendering::CheckGLExtensions() validates a list of legacy ARB extensions by name. In an OpenGL core profile context these ARB extensions are not advertised by name (they were folded into GL 1.3/2.0/3.0 long ago), even though their functionality is guaranteed by the spec. The name check therefore fails spuriously on a perfectly valid core context.

This adds an early return when globalRenderingInfo.glContextIsCore is set, skipping the legacy-extension name check on core profiles.

Why this is cross-platform

The guard is on the runtime glContextIsCore flag, not __APPLE__. It is correct for any core-profile context on any platform; it is a no-op for compatibility-profile contexts (the common case today).

Provenance

Extracted unmodified (via git cherry-pick) from commit 17c55deecf in #2991 (the interim macOS bring-up). Original author credit is preserved. glContextIsCore already exists on master, so the change is self-contained.

The engine checks for ARB_multitexture, ARB_texture_env_combine,
ARB_texture_compression, ARB_texture_float,
ARB_texture_non_power_of_two, and ARB_framebuffer_object extensions
by name. Per the GL spec, these were folded into core GL 1.3-3.0;
core-profile contexts no longer advertise them by name, but their
functionality is guaranteed.

The name-only check is a false-negative on any core-profile context.
Skip it when the active context is core profile so the engine doesn't
spuriously reject otherwise-valid configurations.

@sprunk sprunk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks reasonable, though glContextIsCore seems to also have an alternative resolution by looking at some gl_ARB compat var rather than at the actual "is core" bit so I'm not sure how reliable it is at the edge case:

if (profile != 0)
globalRenderingInfo.glContextIsCore = (profile == GL_CONTEXT_CORE_PROFILE_BIT);
else
globalRenderingInfo.glContextIsCore = !GLAD_GL_ARB_compatibility;

@tomjn

tomjn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@sprunk so I nudged Claude about this and it sort of agreed with you but not quite

You're right that the else branch is a heuristic — it's only hit when GL_CONTEXT_PROFILE_MASK reports 0 (pre-3.2 contexts). The early-return here is still safe in that case, though: it only triggers when the context is detected as core, and the fallback yields core=true only when ARB_compatibility is absent — i.e. a genuinely core/forward-compatible context, where the ARB extensions being checked are guaranteed by the version. A compat context reports ARB_compatibility and so never reaches the early-return. The only misfire would require a compat context that advertises no ARB_compatibility and is actually missing a required extension, which would be non-conformant.

If you'd prefer to avoid depending on the core/compat detection entirely, I can instead gate the skip on the context version (these extensions were all core by GL 3.0), e.g. if (globalRenderingInfo.glContextVersion.x >= 3) return;. Happy to switch to whichever you find more robust.

The one thing I do remember is that on MacOS for those OpenGL contexts it does support, they were a limited set of core profiles with no compatibility profiles at all ( this was one of the bigger contributors that killed the original springrts macos efforts many years ago ).

@sprunk sprunk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The only misfire would require a compat context that advertises no ARB_compatibility and is actually missing a required extension, which would be non-conformant.

In theory the answer is "yes, but there's always been issues with non-conformance so we need to be careful" but maybe the only affected implementations would be so old it doesn't matter much.

@sprunk sprunk added the area: Graphics/Rendering Graphics pipeline, shaders, water rendering, reflections, OpenGL code, etc. label Jun 24, 2026

@lostsquirrel1 lostsquirrel1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great idea. Though I think we can make it more robust (unless I've misunderstood something.)

Comment on lines +785 to +788
// In an OpenGL CORE profile context these ARB extensions are not advertised
// by name (they were folded into GL 1.3/2.0/3.0 long ago) but their
// functionality is guaranteed by the spec. Skip the legacy-extension check.
if (globalRenderingInfo.glContextIsCore)

@lostsquirrel1 lostsquirrel1 Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We usually load the compatibility profile rather than core; we want to also avoid checking for unnecessary extensions.

Suggested change
// In an OpenGL CORE profile context these ARB extensions are not advertised
// by name (they were folded into GL 1.3/2.0/3.0 long ago) but their
// functionality is guaranteed by the spec. Skip the legacy-extension check.
if (globalRenderingInfo.glContextIsCore)
// All checked ARB extensions are part of the core specification since OpenGL 3.0.
if (globalRenderingInfo.glVersionNum >= 30)

The only problem with my suggestion is that I can't see where glVersionNum gets correctly initialized. Engine 2025.04 apparently did this, but I'm not seeing it, so it would be worth checking - maybe we have anopther bug because a Lua Platform.glVersionNum depends on this value.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

glVersionNum was added in 81490a7 but that probably sets the incorrect value, see #3149

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@sprunk @lostsquirrel1 is this something that needs to be changed or is the PR okay as is? Or is it blocked by #3149? The next steps here appear to be inconclusive/unclear

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we need the suggested change, but yes it is blocked by #3149

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm a bit busy, feel free to pick #3149 up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Graphics/Rendering Graphics pipeline, shaders, water rendering, reflections, OpenGL code, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants