From 48968a663fdce9fa389f9488136248d3eb46a8c7 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 22 Aug 2026 21:49:26 -0400 Subject: [PATCH] [Fix, Test] (GLImpl, Util): let a buffer clear take a GL_INT pattern into a normalized format --- MobileGL/MG_Test/Buffer/BufferTest.cpp | 56 +++++++++++++++++++ .../MG_Util/Texture/PixelStoreProcessor.cpp | 24 ++++++-- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Test/Buffer/BufferTest.cpp b/MobileGL/MG_Test/Buffer/BufferTest.cpp index 7a463b9c..61cdc560 100644 --- a/MobileGL/MG_Test/Buffer/BufferTest.cpp +++ b/MobileGL/MG_Test/Buffer/BufferTest.cpp @@ -711,6 +711,62 @@ TEST_F(BufferTest, ClearNamedBufferSubDataRepeatsPattern) { EXPECT_EQ(actual, (Vector{0, pattern, pattern, pattern, 0})); EXPECT_EQ(MobileGL::MG_Impl::GLImpl::GetError(), GL_NO_ERROR); } + +// GL 4.6 core table 8.2 pairs GL_INT with the non-integer base formats as a signed-normalized +// source, so a GL_R8 clear whose pattern arrives as (GL_RED, GL_INT) is legal. The pair used to be +// rejected with INVALID_VALUE, which is the first call +// KHR-GL45.direct_state_access.buffers_functional makes. +TEST_F(BufferTest, ClearNamedBufferSubDataAcceptsSignedNormalizedIntPattern) { + GLuint buffer = 0; + MobileGL::MG_Impl::GLImpl::CreateBuffers(1, &buffer); + + const Vector initial(24, 0x7F); + MobileGL::MG_Impl::GLImpl::NamedBufferStorage( + buffer, initial.size(), initial.data(), + GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_DYNAMIC_STORAGE_BIT | GL_MAP_PERSISTENT_BIT); + ASSERT_EQ(MobileGL::MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + const GLint zero = 0; + MobileGL::MG_Impl::GLImpl::ClearNamedBufferSubData(buffer, GL_R8, 0, sizeof(GLint), GL_RED, GL_INT, &zero); + EXPECT_EQ(MobileGL::MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Vector actual(initial.size()); + auto bufferObject = MobileGL::MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + Memcpy(actual.data(), bufferObject->AcquireMemory(false, true, false), actual.size()); + Vector expected(initial); + for (SizeT i = 0; i < sizeof(GLint); ++i) expected[i] = 0; + EXPECT_EQ(actual, expected); + + MobileGL::MG_Impl::GLImpl::DeleteBuffers(1, &buffer); + DrainPendingGlErrors(); +} + +// The same pair on the bound-target entry point: the DSA and the bound call share +// ClearBufferRange_State, and a regression in either direction has to show up here too. +TEST_F(BufferTest, ClearBufferSubDataAcceptsSignedNormalizedIntPattern) { + GLuint buffer = 0; + MobileGL::MG_Impl::GLImpl::GenBuffers(1, &buffer); + MobileGL::MG_Impl::GLImpl::BindBuffer(GL_ARRAY_BUFFER, buffer); + + const Vector initial(8, 0x7F); + MobileGL::MG_Impl::GLImpl::BufferData(GL_ARRAY_BUFFER, initial.size(), initial.data(), GL_STATIC_DRAW); + // GL_INT is signed-normalized against 2^31-1, so the maximum maps to a saturated GL_R8 texel. + const GLint one = 2147483647; + MobileGL::MG_Impl::GLImpl::ClearBufferSubData(GL_ARRAY_BUFFER, GL_R8, 0, 4, GL_RED, GL_INT, &one); + EXPECT_EQ(MobileGL::MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Vector actual(initial.size()); + auto bufferObject = MobileGL::MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + Memcpy(actual.data(), bufferObject->AcquireMemory(false, true, false), actual.size()); + EXPECT_EQ(actual, (Vector{0xFF, 0xFF, 0xFF, 0xFF, 0x7F, 0x7F, 0x7F, 0x7F})); + + MobileGL::MG_Impl::GLImpl::BindBuffer(GL_ARRAY_BUFFER, 0); + MobileGL::MG_Impl::GLImpl::DeleteBuffers(1, &buffer); + DrainPendingGlErrors(); +} + TEST_F(BufferTest, ClearBufferSubDataInitializesIrisStaticSsboRange) { GLuint buffer = 0; MobileGL::MG_Impl::GLImpl::GenBuffers(1, &buffer); diff --git a/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp b/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp index 62c633ba..a698c4cf 100644 --- a/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp +++ b/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp @@ -442,14 +442,30 @@ namespace MobileGL::MG_Util::PixelStoreProcessor { return packed.fieldCount == mapping.channelCount; } + // GL 4.6 core table 8.2: every unpacked component type pairs with every base format, + // with only two exclusions - an integer format takes integer types only, and the two + // floating types need a non-integer format. This used to be derived from + // GetDirectShadowComponentForType, which answers a different question (is the client + // layout byte-identical to some shadow layout) and has no SNorm32 to hand back for + // (non-integer format, GL_INT). That legal pair was therefore rejected outright, even + // though ConvertUnpackRow decodes it through DecodeComponentToFloat like every other + // normalized type - which is what glClearBufferData(GL_R8, GL_RED, GL_INT) needs. switch (type) { case TexturePixelDataType::UnsignedInt5999Rev: case TexturePixelDataType::UnsignedInt101111Rev: return !mapping.isInteger && mapping.channelCount == 3; - default: { - ShadowComponent component{}; - return GetDirectShadowComponentForType(type, mapping.isInteger, component); - } + case TexturePixelDataType::UnsignedByte: + case TexturePixelDataType::Byte: + case TexturePixelDataType::UnsignedShort: + case TexturePixelDataType::Short: + case TexturePixelDataType::UnsignedInt: + case TexturePixelDataType::Int: + return true; + case TexturePixelDataType::HalfFloat: + case TexturePixelDataType::Float: + return !mapping.isInteger; + default: + return false; } }