Skip to content

Implement the OpenGL backend of the ExternalTexture plugin - #1780

Open
cdschmidt wants to merge 6 commits into
BabylonJS:masterfrom
cdschmidt:users/caschmidt/external-texture-opengl
Open

Implement the OpenGL backend of the ExternalTexture plugin#1780
cdschmidt wants to merge 6 commits into
BabylonJS:masterfrom
cdschmidt:users/caschmidt/external-texture-opengl

Conversation

@cdschmidt

@cdschmidt cdschmidt commented Jul 13, 2026 •
edited by bghgary
Loading

Copy link
Copy Markdown

[Created by Copilot on behalf of @bghgary]

Problem

The OpenGL ExternalTexture backend was a stub. Implementing it against a bare GL texture name leaked two backend-specific contracts into cross-platform code: width/height on the constructor and Update(), and an ownership rule where the caller had to keep the texture alive on GL but not on D3D/Metal.

TextureT is ID3D11Resource* / ID3D12Resource* / MTL::Texture* elsewhere — a pointer to a reference-counted image that knows its own properties. A GL name is an integer with no reference count that can't report its dimensions on ES 3.0 (glGetTexLevelParameteriv is ES 3.1).

Fix

Graphics::GL::Texture adds the missing pieces: an intrusive reference count, width/height/format/usage, and a release callback — glDeleteTextures when we own the image, a no-op when something else does (an Android SurfaceTexture, say). GL::SharedPtr holds it, shaped like winrt::com_ptr. TextureT on OpenGL becomes GL::Texture*.

The count is intrusive because two std::shared_ptrs built from one raw pointer get two control blocks and double-release.

Result: ExternalTexture.h is byte-identical to master.

Worth a look

  • NativeHandleToUintPtr would have passed bgfx the wrapper's address once TextureT became a pointer. Replaced by per-backend Graphics::NativeTextureHandle — reinterpret_cast on D3D/Metal, ptr->Handle() on GL.
  • The scratch-framebuffer format probe and its "unsupported format" failure mode are gone; it existed only to work around ES 3.0's missing glGetTexLevelParameteriv.
  • Texture::Usage records whether the image may be attached to a framebuffer, since GL can't be asked. BGFX_TEXTURE_RT was previously set unconditionally, narrowing bgfx's format validation to renderable formats for sampled-only images.
  • The release callback runs on whichever thread drops the last reference, so one issuing GL calls must marshal to the graphics thread. Documented on the class.

Testing

Install/Test gains an OpenGL/Linux configuration, run by the Linux install workflow, so the new public headers are compile-checked against an installed SDK.

Tests.ExternalTexture.cpp differs from master by one line: AddToContextAsyncWithLayerIndex is skipped where texture arrays are unavailable.

@cdschmidt
cdschmidt force-pushed the users/caschmidt/external-texture-opengl branch 2 times, most recently from ac67b20 to 44becb9 Compare July 13, 2026 22:03
@cdschmidt cdschmidt changed the title ExternalTexture_OpenGL: implement GetInfo/Assign for GL texture handles ExternalTexture_OpenGL: implement GetInfo/Set/Get for GL texture handles Jul 13, 2026
@cdschmidt
cdschmidt force-pushed the users/caschmidt/external-texture-opengl branch from 44becb9 to 7733546 Compare July 13, 2026 22:13
@cdschmidt

cdschmidt commented Jul 13, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Microsoft"

@cdschmidt
cdschmidt marked this pull request as ready for review July 14, 2026 18:57
Copilot AI review requested due to automatic review settings July 14, 2026 18:57
Copilot AI reviewed Jul 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request replaces the previously stubbed OpenGL(OpenGLES) ExternalTexture backend with a working implementation, aligning the OpenGL path with the existing per-API Impl + shared-dispatcher pattern used by the other backends.

Changes:

  • Implement OpenGL ExternalTexture::Impl GetInfo/Set/Get to support importing GL texture handles into bgfx.
  • Add a bgfx-format ⇄ GL internalFormat reverse-lookup table (with sRGB detection) plus guarded enum fallbacks for minimal GLES headers.
  • Validate texture handles via glIsTexture, enforce GL_TEXTURE_2D-only handles, and query dimensions/mip count via GL texture level parameters.

