From 3e5a0327645959e8b599a4832a0fa0624faf7740 Mon Sep 17 00:00:00 2001 From: caschmidt Date: Wed, 6 May 2026 19:23:30 -0600 Subject: [PATCH 1/3] ExternalTexture_OpenGL: implement GetInfo/Set/Get for GL texture handles 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> --- Apps/UnitTests/CMakeLists.txt | 8 +- Apps/UnitTests/Source/Helpers.OpenGL.cpp | 41 +- .../Source/Tests.ExternalTexture.cpp | 2 +- .../Source/ExternalTexture_OpenGL.cpp | 562 ++++++++++++++++-- 4 files changed, 557 insertions(+), 56 deletions(-) diff --git a/Apps/UnitTests/CMakeLists.txt b/Apps/UnitTests/CMakeLists.txt index 4ffe04381..c0b6e429f 100644 --- a/Apps/UnitTests/CMakeLists.txt +++ b/Apps/UnitTests/CMakeLists.txt @@ -57,7 +57,13 @@ if(APPLE) set(ADDITIONAL_LIBRARIES PRIVATE ${JAVASCRIPTCORE_LIBRARY}) elseif(UNIX AND NOT ANDROID) set(SOURCES ${SOURCES} "Source/App.X11.cpp") - set(ADDITIONAL_COMPILE_DEFINITIONS PRIVATE SKIP_EXTERNAL_TEXTURE_TESTS) + # The OpenGL ExternalTexture backend supports single-sample GL_TEXTURE_2D + # handles, so run the basic GetInfo/Set/Get/Update tests. The render-path + # tests (pixel readback) and the array/layer-index test require GL test + # helpers and array-texture support that this backend does not implement yet. + set(ADDITIONAL_COMPILE_DEFINITIONS + PRIVATE SKIP_RENDER_TESTS + PRIVATE SKIP_EXTERNAL_TEXTURE_ARRAY_TESTS) elseif(WIN32) set(SOURCES ${SOURCES} "Source/App.Win32.cpp" diff --git a/Apps/UnitTests/Source/Helpers.OpenGL.cpp b/Apps/UnitTests/Source/Helpers.OpenGL.cpp index d7a3beba2..aa3533009 100644 --- a/Apps/UnitTests/Source/Helpers.OpenGL.cpp +++ b/Apps/UnitTests/Source/Helpers.OpenGL.cpp @@ -1,18 +1,51 @@ #include #include "Helpers.h" +#include + #include namespace Helpers { - Babylon::Graphics::TextureT CreateTexture(Babylon::Graphics::DeviceT, uint32_t, uint32_t, uint32_t, bool, uint32_t) + // The OpenGL ExternalTexture backend imports single-sample, non-array + // GL_TEXTURE_2D handles. glGenTextures/glTexImage2D require a current GL + // context, which the single-threaded Graphics::Device makes current on the + // calling thread between StartRenderingCurrentFrame/FinishRenderingCurrentFrame + // (the same thread the tests create textures on). + Babylon::Graphics::TextureT CreateTexture(Babylon::Graphics::DeviceT, uint32_t width, uint32_t height, uint32_t arraySize, bool renderTarget, uint32_t samples) { - throw std::runtime_error{"not implemented"}; + // The array / render-target / MSAA variants are only exercised by the + // render-path tests, which are skipped on this backend (see CMakeLists.txt). + if (arraySize != 1 || renderTarget || samples != 1) + { + throw std::runtime_error{"Helpers::CreateTexture(OpenGL): only single-sample, non-array GL_TEXTURE_2D textures are supported"}; + } + + GLint previousBinding = 0; + glGetIntegerv(GL_TEXTURE_BINDING_2D, &previousBinding); + + GLuint texture = 0; + glGenTextures(1, &texture); + glBindTexture(GL_TEXTURE_2D, texture); + + // Allocate a single, texture-complete mip level. The pixel contents are + // never read back by the enabled tests; only the queried dimensions and + // internal format matter. + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, static_cast(width), static_cast(height), 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + + glBindTexture(GL_TEXTURE_2D, static_cast(previousBinding)); + + EXPECT_EQ(glGetError(), static_cast(GL_NO_ERROR)); + + return texture; } - void DestroyTexture(Babylon::Graphics::TextureT) + void DestroyTexture(Babylon::Graphics::TextureT texture) { - throw std::runtime_error{"not implemented"}; + GLuint handle = texture; + glDeleteTextures(1, &handle); } Babylon::Graphics::TextureT CreateTextureArrayWithData(Babylon::Graphics::DeviceT, uint32_t, uint32_t, const Color*, uint32_t) diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp index 55083d209..cc2f3a378 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp @@ -169,7 +169,7 @@ TEST(ExternalTexture, AddToContextAsyncAndUpdate) TEST(ExternalTexture, AddToContextAsyncWithLayerIndex) { -#ifdef SKIP_EXTERNAL_TEXTURE_TESTS +#if defined(SKIP_EXTERNAL_TEXTURE_TESTS) || defined(SKIP_EXTERNAL_TEXTURE_ARRAY_TESTS) GTEST_SKIP(); #else Babylon::Graphics::Device device{g_deviceConfig}; diff --git a/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp b/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp index 539a000c0..46ed06291 100644 --- a/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp +++ b/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp @@ -1,50 +1,512 @@ -#include -#include -#include -#include -#include -#include -#include -#include - -#include "ExternalTexture_Base.h" - -namespace Babylon::Plugins -{ - // Suppress C4702 for the shared dispatcher - Impl stubs always throw, - // making the dispatcher's post-call code unreachable. - class ExternalTexture::Impl final : public ImplBase - { - public: - // Implemented in ExternalTexture_Shared.h - Impl(Graphics::TextureT, std::optional); - void Update(Graphics::TextureT, std::optional, std::optional); - - Graphics::TextureT Get() const - { - throw std::runtime_error{"not implemented"}; - } - - private: - static void GetInfo(Graphics::TextureT, std::optional, Info&) - { - throw std::runtime_error{"not implemented"}; - } - - void Set(Graphics::TextureT) - { - throw std::runtime_error{"not implemented"}; - } - }; -} - -#if defined(_MSC_VER) -#pragma warning(push) -#pragma warning(disable : 4702) // unreachable code (Impl stubs always throw) -#endif - -#include "ExternalTexture_Shared.h" - -#if defined(_MSC_VER) -#pragma warning(pop) -#endif +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +// glGetTexLevelParameteriv and the GL_TEXTURE_WIDTH/HEIGHT query tokens require +// OpenGL ES 3.1+. Both Linux (Mesa) and Android NDK (API 21+) ship the gl31.h +// header, so prefer it. Some minimal GLES toolchains only ship gl3.h; rather +// than fail the build there, fall back to gl3.h and supply the handful of 3.1 +// entry points/tokens we use. They resolve at runtime in the GLES 3.1+/desktop +// GL contexts bgfx creates for this backend. +#if !defined(__has_include) || __has_include() +# include +#else +# include +# ifndef GL_TEXTURE_WIDTH +# define GL_TEXTURE_WIDTH 0x1000 +# endif +# ifndef GL_TEXTURE_HEIGHT +# define GL_TEXTURE_HEIGHT 0x1001 +# endif +extern "C" GL_APICALL void GL_APIENTRY glGetTexLevelParameteriv(GLenum target, GLint level, GLenum pname, GLint* params); +#endif + +#include "ExternalTexture_Base.h" + +// The bgfx OpenGLES backend uses a number of compressed/extension texture +// formats whose enum values are not present in the standard GLES headers. +// Define them here (values per the OpenGL registry) so the format table +// below mirrors bgfx/src/renderer_gl.cpp's s_textureFormat[]. These are +// used purely for reverse-mapping a queried internalFormat to a +// bgfx::TextureFormat::Enum and never passed to a GL call here, so it is +// safe to define them unconditionally. +#ifndef GL_COMPRESSED_RGBA_S3TC_DXT1_EXT +# define GL_COMPRESSED_RGBA_S3TC_DXT1_EXT 0x83F1 +#endif +#ifndef GL_COMPRESSED_RGBA_S3TC_DXT3_EXT +# define GL_COMPRESSED_RGBA_S3TC_DXT3_EXT 0x83F2 +#endif +#ifndef GL_COMPRESSED_RGBA_S3TC_DXT5_EXT +# define GL_COMPRESSED_RGBA_S3TC_DXT5_EXT 0x83F3 +#endif +#ifndef GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT +# define GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT 0x8C4D +#endif +#ifndef GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT +# define GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT 0x8C4E +#endif +#ifndef GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT +# define GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT 0x8C4F +#endif +#ifndef GL_COMPRESSED_LUMINANCE_LATC1_EXT +# define GL_COMPRESSED_LUMINANCE_LATC1_EXT 0x8C70 +#endif +#ifndef GL_COMPRESSED_LUMINANCE_ALPHA_LATC2_EXT +# define GL_COMPRESSED_LUMINANCE_ALPHA_LATC2_EXT 0x8C72 +#endif +#ifndef GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT_ARB +# define GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT_ARB 0x8E8E +#endif +#ifndef GL_COMPRESSED_RGBA_BPTC_UNORM_ARB +# define GL_COMPRESSED_RGBA_BPTC_UNORM_ARB 0x8E8C +#endif +#ifndef GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM_ARB +# define GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM_ARB 0x8E8D +#endif +#ifndef GL_ETC1_RGB8_OES +# define GL_ETC1_RGB8_OES 0x8D64 +#endif +#ifndef GL_COMPRESSED_SRGB8_ETC2 +# define GL_COMPRESSED_SRGB8_ETC2 0x9275 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC +# define GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC 0x9279 +#endif +#ifndef GL_COMPRESSED_RGB_PVRTC_2BPPV1_IMG +# define GL_COMPRESSED_RGB_PVRTC_2BPPV1_IMG 0x8C01 +#endif +#ifndef GL_COMPRESSED_RGB_PVRTC_4BPPV1_IMG +# define GL_COMPRESSED_RGB_PVRTC_4BPPV1_IMG 0x8C00 +#endif +#ifndef GL_COMPRESSED_RGBA_PVRTC_2BPPV1_IMG +# define GL_COMPRESSED_RGBA_PVRTC_2BPPV1_IMG 0x8C03 +#endif +#ifndef GL_COMPRESSED_RGBA_PVRTC_4BPPV1_IMG +# define GL_COMPRESSED_RGBA_PVRTC_4BPPV1_IMG 0x8C02 +#endif +#ifndef GL_COMPRESSED_SRGB_PVRTC_2BPPV1_EXT +# define GL_COMPRESSED_SRGB_PVRTC_2BPPV1_EXT 0x8A54 +#endif +#ifndef GL_COMPRESSED_SRGB_PVRTC_4BPPV1_EXT +# define GL_COMPRESSED_SRGB_PVRTC_4BPPV1_EXT 0x8A55 +#endif +#ifndef GL_COMPRESSED_SRGB_ALPHA_PVRTC_2BPPV1_EXT +# define GL_COMPRESSED_SRGB_ALPHA_PVRTC_2BPPV1_EXT 0x8A56 +#endif +#ifndef GL_COMPRESSED_SRGB_ALPHA_PVRTC_4BPPV1_EXT +# define GL_COMPRESSED_SRGB_ALPHA_PVRTC_4BPPV1_EXT 0x8A57 +#endif +#ifndef GL_COMPRESSED_RGBA_PVRTC_2BPPV2_IMG +# define GL_COMPRESSED_RGBA_PVRTC_2BPPV2_IMG 0x9137 +#endif +#ifndef GL_COMPRESSED_RGBA_PVRTC_4BPPV2_IMG +# define GL_COMPRESSED_RGBA_PVRTC_4BPPV2_IMG 0x9138 +#endif +#ifndef GL_ATC_RGB_AMD +# define GL_ATC_RGB_AMD 0x8C92 +#endif +#ifndef GL_ATC_RGBA_EXPLICIT_ALPHA_AMD +# define GL_ATC_RGBA_EXPLICIT_ALPHA_AMD 0x8C93 +#endif +#ifndef GL_ATC_RGBA_INTERPOLATED_ALPHA_AMD +# define GL_ATC_RGBA_INTERPOLATED_ALPHA_AMD 0x87EE +#endif +#ifndef GL_R16 +# define GL_R16 0x822A +#endif +#ifndef GL_R16_SNORM +# define GL_R16_SNORM 0x8F98 +#endif +#ifndef GL_RG16 +# define GL_RG16 0x822C +#endif +#ifndef GL_RG16_SNORM +# define GL_RG16_SNORM 0x8F99 +#endif +#ifndef GL_RGBA16 +# define GL_RGBA16 0x805B +#endif +#ifndef GL_RGBA16_SNORM +# define GL_RGBA16_SNORM 0x8F9B +#endif +#ifndef GL_BGRA +# define GL_BGRA 0x80E1 +#endif +#ifndef GL_DEPTH_COMPONENT32 +# define GL_DEPTH_COMPONENT32 0x81A7 +#endif +#ifndef GL_TEXTURE_INTERNAL_FORMAT +# define GL_TEXTURE_INTERNAL_FORMAT 0x1003 +#endif +// ASTC (KHR_texture_compression_astc_ldr). Standard GLES 3.1 headers do not +// always declare these; define them here so the table compiles. +#ifndef GL_COMPRESSED_RGBA_ASTC_4x4_KHR +# define GL_COMPRESSED_RGBA_ASTC_4x4_KHR 0x93B0 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_5x4_KHR +# define GL_COMPRESSED_RGBA_ASTC_5x4_KHR 0x93B1 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_5x5_KHR +# define GL_COMPRESSED_RGBA_ASTC_5x5_KHR 0x93B2 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_6x5_KHR +# define GL_COMPRESSED_RGBA_ASTC_6x5_KHR 0x93B3 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_6x6_KHR +# define GL_COMPRESSED_RGBA_ASTC_6x6_KHR 0x93B4 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_8x5_KHR +# define GL_COMPRESSED_RGBA_ASTC_8x5_KHR 0x93B5 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_8x6_KHR +# define GL_COMPRESSED_RGBA_ASTC_8x6_KHR 0x93B6 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_8x8_KHR +# define GL_COMPRESSED_RGBA_ASTC_8x8_KHR 0x93B7 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_10x5_KHR +# define GL_COMPRESSED_RGBA_ASTC_10x5_KHR 0x93B8 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_10x6_KHR +# define GL_COMPRESSED_RGBA_ASTC_10x6_KHR 0x93B9 +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_10x8_KHR +# define GL_COMPRESSED_RGBA_ASTC_10x8_KHR 0x93BA +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_10x10_KHR +# define GL_COMPRESSED_RGBA_ASTC_10x10_KHR 0x93BB +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_12x10_KHR +# define GL_COMPRESSED_RGBA_ASTC_12x10_KHR 0x93BC +#endif +#ifndef GL_COMPRESSED_RGBA_ASTC_12x12_KHR +# define GL_COMPRESSED_RGBA_ASTC_12x12_KHR 0x93BD +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_4x4_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_4x4_KHR 0x93D0 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_5x4_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_5x4_KHR 0x93D1 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_5x5_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_5x5_KHR 0x93D2 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_6x5_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_6x5_KHR 0x93D3 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_6x6_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_6x6_KHR 0x93D4 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x5_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x5_KHR 0x93D5 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x6_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x6_KHR 0x93D6 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x8_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x8_KHR 0x93D7 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x5_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x5_KHR 0x93D8 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x6_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x6_KHR 0x93D9 +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x8_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x8_KHR 0x93DA +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x10_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x10_KHR 0x93DB +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x10_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x10_KHR 0x93DC +#endif +#ifndef GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x12_KHR +# define GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x12_KHR 0x93DD +#endif + +namespace Babylon::Plugins +{ + namespace + { + // Two-column slice of the bgfx OpenGLES s_textureFormat[] table + // (see bgfx/src/renderer_gl.cpp). Indexed by bgfx::TextureFormat::Enum. + // Used by GetInfo to reverse-map a queried GL internalFormat back to + // the corresponding bgfx::TextureFormat::Enum (with sRGB detection). + struct TextureFormatInfo + { + GLenum m_fmt; + GLenum m_fmtSrgb; + }; + + constexpr TextureFormatInfo s_textureFormat[] = + { + { GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT }, // BC1 + { GL_COMPRESSED_RGBA_S3TC_DXT3_EXT, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT }, // BC2 + { GL_COMPRESSED_RGBA_S3TC_DXT5_EXT, GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT }, // BC3 + { GL_COMPRESSED_LUMINANCE_LATC1_EXT, GL_ZERO }, // BC4 + { GL_COMPRESSED_LUMINANCE_ALPHA_LATC2_EXT, GL_ZERO }, // BC5 + { GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT_ARB, GL_ZERO }, // BC6H + { GL_COMPRESSED_RGBA_BPTC_UNORM_ARB, GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM_ARB }, // BC7 + { GL_ETC1_RGB8_OES, GL_ZERO }, // ETC1 + { GL_COMPRESSED_RGB8_ETC2, GL_COMPRESSED_SRGB8_ETC2 }, // ETC2 + { GL_COMPRESSED_RGBA8_ETC2_EAC, GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC }, // ETC2A + { GL_COMPRESSED_RGB8_PUNCHTHROUGH_ALPHA1_ETC2, GL_COMPRESSED_SRGB8_PUNCHTHROUGH_ALPHA1_ETC2 }, // ETC2A1 + { GL_COMPRESSED_RGB_PVRTC_2BPPV1_IMG, GL_COMPRESSED_SRGB_PVRTC_2BPPV1_EXT }, // PTC12 + { GL_COMPRESSED_RGB_PVRTC_4BPPV1_IMG, GL_COMPRESSED_SRGB_PVRTC_4BPPV1_EXT }, // PTC14 + { GL_COMPRESSED_RGBA_PVRTC_2BPPV1_IMG, GL_COMPRESSED_SRGB_ALPHA_PVRTC_2BPPV1_EXT }, // PTC12A + { GL_COMPRESSED_RGBA_PVRTC_4BPPV1_IMG, GL_COMPRESSED_SRGB_ALPHA_PVRTC_4BPPV1_EXT }, // PTC14A + { GL_COMPRESSED_RGBA_PVRTC_2BPPV2_IMG, GL_ZERO }, // PTC22 + { GL_COMPRESSED_RGBA_PVRTC_4BPPV2_IMG, GL_ZERO }, // PTC24 + { GL_ATC_RGB_AMD, GL_ZERO }, // ATC + { GL_ATC_RGBA_EXPLICIT_ALPHA_AMD, GL_ZERO }, // ATCE + { GL_ATC_RGBA_INTERPOLATED_ALPHA_AMD, GL_ZERO }, // ATCI + { GL_COMPRESSED_RGBA_ASTC_4x4_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_4x4_KHR }, // ASTC4x4 + { GL_COMPRESSED_RGBA_ASTC_5x4_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_5x4_KHR }, // ASTC5x4 + { GL_COMPRESSED_RGBA_ASTC_5x5_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_5x5_KHR }, // ASTC5x5 + { GL_COMPRESSED_RGBA_ASTC_6x5_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_6x5_KHR }, // ASTC6x5 + { GL_COMPRESSED_RGBA_ASTC_6x6_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_6x6_KHR }, // ASTC6x6 + { GL_COMPRESSED_RGBA_ASTC_8x5_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x5_KHR }, // ASTC8x5 + { GL_COMPRESSED_RGBA_ASTC_8x6_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x6_KHR }, // ASTC8x6 + { GL_COMPRESSED_RGBA_ASTC_8x8_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_8x8_KHR }, // ASTC8x8 + { GL_COMPRESSED_RGBA_ASTC_10x5_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x5_KHR }, // ASTC10x5 + { GL_COMPRESSED_RGBA_ASTC_10x6_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x6_KHR }, // ASTC10x6 + { GL_COMPRESSED_RGBA_ASTC_10x8_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x8_KHR }, // ASTC10x8 + { GL_COMPRESSED_RGBA_ASTC_10x10_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_10x10_KHR }, // ASTC10x10 + { GL_COMPRESSED_RGBA_ASTC_12x10_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x10_KHR }, // ASTC12x10 + { GL_COMPRESSED_RGBA_ASTC_12x12_KHR, GL_COMPRESSED_SRGB8_ALPHA8_ASTC_12x12_KHR }, // ASTC12x12 + { GL_ZERO, GL_ZERO }, // Unknown + { GL_ZERO, GL_ZERO }, // R1 + { GL_ALPHA, GL_ZERO }, // A8 + { GL_R8, GL_ZERO }, // R8 + { GL_R8I, GL_ZERO }, // R8I + { GL_R8UI, GL_ZERO }, // R8U + { GL_R8_SNORM, GL_ZERO }, // R8S + { GL_R16, GL_ZERO }, // R16 + { GL_R16I, GL_ZERO }, // R16I + { GL_R16UI, GL_ZERO }, // R16U + { GL_R16F, GL_ZERO }, // R16F + { GL_R16_SNORM, GL_ZERO }, // R16S + { GL_R32I, GL_ZERO }, // R32I + { GL_R32UI, GL_ZERO }, // R32U + { GL_R32F, GL_ZERO }, // R32F + { GL_RG8, GL_ZERO }, // RG8 + { GL_RG8I, GL_ZERO }, // RG8I + { GL_RG8UI, GL_ZERO }, // RG8U + { GL_RG8_SNORM, GL_ZERO }, // RG8S + { GL_RG16, GL_ZERO }, // RG16 + { GL_RG16I, GL_ZERO }, // RG16I + { GL_RG16UI, GL_ZERO }, // RG16U + { GL_RG16F, GL_ZERO }, // RG16F + { GL_RG16_SNORM, GL_ZERO }, // RG16S + { GL_RG32I, GL_ZERO }, // RG32I + { GL_RG32UI, GL_ZERO }, // RG32U + { GL_RG32F, GL_ZERO }, // RG32F + { GL_RGB8, GL_SRGB8 }, // RGB8 + { GL_RGB8I, GL_ZERO }, // RGB8I + { GL_RGB8UI, GL_ZERO }, // RGB8U + { GL_RGB8_SNORM, GL_ZERO }, // RGB8S + { GL_RGB9_E5, GL_ZERO }, // RGB9E5F + { GL_RGBA8, GL_SRGB8_ALPHA8 }, // BGRA8 (bgfx maps both to GL_RGBA8) + { GL_RGBA8, GL_SRGB8_ALPHA8 }, // RGBA8 + { GL_RGBA8I, GL_ZERO }, // RGBA8I + { GL_RGBA8UI, GL_ZERO }, // RGBA8U + { GL_RGBA8_SNORM, GL_ZERO }, // RGBA8S + { GL_RGBA16, GL_ZERO }, // RGBA16 + { GL_RGBA16I, GL_ZERO }, // RGBA16I + { GL_RGBA16UI, GL_ZERO }, // RGBA16U + { GL_RGBA16F, GL_ZERO }, // RGBA16F + { GL_RGBA16_SNORM, GL_ZERO }, // RGBA16S + { GL_RGBA32I, GL_ZERO }, // RGBA32I + { GL_RGBA32UI, GL_ZERO }, // RGBA32U + { GL_RGBA32F, GL_ZERO }, // RGBA32F + { GL_RGB565, GL_ZERO }, // B5G6R5 + { GL_RGB565, GL_ZERO }, // R5G6B5 + { GL_RGBA4, GL_ZERO }, // BGRA4 + { GL_RGBA4, GL_ZERO }, // RGBA4 + { GL_RGB5_A1, GL_ZERO }, // BGR5A1 + { GL_RGB5_A1, GL_ZERO }, // RGB5A1 + { GL_RGB10_A2, GL_ZERO }, // RGB10A2 + { GL_R11F_G11F_B10F, GL_ZERO }, // RG11B10F + { GL_ZERO, GL_ZERO }, // UnknownDepth + { GL_DEPTH_COMPONENT16, GL_ZERO }, // D16 + { GL_DEPTH_COMPONENT24, GL_ZERO }, // D24 + { GL_DEPTH24_STENCIL8, GL_ZERO }, // D24S8 + { GL_DEPTH_COMPONENT32, GL_ZERO }, // D32 + { GL_DEPTH_COMPONENT32F, GL_ZERO }, // D16F + { GL_DEPTH_COMPONENT32F, GL_ZERO }, // D24F + { GL_DEPTH_COMPONENT32F, GL_ZERO }, // D32F + { GL_STENCIL_INDEX8, GL_ZERO }, // D0S8 + }; + static_assert(bgfx::TextureFormat::Count == BX_COUNTOF(s_textureFormat)); + } + + class ExternalTexture::Impl final : public ImplBase + { + public: + // Implemented in ExternalTexture_Shared.h + Impl(Graphics::TextureT, std::optional); + void Update(Graphics::TextureT, std::optional, std::optional); + + Graphics::TextureT Get() const + { + return m_ptr; + } + + private: + static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, Info& info) + { + // NOTE: GL state queries below require the bgfx OpenGL context to be + // current on this thread. ExternalTexture is constructed/updated on + // the same scheduler that AddToContextAsync uses for bgfx work, so + // the context current-ness is the caller's responsibility. + + const GLuint texture = static_cast(ptr); + + // Reject obviously invalid handles up front. glIsTexture only + // returns true for handles that have already been used (bound at + // least once); any real FBO color attachment will satisfy this. + if (texture == 0 || glIsTexture(texture) == GL_FALSE) + { + throw std::runtime_error{"ExternalTexture: invalid OpenGL texture handle"}; + } + + // Save the current GL_TEXTURE_2D binding so we don't disturb caller state. + GLint previousBinding = 0; + glGetIntegerv(GL_TEXTURE_BINDING_2D, &previousBinding); + + // Drain any pre-existing GL error so we can detect bind failure below. + while (glGetError() != GL_NO_ERROR) { } + + glBindTexture(GL_TEXTURE_2D, texture); + // If the handle is bound to a non-GL_TEXTURE_2D target (e.g. + // GL_TEXTURE_2D_MULTISAMPLE, GL_TEXTURE_2D_ARRAY, cube map, external) + // the bind raises GL_INVALID_OPERATION and the GL_TEXTURE_2D binding + // is unchanged. Querying parameters in that state would silently read + // the previously bound texture. Detect and reject explicitly. + if (glGetError() != GL_NO_ERROR) + { + glBindTexture(GL_TEXTURE_2D, static_cast(previousBinding)); + throw std::runtime_error{"ExternalTexture: only GL_TEXTURE_2D handles are supported (multisample/array/cube/external rejected)"}; + } + + GLint width = 0; + GLint height = 0; + GLint internalFormat = 0; + glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_WIDTH, &width); + glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_HEIGHT, &height); + glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_INTERNAL_FORMAT, &internalFormat); + + if (width <= 0 || height <= 0) + { + glBindTexture(GL_TEXTURE_2D, static_cast(previousBinding)); + throw std::runtime_error{"ExternalTexture: failed to query OpenGL texture dimensions"}; + } + + // Bound the mip-chain walk by the maximum possible mip count for + // this texture's dimensions, so we never query a level that the + // implementation considers out of range (which can pollute GL error + // state). + const GLint maxDimension = std::max(width, height); + uint16_t maxPossibleMips = 1; + for (GLint d = maxDimension; d > 1; d >>= 1) + { + ++maxPossibleMips; + } + + uint16_t mipLevels = 1; + for (GLint level = 1; level < static_cast(maxPossibleMips); ++level) + { + GLint levelWidth = 0; + glGetTexLevelParameteriv(GL_TEXTURE_2D, level, GL_TEXTURE_WIDTH, &levelWidth); + if (levelWidth <= 0) + { + break; + } + ++mipLevels; + } + + info.Width = static_cast(width); + info.Height = static_cast(height); + info.MipLevels = mipLevels; + + // GL_TEXTURE_2D is a single-layer target; array textures are rejected + // above by the GL_TEXTURE_2D-only bind check. bgfx::createTexture2D + // (ImplBase) uses NumLayers directly, so it must be at least 1. + info.NumLayers = 1; + + // OpenGL has no portable way to ask a bare texture handle whether + // it is being used as a render-target attachment. ExternalTexture's + // sole supported use case is bridging an FBO color attachment back + // into bgfx, so we always advertise the texture as a render target. + // Multisample texture handling is omitted - bgfx::overrideInternal + // treats the binding as single-sample. + info.Flags |= BGFX_TEXTURE_RT; + + const GLenum targetFormat = overrideFormat.has_value() + ? static_cast(overrideFormat.value()) + : static_cast(internalFormat); + + bool formatFound = false; + for (size_t i = 0; i < BX_COUNTOF(s_textureFormat); ++i) + { + const auto bgfxFormat = static_cast(i); + + // GL has no distinct BGRA-ordered internal formats: bgfx maps each + // BGRA*/BGR* variant to the SAME GL internal format as its RGBA twin + // (BGRA8/RGBA8 -> GL_RGBA8, B5G6R5/R5G6B5 -> GL_RGB565, etc.). Because + // this is a reverse lookup that stops at the first match and the BGRA + // rows precede their RGBA twins, a GL_RGBA8 FBO attachment would + // otherwise resolve to BGRA8 and get its R/B channels swapped. Skip the + // BGRA-ordered rows so the RGBA (correct channel order) twin wins. + if (bgfxFormat == bgfx::TextureFormat::BGRA8 || + bgfxFormat == bgfx::TextureFormat::BGRA4 || + bgfxFormat == bgfx::TextureFormat::BGR5A1 || + bgfxFormat == bgfx::TextureFormat::B5G6R5) + { + continue; + } + + const auto& format = s_textureFormat[i]; + if (format.m_fmt == GL_ZERO && format.m_fmtSrgb == GL_ZERO) + { + continue; + } + + if (format.m_fmt == targetFormat || format.m_fmtSrgb == targetFormat) + { + info.Format = bgfxFormat; + if (format.m_fmtSrgb == targetFormat && format.m_fmtSrgb != GL_ZERO) + { + info.Flags |= BGFX_TEXTURE_SRGB; + } + formatFound = true; + break; + } + } + + glBindTexture(GL_TEXTURE_2D, static_cast(previousBinding)); + + if (!formatFound) + { + throw std::runtime_error{"ExternalTexture: unsupported OpenGL texture internal format"}; + } + } + + void Set(Graphics::TextureT ptr) + { + m_ptr = static_cast(ptr); + } + + GLuint m_ptr{}; + }; +} + +#include "ExternalTexture_Shared.h" From 3f95a3c5faf882ce028afacc3f8297c2a966fabc Mon Sep 17 00:00:00 2001 From: caschmidt Date: Thu, 16 Jul 2026 16:32:05 -0600 Subject: [PATCH 2/3] ExternalTexture_OpenGL: fix static_assert and add width/height hints 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 --- Apps/UnitTests/CMakeLists.txt | 16 +- .../Source/Tests.ExternalTexture.cpp | 12 +- .../Include/Babylon/Plugins/ExternalTexture.h | 12 +- .../Source/ExternalTexture_D3D11.cpp | 6 +- .../Source/ExternalTexture_D3D12.cpp | 6 +- .../Source/ExternalTexture_Metal.cpp | 6 +- .../Source/ExternalTexture_OpenGL.cpp | 271 +++++++++++------- .../Source/ExternalTexture_Shared.h | 16 +- 8 files changed, 215 insertions(+), 130 deletions(-) diff --git a/Apps/UnitTests/CMakeLists.txt b/Apps/UnitTests/CMakeLists.txt index c0b6e429f..99f206c6b 100644 --- a/Apps/UnitTests/CMakeLists.txt +++ b/Apps/UnitTests/CMakeLists.txt @@ -57,13 +57,6 @@ if(APPLE) set(ADDITIONAL_LIBRARIES PRIVATE ${JAVASCRIPTCORE_LIBRARY}) elseif(UNIX AND NOT ANDROID) set(SOURCES ${SOURCES} "Source/App.X11.cpp") - # The OpenGL ExternalTexture backend supports single-sample GL_TEXTURE_2D - # handles, so run the basic GetInfo/Set/Get/Update tests. The render-path - # tests (pixel readback) and the array/layer-index test require GL test - # helpers and array-texture support that this backend does not implement yet. - set(ADDITIONAL_COMPILE_DEFINITIONS - PRIVATE SKIP_RENDER_TESTS - PRIVATE SKIP_EXTERNAL_TEXTURE_ARRAY_TESTS) elseif(WIN32) set(SOURCES ${SOURCES} "Source/App.Win32.cpp" @@ -95,6 +88,15 @@ if(GRAPHICS_API STREQUAL "D3D12") target_compile_definitions(UnitTests PRIVATE SKIP_RENDER_TESTS) endif() +if(GRAPHICS_API STREQUAL "OpenGL") + # The OpenGL ExternalTexture backend imports single-sample GL_TEXTURE_2D + # handles only, so run the basic GetInfo/Set/Get/Update tests on any OpenGL + # build (Linux, or the OpenGLWindowsDevOnly dev path). The render-path tests + # (pixel readback) and the array/layer-index test require GL test helpers and + # array-texture support that this backend does not implement yet. + target_compile_definitions(UnitTests PRIVATE SKIP_RENDER_TESTS SKIP_EXTERNAL_TEXTURE_ARRAY_TESTS) +endif() + add_test(NAME UnitTests COMMAND UnitTests) # See https://gitlab.kitware.com/cmake/cmake/-/issues/23543 diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp index cc2f3a378..d6d256149 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp @@ -23,7 +23,7 @@ TEST(ExternalTexture, Construction) device.StartRenderingCurrentFrame(); auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); - Babylon::Plugins::ExternalTexture externalTexture{nativeTexture}; + Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; Helpers::DestroyTexture(nativeTexture); EXPECT_EQ(externalTexture.Width(), 256u); @@ -43,7 +43,7 @@ TEST(ExternalTexture, CreateForJavaScript) device.StartRenderingCurrentFrame(); auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); - Babylon::Plugins::ExternalTexture externalTexture{nativeTexture}; + Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; Helpers::DestroyTexture(nativeTexture); std::promise done{}; @@ -82,7 +82,7 @@ TEST(ExternalTexture, Update) device.StartRenderingCurrentFrame(); auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); - Babylon::Plugins::ExternalTexture externalTexture{nativeTexture}; + Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; Helpers::DestroyTexture(nativeTexture); EXPECT_EQ(externalTexture.Width(), 256u); @@ -94,7 +94,7 @@ TEST(ExternalTexture, Update) device.StartRenderingCurrentFrame(); auto nativeTexture2 = Helpers::CreateTexture(device.GetPlatformInfo().Device, 128, 128); - externalTexture.Update(nativeTexture2); + externalTexture.Update(nativeTexture2, {}, {}, 128, 128); Helpers::DestroyTexture(nativeTexture2); EXPECT_EQ(externalTexture.Width(), 128u); @@ -114,7 +114,7 @@ TEST(ExternalTexture, AddToContextAsyncAndUpdate) device.StartRenderingCurrentFrame(); auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); - Babylon::Plugins::ExternalTexture externalTexture{nativeTexture}; + Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; Helpers::DestroyTexture(nativeTexture); std::promise addToContext{}; @@ -160,7 +160,7 @@ TEST(ExternalTexture, AddToContextAsyncAndUpdate) // Update the external texture to a new texture. auto nativeTexture2 = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); - externalTexture.Update(nativeTexture2); + externalTexture.Update(nativeTexture2, {}, {}, 256, 256); Helpers::DestroyTexture(nativeTexture2); device.FinishRenderingCurrentFrame(); diff --git a/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h b/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h index 1291ee363..77fda3247 100644 --- a/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h +++ b/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h @@ -11,7 +11,13 @@ namespace Babylon::Plugins class ExternalTexture final { public: - ExternalTexture(Graphics::TextureT, std::optional = {}); + // width/height are required on the OpenGL/OpenGL ES backend and ignored on all + // others. BabylonNative's OpenGL renderer runs on an OpenGL ES 3.0 context, which + // provides no way to query a texture's dimensions from a bare handle + // (glGetTexLevelParameteriv was only added in ES 3.1). The caller that created the + // texture already knows its size, so it must supply it here. The D3D/Metal backends + // introspect the native texture and ignore these hints. + ExternalTexture(Graphics::TextureT, std::optional = {}, std::optional width = {}, std::optional height = {}); ~ExternalTexture(); // Copy semantics @@ -45,7 +51,9 @@ namespace Babylon::Plugins Napi::Promise AddToContextAsync(Napi::Env, std::optional layerIndex = {}) const; // Updates to a new texture. If layerIndex is set, views only that array layer (single-slice). - void Update(Graphics::TextureT, std::optional = {}, std::optional layerIndex = {}); + // width/height follow the same backend-specific contract as the constructor: required on + // OpenGL/OpenGL ES, ignored elsewhere. + void Update(Graphics::TextureT, std::optional = {}, std::optional layerIndex = {}, std::optional width = {}, std::optional height = {}); private: class Impl; diff --git a/Plugins/ExternalTexture/Source/ExternalTexture_D3D11.cpp b/Plugins/ExternalTexture/Source/ExternalTexture_D3D11.cpp index 333c012e4..bc2f5011d 100644 --- a/Plugins/ExternalTexture/Source/ExternalTexture_D3D11.cpp +++ b/Plugins/ExternalTexture/Source/ExternalTexture_D3D11.cpp @@ -162,8 +162,8 @@ namespace Babylon::Plugins { public: // Implemented in ExternalTexture_Shared.h - Impl(Graphics::TextureT, std::optional); - void Update(Graphics::TextureT, std::optional, std::optional); + Impl(Graphics::TextureT, std::optional, std::optional, std::optional); + void Update(Graphics::TextureT, std::optional, std::optional, std::optional, std::optional); Graphics::TextureT Get() const { @@ -171,7 +171,7 @@ namespace Babylon::Plugins } private: - static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, Info& info) + static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, std::optional /*width*/, std::optional /*height*/, Info& info) { winrt::com_ptr resource; resource.copy_from(ptr); diff --git a/Plugins/ExternalTexture/Source/ExternalTexture_D3D12.cpp b/Plugins/ExternalTexture/Source/ExternalTexture_D3D12.cpp index 0936118a6..72e57e2d6 100644 --- a/Plugins/ExternalTexture/Source/ExternalTexture_D3D12.cpp +++ b/Plugins/ExternalTexture/Source/ExternalTexture_D3D12.cpp @@ -162,8 +162,8 @@ namespace Babylon::Plugins { public: // Implemented in ExternalTexture_Shared.h - Impl(Graphics::TextureT, std::optional); - void Update(Graphics::TextureT, std::optional, std::optional); + Impl(Graphics::TextureT, std::optional, std::optional, std::optional); + void Update(Graphics::TextureT, std::optional, std::optional, std::optional, std::optional); Graphics::TextureT Get() const { @@ -171,7 +171,7 @@ namespace Babylon::Plugins } private: - static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, Info& info) + static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, std::optional /*width*/, std::optional /*height*/, Info& info) { D3D12_RESOURCE_DESC desc = ptr->GetDesc(); diff --git a/Plugins/ExternalTexture/Source/ExternalTexture_Metal.cpp b/Plugins/ExternalTexture/Source/ExternalTexture_Metal.cpp index e8a01e359..fd8cb8bf2 100644 --- a/Plugins/ExternalTexture/Source/ExternalTexture_Metal.cpp +++ b/Plugins/ExternalTexture/Source/ExternalTexture_Metal.cpp @@ -250,8 +250,8 @@ namespace Babylon::Plugins { public: // Implemented in ExternalTexture_Shared.h - Impl(Graphics::TextureT, std::optional); - void Update(Graphics::TextureT, std::optional, std::optional); + Impl(Graphics::TextureT, std::optional, std::optional, std::optional); + void Update(Graphics::TextureT, std::optional, std::optional, std::optional, std::optional); Graphics::TextureT Get() const { @@ -259,7 +259,7 @@ namespace Babylon::Plugins } private: - static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, Info& info) + static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, std::optional /*width*/, std::optional /*height*/, Info& info) { if (ptr->textureType() != MTL::TextureType2D && ptr->textureType() != MTL::TextureType2DMultisample) { diff --git a/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp b/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp index 46ed06291..c20007c5b 100644 --- a/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp +++ b/Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp @@ -9,24 +9,11 @@ #include -// glGetTexLevelParameteriv and the GL_TEXTURE_WIDTH/HEIGHT query tokens require -// OpenGL ES 3.1+. Both Linux (Mesa) and Android NDK (API 21+) ship the gl31.h -// header, so prefer it. Some minimal GLES toolchains only ship gl3.h; rather -// than fail the build there, fall back to gl3.h and supply the handful of 3.1 -// entry points/tokens we use. They resolve at runtime in the GLES 3.1+/desktop -// GL contexts bgfx creates for this backend. -#if !defined(__has_include) || __has_include() -# include -#else -# include -# ifndef GL_TEXTURE_WIDTH -# define GL_TEXTURE_WIDTH 0x1000 -# endif -# ifndef GL_TEXTURE_HEIGHT -# define GL_TEXTURE_HEIGHT 0x1001 -# endif -extern "C" GL_APICALL void GL_APIENTRY glGetTexLevelParameteriv(GLenum target, GLint level, GLenum pname, GLint* params); -#endif +// This backend runs on BabylonNative's OpenGL ES 3.0 context, so it uses only +// ES 3.0 entry points. Texture dimensions cannot be queried from a bare handle +// on ES 3.0 (glGetTexLevelParameteriv is ES 3.1), so the caller supplies them; +// the format is recovered via ES 3.0 framebuffer-attachment queries. +#include #include "ExternalTexture_Base.h" @@ -79,6 +66,18 @@ extern "C" GL_APICALL void GL_APIENTRY glGetTexLevelParameteriv(GLenum target, G #ifndef GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC # define GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC 0x9279 #endif +#ifndef GL_COMPRESSED_R11_EAC +# define GL_COMPRESSED_R11_EAC 0x9270 +#endif +#ifndef GL_COMPRESSED_SIGNED_R11_EAC +# define GL_COMPRESSED_SIGNED_R11_EAC 0x9271 +#endif +#ifndef GL_COMPRESSED_RG11_EAC +# define GL_COMPRESSED_RG11_EAC 0x9272 +#endif +#ifndef GL_COMPRESSED_SIGNED_RG11_EAC +# define GL_COMPRESSED_SIGNED_RG11_EAC 0x9273 +#endif #ifndef GL_COMPRESSED_RGB_PVRTC_2BPPV1_IMG # define GL_COMPRESSED_RGB_PVRTC_2BPPV1_IMG 0x8C01 #endif @@ -259,6 +258,10 @@ namespace Babylon::Plugins { GL_COMPRESSED_RGB8_ETC2, GL_COMPRESSED_SRGB8_ETC2 }, // ETC2 { GL_COMPRESSED_RGBA8_ETC2_EAC, GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC }, // ETC2A { GL_COMPRESSED_RGB8_PUNCHTHROUGH_ALPHA1_ETC2, GL_COMPRESSED_SRGB8_PUNCHTHROUGH_ALPHA1_ETC2 }, // ETC2A1 + { GL_COMPRESSED_R11_EAC, GL_ZERO }, // EACR11 + { GL_COMPRESSED_SIGNED_R11_EAC, GL_ZERO }, // EACR11S + { GL_COMPRESSED_RG11_EAC, GL_ZERO }, // EACRG11 + { GL_COMPRESSED_SIGNED_RG11_EAC, GL_ZERO }, // EACRG11S { GL_COMPRESSED_RGB_PVRTC_2BPPV1_IMG, GL_COMPRESSED_SRGB_PVRTC_2BPPV1_EXT }, // PTC12 { GL_COMPRESSED_RGB_PVRTC_4BPPV1_IMG, GL_COMPRESSED_SRGB_PVRTC_4BPPV1_EXT }, // PTC14 { GL_COMPRESSED_RGBA_PVRTC_2BPPV1_IMG, GL_COMPRESSED_SRGB_ALPHA_PVRTC_2BPPV1_EXT }, // PTC12A @@ -346,14 +349,92 @@ namespace Babylon::Plugins { GL_STENCIL_INDEX8, GL_ZERO }, // D0S8 }; static_assert(bgfx::TextureFormat::Count == BX_COUNTOF(s_textureFormat)); + + // Recover the bgfx color format of a GL_TEXTURE_2D handle using only OpenGL + // ES 3.0 entry points. glGetTexLevelParameteriv (which would report the + // internal format directly) is ES 3.1, so instead the texture is attached to + // a scratch framebuffer and its per-channel bit depths, component type and + // color encoding are read back and matched to a bgfx format. Returns + // bgfx::TextureFormat::Unknown when the texture is not one of the + // color-renderable formats handled here (callers can pass an explicit + // overrideFormat in that case). Restores the previous framebuffer binding. + bgfx::TextureFormat::Enum DeriveBgfxColorFormatFromTexture(GLuint texture, bool& isSrgb) + { + isSrgb = false; + + GLint previousFbo = 0; + glGetIntegerv(GL_FRAMEBUFFER_BINDING, &previousFbo); + + GLuint fbo = 0; + glGenFramebuffers(1, &fbo); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0); + + const auto attachmentParam = [](GLenum pname) { + GLint value = 0; + glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, pname, &value); + return value; + }; + + bgfx::TextureFormat::Enum format = bgfx::TextureFormat::Unknown; + + if (glCheckFramebufferStatus(GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE && + attachmentParam(GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) == GL_TEXTURE) + { + const GLint r = attachmentParam(GL_FRAMEBUFFER_ATTACHMENT_RED_SIZE); + const GLint g = attachmentParam(GL_FRAMEBUFFER_ATTACHMENT_GREEN_SIZE); + const GLint b = attachmentParam(GL_FRAMEBUFFER_ATTACHMENT_BLUE_SIZE); + const GLint a = attachmentParam(GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE); + const GLint type = attachmentParam(GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE); + isSrgb = attachmentParam(GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) == GL_SRGB; + + const auto channels = [&](GLint rr, GLint gg, GLint bb, GLint aa) { + return r == rr && g == gg && b == bb && a == aa; + }; + + if (type == GL_FLOAT) + { + if (channels(32, 32, 32, 32)) format = bgfx::TextureFormat::RGBA32F; + else if (channels(16, 16, 16, 16)) format = bgfx::TextureFormat::RGBA16F; + else if (channels(32, 32, 0, 0)) format = bgfx::TextureFormat::RG32F; + else if (channels(16, 16, 0, 0)) format = bgfx::TextureFormat::RG16F; + else if (channels(32, 0, 0, 0)) format = bgfx::TextureFormat::R32F; + else if (channels(16, 0, 0, 0)) format = bgfx::TextureFormat::R16F; + else if (channels(11, 11, 10, 0)) format = bgfx::TextureFormat::RG11B10F; + } + else if (type == GL_UNSIGNED_NORMALIZED) + { + if (channels(8, 8, 8, 8)) format = bgfx::TextureFormat::RGBA8; + else if (channels(16, 16, 16, 16)) format = bgfx::TextureFormat::RGBA16; + else if (channels(10, 10, 10, 2)) format = bgfx::TextureFormat::RGB10A2; + else if (channels(5, 5, 5, 1)) format = bgfx::TextureFormat::RGB5A1; + else if (channels(4, 4, 4, 4)) format = bgfx::TextureFormat::RGBA4; + else if (channels(5, 6, 5, 0)) format = bgfx::TextureFormat::R5G6B5; + else if (channels(8, 8, 8, 0)) format = bgfx::TextureFormat::RGB8; + else if (channels(8, 8, 0, 0)) format = bgfx::TextureFormat::RG8; + else if (channels(8, 0, 0, 0)) format = bgfx::TextureFormat::R8; + } + } + + // sRGB encoding only distinguishes the 8-bit unorm color formats in bgfx. + if (format != bgfx::TextureFormat::RGBA8 && format != bgfx::TextureFormat::RGB8) + { + isSrgb = false; + } + + glBindFramebuffer(GL_FRAMEBUFFER, static_cast(previousFbo)); + glDeleteFramebuffers(1, &fbo); + + return format; + } } class ExternalTexture::Impl final : public ImplBase { public: // Implemented in ExternalTexture_Shared.h - Impl(Graphics::TextureT, std::optional); - void Update(Graphics::TextureT, std::optional, std::optional); + Impl(Graphics::TextureT, std::optional, std::optional, std::optional); + void Update(Graphics::TextureT, std::optional, std::optional, std::optional, std::optional); Graphics::TextureT Get() const { @@ -361,7 +442,7 @@ namespace Babylon::Plugins } private: - static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, Info& info) + static void GetInfo(Graphics::TextureT ptr, std::optional overrideFormat, std::optional width, std::optional height, Info& info) { // NOTE: GL state queries below require the bgfx OpenGL context to be // current on this thread. ExternalTexture is constructed/updated on @@ -397,106 +478,100 @@ namespace Babylon::Plugins throw std::runtime_error{"ExternalTexture: only GL_TEXTURE_2D handles are supported (multisample/array/cube/external rejected)"}; } - GLint width = 0; - GLint height = 0; - GLint internalFormat = 0; - glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_WIDTH, &width); - glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_HEIGHT, &height); - glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_INTERNAL_FORMAT, &internalFormat); - - if (width <= 0 || height <= 0) + // OpenGL ES 3.0 provides no way to query a texture's dimensions from a + // bare handle (glGetTexLevelParameteriv is ES 3.1), so the caller must + // supply them. Reject missing/degenerate dimensions explicitly. + if (!width.has_value() || !height.has_value() || width.value() == 0 || height.value() == 0) { glBindTexture(GL_TEXTURE_2D, static_cast(previousBinding)); - throw std::runtime_error{"ExternalTexture: failed to query OpenGL texture dimensions"}; + throw std::runtime_error{"ExternalTexture: width and height must be supplied for the OpenGL backend"}; } - // Bound the mip-chain walk by the maximum possible mip count for - // this texture's dimensions, so we never query a level that the - // implementation considers out of range (which can pollute GL error - // state). - const GLint maxDimension = std::max(width, height); - uint16_t maxPossibleMips = 1; - for (GLint d = maxDimension; d > 1; d >>= 1) - { - ++maxPossibleMips; - } + // Restore the caller's GL_TEXTURE_2D binding; the format is recovered + // below via framebuffer-attachment queries, which take the texture id + // directly and do not need it bound. + glBindTexture(GL_TEXTURE_2D, static_cast(previousBinding)); - uint16_t mipLevels = 1; - for (GLint level = 1; level < static_cast(maxPossibleMips); ++level) - { - GLint levelWidth = 0; - glGetTexLevelParameteriv(GL_TEXTURE_2D, level, GL_TEXTURE_WIDTH, &levelWidth); - if (levelWidth <= 0) - { - break; - } - ++mipLevels; - } + info.Width = static_cast(width.value()); + info.Height = static_cast(height.value()); - info.Width = static_cast(width); - info.Height = static_cast(height); - info.MipLevels = mipLevels; + // ES 3.0 cannot introspect the mip chain from a handle; external + // textures bridged into bgfx are single-level render targets. + info.MipLevels = 1; // GL_TEXTURE_2D is a single-layer target; array textures are rejected // above by the GL_TEXTURE_2D-only bind check. bgfx::createTexture2D // (ImplBase) uses NumLayers directly, so it must be at least 1. info.NumLayers = 1; - // OpenGL has no portable way to ask a bare texture handle whether - // it is being used as a render-target attachment. ExternalTexture's - // sole supported use case is bridging an FBO color attachment back - // into bgfx, so we always advertise the texture as a render target. - // Multisample texture handling is omitted - bgfx::overrideInternal - // treats the binding as single-sample. + // OpenGL has no portable way to ask a bare texture handle whether it is + // being used as a render-target attachment. ExternalTexture's sole + // supported use case is bridging an FBO color attachment back into bgfx, + // so we always advertise the texture as a render target. info.Flags |= BGFX_TEXTURE_RT; - const GLenum targetFormat = overrideFormat.has_value() - ? static_cast(overrideFormat.value()) - : static_cast(internalFormat); - - bool formatFound = false; - for (size_t i = 0; i < BX_COUNTOF(s_textureFormat); ++i) + if (overrideFormat.has_value()) { - const auto bgfxFormat = static_cast(i); - - // GL has no distinct BGRA-ordered internal formats: bgfx maps each - // BGRA*/BGR* variant to the SAME GL internal format as its RGBA twin - // (BGRA8/RGBA8 -> GL_RGBA8, B5G6R5/R5G6B5 -> GL_RGB565, etc.). Because - // this is a reverse lookup that stops at the first match and the BGRA - // rows precede their RGBA twins, a GL_RGBA8 FBO attachment would - // otherwise resolve to BGRA8 and get its R/B channels swapped. Skip the - // BGRA-ordered rows so the RGBA (correct channel order) twin wins. - if (bgfxFormat == bgfx::TextureFormat::BGRA8 || - bgfxFormat == bgfx::TextureFormat::BGRA4 || - bgfxFormat == bgfx::TextureFormat::BGR5A1 || - bgfxFormat == bgfx::TextureFormat::B5G6R5) - { - continue; - } + // The caller supplied a GL internal format; reverse-map it to a + // bgfx format via the same table the D3D/Metal backends use. + const GLenum targetFormat = static_cast(overrideFormat.value()); - const auto& format = s_textureFormat[i]; - if (format.m_fmt == GL_ZERO && format.m_fmtSrgb == GL_ZERO) + bool formatFound = false; + for (size_t i = 0; i < BX_COUNTOF(s_textureFormat); ++i) { - continue; - } + const auto bgfxFormat = static_cast(i); - if (format.m_fmt == targetFormat || format.m_fmtSrgb == targetFormat) - { - info.Format = bgfxFormat; - if (format.m_fmtSrgb == targetFormat && format.m_fmtSrgb != GL_ZERO) + // GL has no distinct BGRA-ordered internal formats: bgfx maps each + // BGRA*/BGR* variant to the SAME GL internal format as its RGBA twin + // (BGRA8/RGBA8 -> GL_RGBA8, B5G6R5/R5G6B5 -> GL_RGB565, etc.). Because + // this reverse lookup stops at the first match and the BGRA rows + // precede their RGBA twins, skip the BGRA-ordered rows so the RGBA + // (correct channel order) twin wins. + if (bgfxFormat == bgfx::TextureFormat::BGRA8 || + bgfxFormat == bgfx::TextureFormat::BGRA4 || + bgfxFormat == bgfx::TextureFormat::BGR5A1 || + bgfxFormat == bgfx::TextureFormat::B5G6R5) { - info.Flags |= BGFX_TEXTURE_SRGB; + continue; } - formatFound = true; - break; - } - } - glBindTexture(GL_TEXTURE_2D, static_cast(previousBinding)); + const auto& format = s_textureFormat[i]; + if (format.m_fmt == GL_ZERO && format.m_fmtSrgb == GL_ZERO) + { + continue; + } - if (!formatFound) + if (format.m_fmt == targetFormat || format.m_fmtSrgb == targetFormat) + { + info.Format = bgfxFormat; + if (format.m_fmtSrgb == targetFormat && format.m_fmtSrgb != GL_ZERO) + { + info.Flags |= BGFX_TEXTURE_SRGB; + } + formatFound = true; + break; + } + } + + if (!formatFound) + { + throw std::runtime_error{"ExternalTexture: unsupported OpenGL texture internal format override"}; + } + } + else { - throw std::runtime_error{"ExternalTexture: unsupported OpenGL texture internal format"}; + // No override: recover the format directly from the live texture + // using ES 3.0 framebuffer-attachment queries. + bool isSrgb = false; + info.Format = DeriveBgfxColorFormatFromTexture(texture, isSrgb); + if (info.Format == bgfx::TextureFormat::Unknown) + { + throw std::runtime_error{"ExternalTexture: could not determine the OpenGL texture format; pass an explicit format override"}; + } + if (isSrgb) + { + info.Flags |= BGFX_TEXTURE_SRGB; + } } } diff --git a/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h b/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h index 637f95cd8..e375ed633 100644 --- a/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h +++ b/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h @@ -2,9 +2,9 @@ namespace Babylon::Plugins { - ExternalTexture::Impl::Impl(Graphics::TextureT ptr, std::optional overrideFormat) + ExternalTexture::Impl::Impl(Graphics::TextureT ptr, std::optional overrideFormat, std::optional width, std::optional height) { - GetInfo(ptr, overrideFormat, m_info); + GetInfo(ptr, overrideFormat, width, height, m_info); if (m_info.MipLevels != 1 && m_info.MipLevels != 0 && !IsFullMipChain(m_info.MipLevels, m_info.Width, m_info.Height)) { @@ -14,10 +14,10 @@ namespace Babylon::Plugins Set(ptr); } - void ExternalTexture::Impl::Update(Graphics::TextureT ptr, std::optional overrideFormat, std::optional layerIndex) + void ExternalTexture::Impl::Update(Graphics::TextureT ptr, std::optional overrideFormat, std::optional layerIndex, std::optional width, std::optional height) { Info info; - GetInfo(ptr, overrideFormat, info); + GetInfo(ptr, overrideFormat, width, height, info); DEBUG_TRACE("ExternalTexture [0x%p] Update %d x %d %d mips %d layers", this, int(info.Width), int(info.Height), int(info.MipLevels), int(info.NumLayers)); @@ -78,8 +78,8 @@ namespace Babylon::Plugins delete texture; } - ExternalTexture::ExternalTexture(Graphics::TextureT ptr, std::optional overrideFormat) - : m_impl{std::make_shared(ptr, overrideFormat)} + ExternalTexture::ExternalTexture(Graphics::TextureT ptr, std::optional overrideFormat, std::optional width, std::optional height) + : m_impl{std::make_shared(ptr, overrideFormat, width, height)} { } @@ -131,9 +131,9 @@ namespace Babylon::Plugins }); } - void ExternalTexture::Update(Graphics::TextureT ptr, std::optional overrideFormat, std::optional layerIndex) + void ExternalTexture::Update(Graphics::TextureT ptr, std::optional overrideFormat, std::optional layerIndex, std::optional width, std::optional height) { - m_impl->Update(ptr, overrideFormat, layerIndex); + m_impl->Update(ptr, overrideFormat, layerIndex, width, height); } Napi::Promise ExternalTexture::AddToContextAsync(Napi::Env env, std::optional layerIndex) const From 3219da489a7508e1cce0980229783753050d257d Mon Sep 17 00:00:00 2001 From: caschmidt Date: Tue, 21 Jul 2026 17:32:20 -0600 Subject: [PATCH 3/3] ExternalTexture: keep source GL texture alive until bgfx is done with it 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 --- .../Source/Tests.ExternalTexture.cpp | 30 ++++++++++++++----- .../Include/Babylon/Plugins/ExternalTexture.h | 8 +++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp index d6d256149..97d54d735 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp @@ -24,12 +24,15 @@ TEST(ExternalTexture, Construction) auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; - Helpers::DestroyTexture(nativeTexture); EXPECT_EQ(externalTexture.Width(), 256u); EXPECT_EQ(externalTexture.Height(), 256u); device.FinishRenderingCurrentFrame(); + + // The OpenGL backend does not take ownership of the handle, so the source texture must + // outlive the ExternalTexture (see ExternalTexture.h). Destroy it only now. + Helpers::DestroyTexture(nativeTexture); #endif } @@ -44,7 +47,6 @@ TEST(ExternalTexture, CreateForJavaScript) auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; - Helpers::DestroyTexture(nativeTexture); std::promise done{}; @@ -69,6 +71,10 @@ TEST(ExternalTexture, CreateForJavaScript) done.get_future().wait(); device.FinishRenderingCurrentFrame(); + + // The GL backend doesn't own the handle; destroy the source texture only after bgfx has + // finished using it (see ExternalTexture.h). + Helpers::DestroyTexture(nativeTexture); #endif } @@ -83,7 +89,6 @@ TEST(ExternalTexture, Update) auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; - Helpers::DestroyTexture(nativeTexture); EXPECT_EQ(externalTexture.Width(), 256u); EXPECT_EQ(externalTexture.Height(), 256u); @@ -95,12 +100,16 @@ TEST(ExternalTexture, Update) auto nativeTexture2 = Helpers::CreateTexture(device.GetPlatformInfo().Device, 128, 128); externalTexture.Update(nativeTexture2, {}, {}, 128, 128); - Helpers::DestroyTexture(nativeTexture2); EXPECT_EQ(externalTexture.Width(), 128u); EXPECT_EQ(externalTexture.Height(), 128u); device.FinishRenderingCurrentFrame(); + + // The GL backend doesn't own these handles; destroy them only after all rendering that + // referenced them has completed (see ExternalTexture.h). + Helpers::DestroyTexture(nativeTexture); + Helpers::DestroyTexture(nativeTexture2); #endif } @@ -115,7 +124,6 @@ TEST(ExternalTexture, AddToContextAsyncAndUpdate) auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); Babylon::Plugins::ExternalTexture externalTexture{nativeTexture, {}, 256, 256}; - Helpers::DestroyTexture(nativeTexture); std::promise addToContext{}; std::promise promiseResolved{}; @@ -161,9 +169,13 @@ TEST(ExternalTexture, AddToContextAsyncAndUpdate) // Update the external texture to a new texture. auto nativeTexture2 = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256); externalTexture.Update(nativeTexture2, {}, {}, 256, 256); - Helpers::DestroyTexture(nativeTexture2); device.FinishRenderingCurrentFrame(); + + // The GL backend doesn't own these handles; destroy them only after all frames that + // referenced them have completed (see ExternalTexture.h). + Helpers::DestroyTexture(nativeTexture); + Helpers::DestroyTexture(nativeTexture2); #endif } @@ -179,7 +191,6 @@ TEST(ExternalTexture, AddToContextAsyncWithLayerIndex) // Array texture (3 layers) so a non-zero layer index is valid. auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256, 3); Babylon::Plugins::ExternalTexture externalTexture{nativeTexture}; - Helpers::DestroyTexture(nativeTexture); std::promise addToContext{}; std::promise promiseResolved{}; @@ -220,5 +231,10 @@ TEST(ExternalTexture, AddToContextAsyncWithLayerIndex) // get() (not wait()) so a rejected promise rethrows and fails the test. EXPECT_NO_THROW(promiseResolved.get_future().get()); + + // The GL backend doesn't own the handle; destroy the source texture only after all + // rendering that referenced it has completed (see ExternalTexture.h). Harmless on + // D3D/Metal, which retain the native texture. + Helpers::DestroyTexture(nativeTexture); #endif } diff --git a/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h b/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h index 77fda3247..f0eb38c00 100644 --- a/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h +++ b/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h @@ -8,6 +8,14 @@ namespace Babylon::Plugins { // All operations of this class must be called from the graphics thread unless otherwise noted. + // + // Ownership: on the OpenGL/OpenGL ES backend, ExternalTexture does NOT take ownership of the + // texture handle -- OpenGL has no way to reference-count a texture name -- so the caller must + // keep the source texture alive for as long as this ExternalTexture (and any JavaScript texture + // created from it) is in use. Deleting the handle earlier leaves a dangling GL name that bgfx + // later dereferences (e.g. glObjectLabel under a debug build), which is a fatal error on strict + // drivers. The D3D11/D3D12/Metal backends retain the native texture, so on those backends the + // caller may release its own reference immediately after construction. class ExternalTexture final { public: