From 50815a232ebb969996e5434c5955d7c5c9e7b8b7 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 5 Sep 2026 13:44:50 -0400 Subject: [PATCH] [Fix] (Backend, Getter): retire the two frontend queries that were never asked and strip the unreachable frontend arms from the third MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GLFunctionsTable had three entries that are frontend queries wearing a backend interface (plan B v2 §2.1(a)): GetIntegeri_v, GetInteger64i_v and GetProgramiv. Two of them have NO caller at all - `grep -o 'gBackendFunctionsTable\.GL\.[A-Za-z_0-9]*'` outside MG_Backend/ lists 69 distinct entries and neither GetInteger64i_v nor GetProgramiv is among them - so both table slots, both backends' implementations and both registrations are deleted here. This is plan B §11 P0's first "strictly no-op free win". - Nothing was moved into MG_Impl, because MG_Impl already answers all of it. glGetInteger64i_v is served by GL_Getter.cpp:1240-1314, which handles the indexed buffer queries itself and derives every other pname from its own GetIntegeri_v ("Handing the leftovers straight to the backend instead made glGetInteger64i_v disagree with glGetIntegeri_v on the very same pname"). glGetProgramiv is served by GL_Program.cpp, whose GL_COMPUTE_WORK_GROUP_SIZE arm (:928-946) reads ProgramObject::GetComputeLocalSize - a link artifact of the program the APPLICATION wrote, which is the only program in the application's namespace. §4.7.1 class B. - GetIntegeri_v stays, but only for what a backend genuinely owns. Its pure frontend arms were unreachable: GL_Getter::GetIntegeri_v answers GL_SHADER_STORAGE_BUFFER_{BINDING,START,SIZE} through TryDecodeIndexedBufferQuery (:991-1029) and the six GL_IMAGE_BINDING_* pnames at :1115-1153, and returns before touching the table. That left 9 dead cases in DirectGLES.cpp and 9 in DirectVulkan.cpp - the plan's "15" undercounts the two files separately. What still arrives is GL_MAX_COMPUTE_WORK_GROUP_COUNT / _SIZE (GL_Getter.cpp:1161-1177, and MG_Util/ShaderTranspiler/CompileEnv.cpp :134-138 asks the table directly), so DirectVulkan keeps exactly those two and DirectGLES becomes a plain driver passthrough. - The dead arms were also WRONG, which is why deleting rather than reconciling them is the strict no-op: they clamped a bound range's size to the buffer's current storage, while the frontend reports the size glBindBufferRange was asked for verbatim (GL 4.6 core tables 23.4/23.5 - the clamp answered 0 for KHR-GL43.shader_storage_buffer_object.basic-binding's shape). Had a later refactor made the table the answer, the regression would have been silent. - ProgramResourceCache::computeWorkGroupSize and the spirv-reflect entry-point loop that filled it go with DirectVulkan's GetProgramiv; nothing else read it. - Three cases added to AdvertisedLimitsScenario pin what the frontend answers, on both lanes: the indexed SSBO binding/start/size on the 32- and 64-bit widths INCLUDING a shrink of the store underneath the binding (the arm that actually separates verbatim from clamped), the six image-unit pnames on both widths, and the compute local size plus the INVALID_OPERATION a program with no compute stage must give. - Both new gates were shown to go red for their reason: making GL_COMPUTE_WORK_GROUP_SIZE answer a defaulted (1,1,1) fails ComputeLocalSizeComesFromTheLinkedProgram on both lanes, and re-introducing the deleted store clamp in GL_Getter fails IndexedBufferBindingsAreReportedVerbatimOnBothWidths on both lanes. - Tested: cmake --build build-linux -j 24 (clean); ctest -L unit -j 12 -> 1382/1382 passed; ctest -R AdvertisedLimits -> 18/18 passed (6 pre-existing + 3 new, x DirectGLES and DirectVulkan). --- MobileGL/MG_Backend/BackendObject.h | 18 +- .../DirectGLES/BackendObject_DirectGLES.cpp | 2 - MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 142 ++----------- MobileGL/MG_Backend/DirectGLES/DirectGLES.h | 2 - .../BackendObject_DirectVulkan.cpp | 2 - .../MG_Backend/DirectVulkan/DirectVulkan.cpp | 130 ++---------- .../MG_Backend/DirectVulkan/DirectVulkan.h | 2 - .../Scenarios/AdvertisedLimitsScenario.cpp | 186 ++++++++++++++++++ 8 files changed, 228 insertions(+), 256 deletions(-) diff --git a/MobileGL/MG_Backend/BackendObject.h b/MobileGL/MG_Backend/BackendObject.h index d528d820..6b2c0231 100644 --- a/MobileGL/MG_Backend/BackendObject.h +++ b/MobileGL/MG_Backend/BackendObject.h @@ -192,9 +192,23 @@ namespace MobileGL { void (*MemoryBarrierByRegion)(GLbitfield barriers); void (*BindImageTexture)(GLuint unit, GLuint texture, GLint level, GLboolean layered, GLint layer, GLenum access, GLenum format); + // The ONLY indexed query that is genuinely a backend one, and only for the pnames + // MG_Impl/GLImpl/Getter/GL_Getter.cpp does not already own. Every indexed pname that + // names FRONTEND state - the indexed buffer bindings, the per-unit texture/sampler + // bindings, the image-unit bindings, the viewport rectangles, the indexed capabilities + // - is answered in GL_Getter::GetIntegeri_v and never reaches this entry; the + // 64-bit and float/double widths are derived there from the same answer, which is why + // no GetInteger64i_v/GetFloati_v/GetDoublei_v table entry exists. In practice this + // leaves GL_MAX_COMPUTE_WORK_GROUP_COUNT / _SIZE (also asked directly by + // MG_Util/ShaderTranspiler/CompileEnv.cpp) plus whatever pname the frontend has no + // case for at all. void (*GetIntegeri_v)(GLenum target, GLuint index, GLint* data); - void (*GetInteger64i_v)(GLenum target, GLuint index, GLint64* data); - void (*GetProgramiv)(GLuint program, GLenum pname, GLint* params); + // There is deliberately NO GetProgramiv entry: glGetProgramiv describes the program + // the APPLICATION wrote - link status, the transform-feedback mode, the compute local + // size - all of which are frontend link artifacts on ProgramObject, and + // MG_Impl/GLImpl/Program/GL_Program.cpp answers every one of them from there. Asking a + // backend would mean asking about a DIFFERENT program (a SPIRV-Cross-generated ESSL + // one, or a SPIR-V module), in a namespace the application never sees. // The GL program interface (glGetProgramInterfaceiv / glGetProgramResource*) is NOT // a backend query: it describes the program the application wrote, in the // application's namespace, which neither backend program is in. It is answered diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index c9905508..c8d7c5a1 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -1255,8 +1255,6 @@ namespace MobileGL::MG_Backend::DirectGLES { funcsTable.GL.MemoryBarrierByRegion = MemoryBarrierByRegion; funcsTable.GL.BindImageTexture = BindImageTexture; funcsTable.GL.GetIntegeri_v = GetIntegeri_v; - funcsTable.GL.GetInteger64i_v = GetInteger64i_v; - funcsTable.GL.GetProgramiv = GetProgramiv; funcsTable.GL.ShaderStorageBlockBinding = ShaderStorageBlockBinding; funcsTable.GL.Clear = Clear; funcsTable.GL.ClearBufferfi = ClearBufferfi; diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 6c26f1f6..85a10e00 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -7317,138 +7317,22 @@ namespace MobileGL::MG_Backend::DirectGLES { TextureImpl::SyncImageTextureBinding(unit); } + // Only the pnames MG_Impl/GLImpl/Getter/GL_Getter.cpp has no case for reach here. Every + // indexed pname naming FRONTEND state - the indexed buffer bindings, the per-unit + // texture/sampler bindings, the image-unit bindings, the viewport rectangles, the indexed + // capabilities - is answered there and returns before the table is consulted, so the arms + // this function used to carry for GL_SHADER_STORAGE_BUFFER_* and GL_IMAGE_BINDING_* were + // unreachable duplicates of the frontend's, and they did not even agree with it (the + // frontend reports the range glBindBufferRange was ASKED for, verbatim and unclamped; these + // clamped it to the buffer's current storage). In practice what arrives is + // GL_MAX_COMPUTE_WORK_GROUP_COUNT / _SIZE, which the driver owns. void GetIntegeri_v(GLenum target, GLuint index, GLint* data) { if (!data) return; - - switch (target) { - case GL_SHADER_STORAGE_BUFFER_BINDING: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - auto& obj = point.GetBoundObject(); - *data = obj ? static_cast(obj->GetExternalIndex()) : 0; - return; + if (g_GLESFuncs.glGetIntegeri_v) { + g_GLESFuncs.glGetIntegeri_v(target, index, data); + } else { + *data = 0; } - case GL_SHADER_STORAGE_BUFFER_START: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - *data = static_cast(point.GetRange().start); - return; - } - case GL_SHADER_STORAGE_BUFFER_SIZE: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - auto& obj = point.GetBoundObject(); - if (!obj) { - *data = 0; - return; - } - const auto& range = point.GetRange(); - const auto start = std::min(range.start, obj->GetSize()); - const auto end = std::min(range.end, obj->GetSize()); - *data = static_cast(end - start); - return; - } - case GL_IMAGE_BINDING_NAME: { - if (index >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) { - *data = 0; - return; - } - auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(index)); - *data = imageBinding.Texture ? static_cast(imageBinding.Texture->GetExternalIndex()) : 0; - return; - } - case GL_IMAGE_BINDING_LEVEL: { - if (index >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) { - *data = 0; - return; - } - auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(index)); - *data = imageBinding.Level; - return; - } - case GL_IMAGE_BINDING_LAYERED: { - if (index >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) { - *data = 0; - return; - } - auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(index)); - *data = imageBinding.Layered; - return; - } - case GL_IMAGE_BINDING_LAYER: { - if (index >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) { - *data = 0; - return; - } - auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(index)); - *data = imageBinding.Layer; - return; - } - case GL_IMAGE_BINDING_ACCESS: { - if (index >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) { - *data = 0; - return; - } - auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(index)); - *data = static_cast(imageBinding.Access); - return; - } - case GL_IMAGE_BINDING_FORMAT: { - if (index >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) { - *data = 0; - return; - } - auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(index)); - *data = static_cast(imageBinding.Format); - return; - } - default: - if (g_GLESFuncs.glGetIntegeri_v) { - g_GLESFuncs.glGetIntegeri_v(target, index, data); - } else { - *data = 0; - } - return; - } - } - - void GetInteger64i_v(GLenum target, GLuint index, GLint64* data) { - if (!data) return; - - switch (target) { - case GL_SHADER_STORAGE_BUFFER_START: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - *data = static_cast(point.GetRange().start); - return; - } - case GL_SHADER_STORAGE_BUFFER_SIZE: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - auto& obj = point.GetBoundObject(); - if (!obj) { - *data = 0; - return; - } - const auto& range = point.GetRange(); - const auto start = std::min(range.start, obj->GetSize()); - const auto end = std::min(range.end, obj->GetSize()); - *data = static_cast(end - start); - return; - } - default: - if (g_GLESFuncs.glGetInteger64i_v) { - g_GLESFuncs.glGetInteger64i_v(target, index, data); - } else { - *data = 0; - } - return; - } - } - - void GetProgramiv(GLuint program, GLenum pname, GLint* params) { - if (!params) return; - GLuint backendProgramId = GetBackendProgramId(program); - if (!backendProgramId) { - params[0] = 0; - return; - } - g_GLESFuncs.glGetProgramiv(backendProgramId, pname, params); } // NOTE the shape here, and do not "simplify" it back to GetBackendProgramId(): this entry diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h index 94947965..63a48bc4 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h @@ -92,8 +92,6 @@ namespace MobileGL::MG_Backend::DirectGLES { void BindImageTexture(GLuint unit, GLuint texture, GLint level, GLboolean layered, GLint layer, GLenum access, GLenum format); void GetIntegeri_v(GLenum target, GLuint index, GLint* data); - void GetInteger64i_v(GLenum target, GLuint index, GLint64* data); - void GetProgramiv(GLuint program, GLenum pname, GLint* params); void ShaderStorageBlockBinding(GLuint program, const GLchar* storageBlockName, GLuint storageBlockBinding); Bool InitWindowSurface(NativeWindowType window); Bool InitPbufferSurface(EGLint width, EGLint height); diff --git a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp index e8ce9d1c..e113d81a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp @@ -740,8 +740,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { funcsTable.GL.MemoryBarrierByRegion = MemoryBarrierByRegion; funcsTable.GL.BindImageTexture = BindImageTexture; funcsTable.GL.GetIntegeri_v = GetIntegeri_v; - funcsTable.GL.GetInteger64i_v = GetInteger64i_v; - funcsTable.GL.GetProgramiv = GetProgramiv; funcsTable.GL.ShaderStorageBlockBinding = ShaderStorageBlockBinding; funcsTable.GL.FenceSync = FenceSync; funcsTable.GL.ClientWaitSync = ClientWaitSync; diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp index 16cc7263..33789e61 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp @@ -78,7 +78,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 blockBindingVersion = 0; Vector storageBlocks; Vector bufferVariables; - GLint computeWorkGroupSize[3] = {1, 1, 1}; }; struct DrawElementsIndirectCommand { @@ -209,16 +208,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { } for (auto& module : modules) { - for (Uint32 entryIndex = 0; entryIndex < module.entry_point_count; ++entryIndex) { - const auto& entryPoint = module.entry_points[entryIndex]; - if ((entryPoint.shader_stage & SPV_REFLECT_SHADER_STAGE_COMPUTE_BIT) == 0) { - continue; - } - cache.computeWorkGroupSize[0] = static_cast(std::max(entryPoint.local_size.x, 1)); - cache.computeWorkGroupSize[1] = static_cast(std::max(entryPoint.local_size.y, 1)); - cache.computeWorkGroupSize[2] = static_cast(std::max(entryPoint.local_size.z, 1)); - } - uint32_t bindingCount = 0; SpvReflectResult result = spvReflectEnumerateDescriptorBindings(&module, &bindingCount, nullptr); if (result != SPV_REFLECT_RESULT_SUCCESS || bindingCount == 0) { @@ -683,130 +672,37 @@ namespace MobileGL::MG_Backend::DirectVulkan { (void)format; } + // The two compute limits are the only indexed pnames a backend genuinely owns: they come + // from the physical device, and MG_Impl/GLImpl/Getter/GL_Getter.cpp asks for them here so it + // can raise the answer to the GL required minimum. Every other indexed pname names FRONTEND + // state (the indexed buffer bindings, the per-unit texture/sampler bindings, the image-unit + // bindings, the viewport rectangles, the indexed capabilities) and is answered there before + // the table is consulted, so the arms this function used to carry for + // GL_SHADER_STORAGE_BUFFER_* and GL_IMAGE_BINDING_* were unreachable duplicates - and not + // even faithful ones: the frontend reports the range glBindBufferRange was ASKED for, + // verbatim, while these clamped it to the buffer's current storage. void GetIntegeri_v(GLenum target, GLuint index, GLint* data) { if (!data) return; MOBILEGL_ASSERT(pVulkanRenderer, "DirectVulkan::GetIntegeri_v called with null VulkanRenderer"); + if (index >= 3) { + *data = 0; + return; + } switch (target) { case GL_MAX_COMPUTE_WORK_GROUP_COUNT: - if (index >= 3) { - *data = 0; - return; - } *data = static_cast( pVulkanRenderer->GetPhysicalDevice().properties.limits.maxComputeWorkGroupCount[index]); return; case GL_MAX_COMPUTE_WORK_GROUP_SIZE: - if (index >= 3) { - *data = 0; - return; - } *data = static_cast( pVulkanRenderer->GetPhysicalDevice().properties.limits.maxComputeWorkGroupSize[index]); return; - case GL_SHADER_STORAGE_BUFFER_BINDING: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - auto& obj = point.GetBoundObject(); - *data = obj ? static_cast(obj->GetExternalIndex()) : 0; - return; - } - case GL_SHADER_STORAGE_BUFFER_START: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - *data = static_cast(point.GetRange().start); - return; - } - case GL_SHADER_STORAGE_BUFFER_SIZE: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - auto& obj = point.GetBoundObject(); - if (!obj) { - *data = 0; - return; - } - const auto& range = point.GetRange(); - const auto start = std::min(range.start, obj->GetSize()); - const auto end = std::min(range.end, obj->GetSize()); - *data = static_cast(end - start); - return; - } - case GL_IMAGE_BINDING_NAME: - case GL_IMAGE_BINDING_LEVEL: - case GL_IMAGE_BINDING_LAYERED: - case GL_IMAGE_BINDING_LAYER: - case GL_IMAGE_BINDING_ACCESS: - case GL_IMAGE_BINDING_FORMAT: { - if (index >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) { - *data = 0; - return; - } - auto& imageBinding = MG_State::pGLContext->GetImageTextureBinding(static_cast(index)); - if (target == GL_IMAGE_BINDING_NAME) { - *data = imageBinding.Texture ? static_cast(imageBinding.Texture->GetExternalIndex()) : 0; - } else if (target == GL_IMAGE_BINDING_LEVEL) { - *data = imageBinding.Level; - } else if (target == GL_IMAGE_BINDING_LAYERED) { - *data = imageBinding.Layered; - } else if (target == GL_IMAGE_BINDING_LAYER) { - *data = imageBinding.Layer; - } else if (target == GL_IMAGE_BINDING_ACCESS) { - *data = static_cast(imageBinding.Access); - } else { - *data = static_cast(imageBinding.Format); - } - return; - } default: *data = 0; return; } } - void GetInteger64i_v(GLenum target, GLuint index, GLint64* data) { - if (!data) return; - switch (target) { - case GL_SHADER_STORAGE_BUFFER_START: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - *data = static_cast(point.GetRange().start); - return; - } - case GL_SHADER_STORAGE_BUFFER_SIZE: { - auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, index); - auto& obj = point.GetBoundObject(); - if (!obj) { - *data = 0; - return; - } - const auto& range = point.GetRange(); - const auto start = std::min(range.start, obj->GetSize()); - const auto end = std::min(range.end, obj->GetSize()); - *data = static_cast(end - start); - return; - } - default: - *data = 0; - return; - } - } - - void GetProgramiv(GLuint program, GLenum pname, GLint* params) { - if (!params) return; - auto* programObject = TryGetDirectVulkanProgram(program); - if (!programObject) { - params[0] = 0; - return; - } - switch (pname) { - case GL_COMPUTE_WORK_GROUP_SIZE: { - auto& cache = GetProgramResourceCache(*programObject); - params[0] = cache.computeWorkGroupSize[0]; - params[1] = cache.computeWorkGroupSize[1]; - params[2] = cache.computeWorkGroupSize[2]; - return; - } - default: - params[0] = 0; - return; - } - } - void ShaderStorageBlockBinding(GLuint program, const GLchar* storageBlockName, GLuint storageBlockBinding) { auto* programObject = TryGetDirectVulkanProgram(program); if (!programObject || storageBlockName == nullptr) return; diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h index 74241e81..cdf36141 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h @@ -95,8 +95,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { void BindImageTexture(GLuint unit, GLuint texture, GLint level, GLboolean layered, GLint layer, GLenum access, GLenum format); void GetIntegeri_v(GLenum target, GLuint index, GLint* data); - void GetInteger64i_v(GLenum target, GLuint index, GLint64* data); - void GetProgramiv(GLuint program, GLenum pname, GLint* params); void ShaderStorageBlockBinding(GLuint program, const GLchar* storageBlockName, GLuint storageBlockBinding); void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels); void GetTexImage(GLenum target, GLint level, GLenum format, GLenum type, GLvoid* pixels); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp index 5ee41649..0e929618 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/AdvertisedLimitsScenario.cpp @@ -378,5 +378,191 @@ namespace MGITest { EXPECT_GE(viewportDims[1], maxRenderbufferSize); } + + // THE INDEXED AND PER-PROGRAM QUERIES THAT NAME FRONTEND STATE, pinned on both lanes. + // + // Both backends used to carry their own arms for GL_SHADER_STORAGE_BUFFER_* and + // GL_IMAGE_BINDING_* inside GLFunctionsTable::GetIntegeri_v, and their own + // GetInteger64i_v / GetProgramiv table entries. None of it was reachable: GL_Getter and + // GL_Program answer every one of these pnames from the frontend's own state and return + // before the table is consulted. The duplicates did not even agree - the backend arms + // clamped a bound range to the buffer's current storage, which GL 4.6 core tables + // 23.4/23.5 do not permit - so the code was one refactor away from becoming the answer. + // These cases pin what the frontend actually reports, so a future move of any of it back + // behind the interface has to keep saying the same thing. + TEST_F(AdvertisedLimitsScenario, IndexedBufferBindingsAreReportedVerbatimOnBothWidths) { + GLuint buffer = 0; + glGenBuffers(1, &buffer); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, buffer); + glBufferData(GL_SHADER_STORAGE_BUFFER, 1024, nullptr, GL_DYNAMIC_DRAW); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + + // A range that is NOT the whole buffer, so a clamp to the store would be visible. + glBindBufferRange(GL_SHADER_STORAGE_BUFFER, 1, buffer, 256, 512); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + + GLint binding32 = -1; + GLint start32 = -1; + GLint size32 = -1; + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_BINDING, 1, &binding32); + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_START, 1, &start32); + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_SIZE, 1, &size32); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_EQ(binding32, static_cast(buffer)); + EXPECT_EQ(start32, 256); + EXPECT_EQ(size32, 512); + + // The 64-bit width has to agree pname for pname. It has no backend entry of its own + // and derives everything from the 32-bit answer above plus its own buffer arm. + GLint64 binding64 = -1; + GLint64 start64 = -1; + GLint64 size64 = -1; + glGetInteger64i_v(GL_SHADER_STORAGE_BUFFER_BINDING, 1, &binding64); + glGetInteger64i_v(GL_SHADER_STORAGE_BUFFER_START, 1, &start64); + glGetInteger64i_v(GL_SHADER_STORAGE_BUFFER_SIZE, 1, &size64); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_EQ(binding64, static_cast(buffer)); + EXPECT_EQ(start64, static_cast(256)); + EXPECT_EQ(size64, static_cast(512)); + + // An unbound index answers zero rather than erroring or leaking the driver's answer. + GLint unbound = -1; + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_BINDING, 0, &unbound); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_EQ(unbound, 0); + + // THE ARM THAT SEPARATES VERBATIM FROM CLAMPED. GL 4.6 core tables 23.4/23.5 report + // the size glBindBufferRange was ASKED for; it does not follow the buffer, so + // shrinking the store underneath the binding must not move it. A clamp to the + // current storage - which is exactly what both backends' deleted arms did - answers + // 128 here, and answers 0 for the bind-then-allocate shape + // KHR-GL43.shader_storage_buffer_object.basic-binding uses. + glBufferData(GL_SHADER_STORAGE_BUFFER, 128, nullptr, GL_DYNAMIC_DRAW); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + GLint startAfterShrink = -1; + GLint sizeAfterShrink = -1; + GLint64 sizeAfterShrink64 = -1; + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_START, 1, &startAfterShrink); + glGetIntegeri_v(GL_SHADER_STORAGE_BUFFER_SIZE, 1, &sizeAfterShrink); + glGetInteger64i_v(GL_SHADER_STORAGE_BUFFER_SIZE, 1, &sizeAfterShrink64); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_EQ(startAfterShrink, 256) + << "the bound range's start followed the buffer through a re-specification"; + EXPECT_EQ(sizeAfterShrink, 512) + << "the bound range's size was clamped to the buffer's current 128-byte storage; the range is " + "state of the BINDING POINT and is reported verbatim"; + EXPECT_EQ(sizeAfterShrink64, static_cast(512)) + << "the 64-bit width disagreed with the 32-bit one about the same pname"; + + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, 0); + glDeleteBuffers(1, &buffer); + (void)FirstGLError(); + } + + TEST_F(AdvertisedLimitsScenario, ImageUnitBindingsAreReportedFromTheFrontendState) { + GLint maxImageUnits = 0; + glGetIntegerv(GL_MAX_IMAGE_UNITS, &maxImageUnits); + (void)FirstGLError(); + if (maxImageUnits < 2) GTEST_SKIP() << "no image units to bind on this lane"; + + GLuint texture = 0; + glGenTextures(1, &texture); + glBindTexture(GL_TEXTURE_2D, texture); + glTexStorage2D(GL_TEXTURE_2D, 2, GL_RGBA8, 8, 8); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + + glBindImageTexture(1, texture, 1, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + + struct Expectation { + GLenum pname; + const char* name; + GLint expected; + }; + const Expectation expectations[] = { + {GL_IMAGE_BINDING_NAME, "GL_IMAGE_BINDING_NAME", static_cast(texture)}, + {GL_IMAGE_BINDING_LEVEL, "GL_IMAGE_BINDING_LEVEL", 1}, + {GL_IMAGE_BINDING_LAYERED, "GL_IMAGE_BINDING_LAYERED", GL_FALSE}, + {GL_IMAGE_BINDING_LAYER, "GL_IMAGE_BINDING_LAYER", 0}, + {GL_IMAGE_BINDING_ACCESS, "GL_IMAGE_BINDING_ACCESS", GL_READ_ONLY}, + {GL_IMAGE_BINDING_FORMAT, "GL_IMAGE_BINDING_FORMAT", GL_RGBA8}, + }; + for (const Expectation& expectation : expectations) { + GLint value = -424242; + glGetIntegeri_v(expectation.pname, 1, &value); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << expectation.name; + EXPECT_EQ(value, expectation.expected) << expectation.name; + + // Same pname through the wide width - it must not fall through to a driver that + // knows nothing about MobileGL's image-unit state. + GLint64 wide = -424242; + glGetInteger64i_v(expectation.pname, 1, &wide); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << expectation.name << " (64-bit)"; + EXPECT_EQ(wide, static_cast(expectation.expected)) << expectation.name << " (64-bit)"; + } + + glBindImageTexture(1, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + glDeleteTextures(1, &texture); + (void)FirstGLError(); + } + + // glGetProgramiv(GL_COMPUTE_WORK_GROUP_SIZE) is a LINK ARTIFACT of the program the + // application wrote. DirectVulkan used to answer it from its own spirv-reflect cache and + // DirectGLES by forwarding to the driver's ESSL program - neither of which the + // application ever named - while GL_Program.cpp has always answered it from + // ProgramObject::GetComputeLocalSize. This pins the declared local size on both lanes. + TEST_F(AdvertisedLimitsScenario, ComputeLocalSizeComesFromTheLinkedProgram) { + static const char* kSource = R"(#version 430 core +layout(local_size_x = 4, local_size_y = 3, local_size_z = 2) in; +layout(std430, binding = 0) buffer Output { uint g_data[]; }; +void main() { g_data[gl_LocalInvocationIndex] = 1u; } +)"; + const GLuint shader = glCreateShader(GL_COMPUTE_SHADER); + glShaderSource(shader, 1, &kSource, nullptr); + glCompileShader(shader); + GLint compiled = 0; + glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled); + if (compiled == GL_FALSE) { + char log[2048] = {}; + glGetShaderInfoLog(shader, sizeof(log) - 1, nullptr, log); + glDeleteShader(shader); + (void)FirstGLError(); + GTEST_SKIP() << "no compute shader support on this lane: " << log; + } + const GLuint program = glCreateProgram(); + glAttachShader(program, shader); + glLinkProgram(program); + glDeleteShader(shader); + GLint linked = 0; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + if (linked == GL_FALSE) { + char log[2048] = {}; + glGetProgramInfoLog(program, sizeof(log) - 1, nullptr, log); + glDeleteProgram(program); + (void)FirstGLError(); + GTEST_SKIP() << "the compute program did not link on this lane: " << log; + } + + GLint localSize[3] = {-1, -1, -1}; + glGetProgramiv(program, GL_COMPUTE_WORK_GROUP_SIZE, localSize); + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + EXPECT_EQ(localSize[0], 4); + EXPECT_EQ(localSize[1], 3); + EXPECT_EQ(localSize[2], 2); + + // A program with no compute stage must answer INVALID_OPERATION, not a stale or + // defaulted (1, 1, 1) - the frontend's rule, and the one a backend that answers from + // its own reflection cache cannot express. + const GLuint empty = glCreateProgram(); + GLint ignored[3] = {0, 0, 0}; + glGetProgramiv(empty, GL_COMPUTE_WORK_GROUP_SIZE, ignored); + EXPECT_EQ(FirstGLError(), GLenum(GL_INVALID_OPERATION)) + << "GL 4.6 core 7.13: the query is only defined for a linked program with a compute shader"; + + glDeleteProgram(empty); + glDeleteProgram(program); + (void)FirstGLError(); + } + } // namespace } // namespace MGITest