Comment thread Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp Outdated
Comment thread Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp Outdated
Comment thread Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp
@cdschmidt
cdschmidt force-pushed the users/caschmidt/external-texture-opengl branch from 0f2a2eb to c7e9a7f Compare July 14, 2026 22:05
@cdschmidt cdschmidt mentioned this pull request Jul 16, 2026
Closed
@bkaradzic-microsoft

bkaradzic-microsoft commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Merged latest bgfx which has external texture GL fix:
#1784

Please rebase to latest.

caschmidt and others added 2 commits July 21, 2026 16:04
caschmidt
ExternalTexture_OpenGL: implement GetInfo/Set/Get for GL texture handles
3e5a032 
Implements the previously-stubbed OpenGL backend of the ExternalTexture
plugin. Mirrors the D3D11 backend pattern: per-API Impl class derived
from ImplBase, GetInfo/Set/Get triplet, full bgfx<->GL format table.

Key details:
- 96-row s_textureFormat[] with static_assert against bgfx::TextureFormat::Count.
- All non-core extension constants (S3TC, BPTC, ASTC, BGRA, etc.) guarded
  with #ifndef + OpenGL registry hex fallbacks so it compiles on minimal
  Mesa GLES headers.
- Always sets BGFX_TEXTURE_RT (FBO bridging is the only supported use case;
  GL has no introspection equivalent of D3D11 BIND_RENDER_TARGET).
- glIsTexture validation + pre/post-bind error drain; bounded mip walk;
  GL_TEXTURE_BINDING_2D save/restore on every exit path.
- Sets NumLayers = 1 (GL_TEXTURE_2D is single-layer); array/multisample/cube
  handles are rejected by the GL_TEXTURE_2D-only bind check.
- Reverse format lookup skips the BGRA-ordered rows (BGRA8/BGRA4/BGR5A1/
  B5G6R5): GL has no distinct BGRA internal format, so bgfx maps them to the
  same GL internal format as their RGBA twins. Skipping ensures a GL_RGBA8
  attachment resolves to RGBA8 rather than the earlier-listed BGRA8 (which
  would swap R/B).
- Throws on unsupported format rather than returning Unknown.
- MSAA explicitly unsupported for now (would need TEXTURE_2D_MULTISAMPLE +
  GL_TEXTURE_SAMPLES query).

Review feedback:
- Guard the GLES 3.1 header include with __has_include, falling back to gl3.h
  (declaring glGetTexLevelParameteriv + GL_TEXTURE_WIDTH/HEIGHT) so builds on
  toolchains without gl31.h don't fail.
- Expand the GL_TEXTURE_2D-only rejection message to also name array textures.
- Enable OpenGL test coverage: implement the GL Helpers::CreateTexture/
  DestroyTexture, drop SKIP_EXTERNAL_TEXTURE_TESTS on UNIX (run Construction/
  CreateForJavaScript/Update/AddToContextAsyncAndUpdate). Render-path tests
  (SKIP_RENDER_TESTS) and the array/layer-index test (SKIP_EXTERNAL_TEXTURE_
  ARRAY_TESTS) remain skipped pending GL readback + array support.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
caschmidt
ExternalTexture_OpenGL: fix static_assert and add width/height hints
3f95a3c 
Grow the OpenGL backend's s_textureFormat[] reverse-map table to cover all
100 bgfx::TextureFormat::Count formats (was 96), fixing the
"bgfx::TextureFormat::Count == BX_COUNTOF(s_textureFormat)" static assertion
that broke every Linux/OpenGL build. Reads GL internal formats back via ES 3.0
framebuffer-attachment queries and matches them to bgfx formats.

Add optional width/height hints to the ExternalTexture constructor and Update:
required on the OpenGL/OpenGL ES backend (ES 3.0 cannot query a texture's
dimensions from a bare handle) and ignored on D3D/Metal, which introspect the
native texture.

