[Fix, Test] (MG_Impl, MG_Test): a persistently mapped unpack buffer is a legal source for the compressed uploads too

This commit is contained in:
2026-08-12 09:24:57 -04:00
parent 442e7eec1c
commit 7ccb762936
2 changed files with 93 additions and 44 deletions
+45 -44
View File
@@ -1376,6 +1376,47 @@ namespace MobileGL::MG_Impl::GLImpl {
return true;
}
// The same rules for the COMPRESSED entry points, whose payload size is the imageSize the
// caller passed rather than something derived from a (format, type) pair - and which have no
// datum size, so the alignment rule above does not apply to them. Shared by
// glCompressedTexImage2D and glCompressedTexSubImage2D so the two cannot drift; the point
// that is easy to get wrong and that KHR-GL44.buffer_storage.map_persistent_texture exists to
// check is the first one: a PERSISTENT mapping stays a legal transfer source.
Bool ValidateCompressedUnpackBufferSource(const void* data, SizeT imageSize, const char* caller) {
const auto& unpackBuffer =
MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject();
if (!unpackBuffer) return true;
if (unpackBuffer->IsMapped() && !(unpackBuffer->GetMappingAccess() & BufferMappingAccessBit::Persistent)) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller, "Pixel unpack buffer is currently mapped."));
return false;
}
const SizeT offset = reinterpret_cast<SizeT>(data);
const SizeT bufferSize = unpackBuffer->GetSize();
if (offset > bufferSize || imageSize > bufferSize - offset) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller,
"Unpacking would read past the end of the pixel unpack buffer."));
return false;
}
return true;
}
// Where a compressed upload reads its blocks from: `data` is an offset into the bound unpack
// buffer when there is one, and a client pointer otherwise. Only meaningful once
// ValidateCompressedUnpackBufferSource has passed. Null means there is nothing to read, which
// GL leaves undefined and which callers must not dereference.
const void* CompressedUnpackSource(const void* data) {
const auto& unpackBuffer =
MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject();
if (!unpackBuffer) return data;
return reinterpret_cast<const char*>(unpackBuffer->MappedData()) + reinterpret_cast<SizeT>(data);
}
void TexSubImage3D_State(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width,
GLsizei height, GLsizei depth, GLenum format, GLenum type, const void* pixels) {
TextureUploadTarget textureUploadTarget = MG_Util::ConvertGLEnumToTextureUploadTarget(target);
@@ -3587,28 +3628,8 @@ namespace MobileGL::MG_Impl::GLImpl {
}
// ======================= Processing ================================
const void* compressedBytes = data;
const auto& pixelUnpackBufferObject =
MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject();
if (pixelUnpackBufferObject) {
if (pixelUnpackBufferObject->IsMapped()) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
"Pixel unpack buffer is currently mapped."));
return;
}
const SizeT offset = reinterpret_cast<SizeT>(data);
const SizeT bufferSize = pixelUnpackBufferObject->GetSize();
if (offset > bufferSize || expectedImageSize > bufferSize - offset) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
"Unpacking would read past the end of the pixel unpack buffer."));
return;
}
compressedBytes = reinterpret_cast<const char*>(pixelUnpackBufferObject->MappedData()) + offset;
}
if (!ValidateCompressedUnpackBufferSource(data, expectedImageSize, __func__)) return;
const void* compressedBytes = CompressedUnpackSource(data);
if (expectedImageSize == 0) return; // a zero-sized region is a legal no-op
if (compressedBytes == nullptr) {
// No unpack buffer and a null client pointer: there is nothing to read. GL leaves
@@ -3746,28 +3767,8 @@ namespace MobileGL::MG_Impl::GLImpl {
// SetMipmapCompressedImage re-arms it.
textureMipmapObject->AllocateStorage(textureUploadTarget, level, {{width, height, 1}, internalBytes});
const void* compressedBytes = data;
const auto& pixelUnpackBufferObject =
MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelUnpack).GetBoundObject();
if (pixelUnpackBufferObject) {
if (pixelUnpackBufferObject->IsMapped()) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
"Pixel unpack buffer is currently mapped."));
return;
}
const SizeT offset = reinterpret_cast<SizeT>(data);
const SizeT bufferSize = pixelUnpackBufferObject->GetSize();
if (offset > bufferSize || expectedImageSize > bufferSize - offset) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
"Unpacking would read past the end of the pixel unpack buffer."));
return;
}
compressedBytes = reinterpret_cast<const char*>(pixelUnpackBufferObject->MappedData()) + offset;
}
if (!ValidateCompressedUnpackBufferSource(data, expectedImageSize, __func__)) return;
const void* compressedBytes = CompressedUnpackSource(data);
textureMipmapObject->SetMipmapCompressedImage(textureUploadTarget, level, internalformat, compressedBytes,
expectedImageSize);
textureMipmapObject->MarkStorageDirty(textureUploadTarget, level, true);
+48
View File
@@ -1732,6 +1732,54 @@ TEST_F(TextureTest, CompressedTexSubImage2DUnpacksFromAPixelUnpackBuffer) {
(void)texture;
}
// ARB_buffer_storage's whole point: a PERSISTENTLY mapped buffer stays usable while the map is
// live, including as the source of a texture upload - which is what
// KHR-GL44.buffer_storage.map_persistent_texture checks. An ordinary map still disqualifies it.
// Both compressed entry points share one validator, so both are checked here.
TEST_F(TextureTest, CompressedUploadsAcceptAPersistentlyMappedUnpackBuffer) {
Uint8 source[256];
for (Int i = 0; i < 256; ++i) source[i] = static_cast<Uint8>(255 - i);
GLuint buffer = 0;
MG_Impl::GLImpl::GenBuffers(1, &buffer);
MG_Impl::GLImpl::BindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer);
MG_Impl::GLImpl::BufferStorage(GL_PIXEL_UNPACK_BUFFER, sizeof(source), source,
GL_MAP_PERSISTENT_BIT | GL_MAP_READ_BIT | GL_MAP_WRITE_BIT);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
void* mapped = MG_Impl::GLImpl::MapBufferRange(GL_PIXEL_UNPACK_BUFFER, 0, sizeof(source),
GL_MAP_PERSISTENT_BIT | GL_MAP_READ_BIT | GL_MAP_WRITE_BIT);
ASSERT_NE(mapped, nullptr);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
GLuint texture = 0;
MG_Impl::GLImpl::GenTextures(1, &texture);
MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
MG_Impl::GLImpl::CompressedTexImage2D(GL_TEXTURE_2D, 0, GL_COMPRESSED_RED_RGTC1, 8, 8, 0, kRgtc1Size8x8, nullptr);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "glCompressedTexImage2D over a persistent map";
MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8,
reinterpret_cast<const void*>(static_cast<SizeT>(0)));
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "glCompressedTexSubImage2D over a persistent map";
Uint8 stored[kRgtc1Size8x8] = {};
MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored);
EXPECT_EQ(std::memcmp(stored, source, sizeof(stored)), 0);
MG_Impl::GLImpl::UnmapBuffer(GL_PIXEL_UNPACK_BUFFER);
// The negative control: an ORDINARY map is still an error, so the check above is not just
// "the mapped test was dropped".
GLuint plainBuffer = 0;
MG_Impl::GLImpl::GenBuffers(1, &plainBuffer);
MG_Impl::GLImpl::BindBuffer(GL_PIXEL_UNPACK_BUFFER, plainBuffer);
MG_Impl::GLImpl::BufferData(GL_PIXEL_UNPACK_BUFFER, sizeof(source), source, GL_STATIC_DRAW);
ASSERT_NE(MG_Impl::GLImpl::MapBuffer(GL_PIXEL_UNPACK_BUFFER, GL_READ_ONLY), nullptr);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8,
reinterpret_cast<const void*>(static_cast<SizeT>(0)));
ExpectSingleGlError(GL_INVALID_OPERATION);
MG_Impl::GLImpl::UnmapBuffer(GL_PIXEL_UNPACK_BUFFER);
MG_Impl::GLImpl::BindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
}
// glCompressedTextureSubImage2D was an exported no-op that raised no error at all, so an
// application could not tell the write had not happened. It must reach the NAMED texture and leave
// the binding it borrowed exactly as it found it.