Move the UnitTests SKIP_RENDER_TESTS / SKIP_EXTERNAL_TEXTURE_ARRAY_TESTS
definitions into a GRAPHICS_API==OpenGL block so they apply to every OpenGL
build rather than only the Linux path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 266f2ffa-1a30-41c7-96de-0ec7782df1bd
@cdschmidt
cdschmidt force-pushed the users/caschmidt/external-texture-opengl branch from eff3913 to 3f95a3c Compare July 21, 2026 22:05
caschmidt and others added 4 commits July 21, 2026 17:32
caschmidt
ExternalTexture: keep source GL texture alive until bgfx is done with it
3219da4 
The OpenGL/OpenGL ES ExternalTexture backend stores the raw GL texture name
without taking ownership (OpenGL has no texture-name reference counting),
unlike the D3D11/D3D12/Metal backends which retain the native texture. The
unit tests destroyed the source texture (glDeleteTextures) immediately after
construction, leaving a dangling GL name. On the next frame bgfx dereferences
it (glObjectLabel under BX_CONFIG_DEBUG), which strict drivers (Mesa on the
Linux CI runners) reject with GL_INVALID_VALUE, aborting the process. ANGLE on
Windows is permissive, so it only reproduced on Linux.

Move each DestroyTexture call to after the last frame that references the
texture, and document the GL ownership contract in ExternalTexture.h. No
backend code change is required; bgfx behaves correctly.

Verified in a clean Ubuntu 24.04 container (matching CI: GCC + JavaScriptCore,
RelWithDebInfo, BX_CONFIG_DEBUG=ON): UnitTests now exits 0 (9 passed, 5
skipped) where it previously core-dumped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 266f2ffa-1a30-41c7-96de-0ec7782df1bd
@bghgary
ExternalTexture: represent an OpenGL texture as a reference-counted i…
6f2fe31 
…mage

On D3D and Metal, TextureT is a pointer to a reference-counted image. On OpenGL it was a bare
texture name, which carries neither a reference count nor any metadata, and that difference
leaked into cross-platform code as width/height parameters on ExternalTexture's constructor and
Update() that were required on GL and ignored elsewhere, plus an ownership caveat that only GL
callers had to honor.

Babylon::Graphics::GL::Texture supplies what the GL name lacks: an intrusive reference count,
the width, height, format and usage that D3D and Metal read back from the resource itself, and
a release callback that decides what releasing means. GL::SharedPtr holds it, shaped like
winrt::com_ptr so the per-backend plugin code reads the same. TextureT on OpenGL becomes
GL::Texture*, so value semantics match the other backends exactly.

ExternalTexture.h is now byte-identical to master: no width/height, no ownership caveat. Every
backend takes its own reference and holds it, and the caller releases whenever it likes.

Because TextureT is a pointer everywhere, the shared NativeHandleToUintPtr helper would have
handed bgfx the wrapper's address instead of the GL name. It is replaced by a per-backend
Graphics::NativeTextureHandle: a reinterpret_cast on D3D and Metal, ptr->Handle() on GL.

The scratch-framebuffer format probe is gone along with its "unsupported format" failure mode.
It existed only because glGetTexLevelParameteriv is OpenGL ES 3.1 while BabylonNative renders
on an ES 3.0 context; the format now travels with the image.

The only remaining test delta is that AddToContextAsyncWithLayerIndex is skipped where texture
arrays are unavailable; GL supports ExternalTexture but not array layers.

Install/Test gains an OpenGL/Linux configuration and the Linux install workflow runs it, so the
new public headers are compile-checked against an installed SDK in CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 714b4495-258e-4645-abf7-26c17bc29d5b
@bghgary
Merge remote-tracking branch 'origin/master' into external-image-desc…
4ec5ec5 
…riptor-sketch
@bghgary
ExternalTexture_OpenGL: add the texture formats bgfx gained since thi…
a2f8db6 
…s branch

Merging master brought a bgfx update that adds BC4S, BC5S, BC6HU, RGB10A2U and D32FS8, so
the reverse-mapping table no longer satisfied its static_assert against
bgfx::TextureFormat::Count. The GL internal formats match bgfx's own s_textureFormat in
renderer_gl.cpp, and the two LATC and one BPTC constant get the same #ifndef registry
fallbacks as their existing counterparts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 714b4495-258e-4645-abf7-26c17bc29d5b
@bghgary bghgary changed the title ExternalTexture_OpenGL: implement GetInfo/Set/Get for GL texture handles Implement the OpenGL backend of the ExternalTexture plugin Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cdschmidt @bkaradzic-microsoft @bghgary