diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index ef8e967c..e6aa40a1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -358,8 +358,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // Require working fences: recycling is gated on the frame-completion // watermark, which only advances if Present can insert/poll fences. return g_GLESFuncs.glFenceSync != nullptr && g_GLESFuncs.glGetSynciv != nullptr && - r.id != 0 && !r.persistentMapped && r.contextGeneration == g_bufferContextGeneration && - r.storageInitialized && r.storageSize > 0 && r.storageSize <= kMaxPoolableBufferBytes; + r.id != 0 && !r.persistentMapped && !r.immutableStorage && + r.contextGeneration == g_bufferContextGeneration && r.storageInitialized && + r.storageSize > 0 && r.storageSize <= kMaxPoolableBufferBytes; } // Retire a buffer id into the pool (owning thread; caller verified IsPoolable). @@ -555,6 +556,17 @@ namespace MobileGL::MG_Backend::DirectGLES { resource = created.get(); bufferObject.SetBackendResource(std::move(created)); } + // Before the generation is stamped, not after: everything on the resource + // describes a context that is gone, and the idempotency check below would + // otherwise hand the caller the dead context's mapped pointer. + if (resource->contextGeneration != g_bufferContextGeneration) { + resource->id = 0; + resource->persistentMapped = false; + resource->persistentPtr = nullptr; + resource->immutableStorage = false; + resource->storageInitialized = false; + resource->storageSize = 0; + } resource->contextGeneration = g_bufferContextGeneration; if (resource->persistentMapped && resource->persistentPtr && resource->storageSize == size) { @@ -564,9 +576,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // Need a fresh id: glBufferStorage fails on a buffer that already has // immutable storage, and any prior mutable store is replaced anyway. if (resource->id != 0) { - ScrubBufferBindingShadowsForId(resource->id); + NoteBufferIdDeleted(resource->id); g_GLESFuncs.glDeleteBuffers(1, &resource->id); resource->id = 0; + resource->immutableStorage = false; } g_GLESFuncs.glGenBuffers(1, &resource->id); if (resource->id == 0) return nullptr; @@ -578,6 +591,10 @@ namespace MobileGL::MG_Backend::DirectGLES { g_GLESFuncs.glBufferStorageEXT(TempBufferTarget, static_cast(size), initial, GL_MAP_WRITE_BIT | kMapPersistentBit | kMapCoherentBit | kDynamicStorageBit); + // Set as soon as the store exists, not once the map succeeds: the failure + // path below leaves this id holding immutable storage, and whoever touches + // it next has to know that glBufferData cannot redefine it. + resource->immutableStorage = true; void* ptr = g_GLESFuncs.glMapBufferRange(TempBufferTarget, 0, static_cast(size), GL_MAP_WRITE_BIT | kMapPersistentBit | kMapCoherentBit); if (!ptr) { @@ -603,7 +620,37 @@ namespace MobileGL::MG_Backend::DirectGLES { void Ops_Respecify(BufferObject& bufferObject) { auto* resource = ResourceOf(bufferObject); if (!resource) return; // lazy: EnsureBufferResource full-uploads on creation - if (resource->persistentMapped) return; // immutable persistent storage is never respecified + // The frontend hands an adopted mapping back before it redefines the store + // (BufferObject::RedefineStorage), so a resource that still carries the + // persistent state here describes the OLD store - and its storage is + // IMMUTABLE (glBufferStorageEXT), which the glBufferData below cannot + // respecify and which the driver would refuse in silence. Retire the id so + // EnsureBufferResource mints a mutable one, with a full upload from the + // shadow the frontend has just filled. + // + // Keyed on the STORAGE, not on persistentMapped: a glMapBufferRange that + // failed after its glBufferStorageEXT succeeded clears persistentMapped and + // still leaves an immutable store behind, and that one reached glBufferData. + if (resource->immutableStorage) { + resource->persistentMapped = false; + resource->persistentPtr = nullptr; + if (resource->id != 0 && CanTouchGLNow() && + resource->contextGeneration == g_bufferContextGeneration) { + NoteBufferIdDeleted(resource->id); + g_GLESFuncs.glDeleteBuffers(1, &resource->id); + resource->id = 0; + resource->immutableStorage = false; + } + // Off the context thread the id cannot be deleted here, and dropping it + // would leak an immutable, persistently mapped store. It stays put, and + // stays flagged, until EnsureBufferResource retires it on the thread + // that owns the context. + resource->storageInitialized = false; + resource->storageSize = 0; + resource->pendingRespecify = true; + resource->pendingRanges.clear(); + return; + } if (!CanTouchGLNow() || resource->id == 0 || resource->contextGeneration != g_bufferContextGeneration) { resource->pendingRespecify = true; @@ -899,6 +946,21 @@ namespace MobileGL::MG_Backend::DirectGLES { // frontend re-acquires a fresh one on its next map. resource->persistentMapped = false; resource->persistentPtr = nullptr; + resource->immutableStorage = false; + } + + // An immutable store nothing maps any more: a respecification of a buffer that + // had been persistently mapped, which Ops_Respecify could not retire because it + // ran off the context thread. glBufferData cannot redefine it, so it is retired + // here, on the thread that can, and the id is re-minted below. + if (resource->immutableStorage && !resource->persistentMapped && resource->id != 0) { + NoteBufferIdDeleted(resource->id); + g_GLESFuncs.glDeleteBuffers(1, &resource->id); + resource->id = 0; + resource->immutableStorage = false; + resource->storageInitialized = false; + resource->storageSize = 0; + resource->pendingRespecify = true; } // Zero-copy coherent persistent buffer: the app writes straight into the diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index a03ec03b..bd7fc955 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -286,6 +286,14 @@ namespace MobileGL::MG_Backend::DirectGLES { // context loss. Bool persistentMapped = false; void* persistentPtr = nullptr; + // The GL store behind `id` was created with glBufferStorageEXT and is + // therefore IMMUTABLE - glBufferData cannot respecify it and it must never be + // recycled through the size-keyed buffer pool. Tracked separately from + // persistentMapped because the two come apart: a glMapBufferRange that fails + // after its glBufferStorageEXT succeeded leaves immutable storage behind with + // no map, and a respecification then has to retire the id rather than hand it + // to glBufferData, which the driver would silently refuse. + Bool immutableStorage = false; }; // Registered as the frontend's BufferBackendOps at backend init and on diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp index 218a3daf..c579c612 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp @@ -379,6 +379,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { BumpSliceEpoch(*resource); // Any cached streaming slice refers to the previous contents. resource->transientFrameSerial = 0; + // Redefining the store hands any adopted mapping back to the CPU shadow + // (BufferObject::RedefineStorage), so a buffer that reaches here persistent-mapped + // is an ordinary resident one again: it needs the busy-tracking and conditional + // orphan below, and the next AcquirePersistentMap has to mint storage for the new + // store rather than hand back a mapping of the old one. + resource->persistentMapped = false; if (!resource->buffer.IsValid()) { return; // streaming-only resource: shadow + serial are enough } diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index 62e0e1a2..bee83f4c 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -1061,7 +1061,7 @@ DECLARE_GL_FUNCTION_HEAD(void, TextureSubImage1D, GLuint texture, GLint level, G DECLARE_GL_FUNCTION_HEAD(void, TextureSubImage2D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLenum type, const void* pixels) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureSubImage2D, texture, level, xoffset, yoffset, width, height, format, type, pixels) DECLARE_GL_FUNCTION_HEAD(void, TextureSubImage3D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, const void* pixels) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureSubImage3D, texture, level, xoffset, yoffset, zoffset, width, height, depth, format, type, pixels) DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureSubImage1D, GLuint texture, GLint level, GLint xoffset, GLsizei width, GLenum format, GLsizei imageSize, const void* data) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureSubImage1D, texture, level, xoffset, width, format, imageSize, data) -DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureSubImage2D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLsizei imageSize, const void* data) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureSubImage2D, texture, level, xoffset, yoffset, width, height, format, imageSize, data) +DECLARE_GL_FUNCTION_HEAD(void, CompressedTextureSubImage2D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLsizei imageSize, const void* data) DECLARE_GL_FUNCTION_END_NO_RETURN(void, CompressedTextureSubImage2D, texture, level, xoffset, yoffset, width, height, format, imageSize, data) DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureSubImage3D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLsizei imageSize, const void* data) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureSubImage3D, texture, level, xoffset, yoffset, zoffset, width, height, depth, format, imageSize, data) DECLARE_GL_FUNCTION_HEAD(void, CopyTextureSubImage1D, GLuint texture, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width) DECLARE_GL_FUNCTION_END_NO_RETURN(void, CopyTextureSubImage1D, texture, level, xoffset, x, y, width) DECLARE_GL_FUNCTION_HEAD(void, CopyTextureSubImage2D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_END_NO_RETURN(void, CopyTextureSubImage2D, texture, level, xoffset, yoffset, x, y, width, height) @@ -1849,7 +1849,7 @@ DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureImage3DEXT, GLuint texture, DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureImage2DEXT, GLuint texture, GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLsizei imageSize, const void* bits) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureImage2DEXT, texture, target, level, internalformat, width, height, border, imageSize, bits) DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureImage1DEXT, GLuint texture, GLenum target, GLint level, GLenum internalformat, GLsizei width, GLint border, GLsizei imageSize, const void* bits) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureImage1DEXT, texture, target, level, internalformat, width, border, imageSize, bits) DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureSubImage3DEXT, GLuint texture, GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLsizei imageSize, const void* bits) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureSubImage3DEXT, texture, target, level, xoffset, yoffset, zoffset, width, height, depth, format, imageSize, bits) -DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureSubImage2DEXT, GLuint texture, GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLsizei imageSize, const void* bits) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureSubImage2DEXT, texture, target, level, xoffset, yoffset, width, height, format, imageSize, bits) +DECLARE_GL_FUNCTION_HEAD(void, CompressedTextureSubImage2DEXT, GLuint texture, GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLsizei imageSize, const void* bits) DECLARE_GL_FUNCTION_END_NO_RETURN(void, CompressedTextureSubImage2D, texture, level, xoffset, yoffset, width, height, format, imageSize, bits) DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedTextureSubImage1DEXT, GLuint texture, GLenum target, GLint level, GLint xoffset, GLsizei width, GLenum format, GLsizei imageSize, const void* bits) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedTextureSubImage1DEXT, texture, target, level, xoffset, width, format, imageSize, bits) DECLARE_GL_FUNCTION_STUB_HEAD(void, GetCompressedTextureImageEXT, GLuint texture, GLenum target, GLint lod, void* img) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetCompressedTextureImageEXT, texture, target, lod, img) DECLARE_GL_FUNCTION_STUB_HEAD(void, CompressedMultiTexImage3DEXT, GLenum texunit, GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLsizei depth, GLint border, GLsizei imageSize, const void* bits) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CompressedMultiTexImage3DEXT, texunit, target, level, internalformat, width, height, depth, border, imageSize, bits) diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index b4237cb7..490278e7 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -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("MG_Impl/GLImpl", caller, "Pixel unpack buffer is currently mapped.")); + return false; + } + + const SizeT offset = reinterpret_cast(data); + const SizeT bufferSize = unpackBuffer->GetSize(); + if (offset > bufferSize || imageSize > bufferSize - offset) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("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(unpackBuffer->MappedData()) + reinterpret_cast(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); @@ -2238,6 +2279,23 @@ namespace MobileGL::MG_Impl::GLImpl { DiscardMipmapChainOnBaseRespecification(textureMipmapObject, textureUploadTarget, level); textureMipmapObject->AllocateStorage(textureUploadTarget, level, {{width, height, 1}, internalBytes}); + // GL 4.6 core 8.5: a SPECIFIC compressed internalformat (unlike a generic + // GL_COMPRESSED_* one, where the implementation is free to choose) commits the + // level to that format - GL_TEXTURE_COMPRESSED must then answer true for it and + // GL_TEXTURE_INTERNAL_FORMAT must report it, which is how an application asks for + // the size to hand glCompressedTexSubImage2D afterwards. Only the tag and the size + // are recorded: there is no BC/ETC codec here, so the texel shadow keeps the + // uncompressed storage this format resolved to (which is also what lets the level + // sample as the application's texels), and the compressed image the tag describes + // is zero-filled - the one reproducible answer glGetCompressedTexImage can give for + // an image nothing ever compressed. AllocateStorage above clears the tag, so this + // has to follow it. + const auto compressedInfo = MG_Util::GetCompressedFormatInfo(static_cast(internalformat)); + if (compressedInfo.blockWidth != 0) { + textureMipmapObject->SetMipmapCompressedImage( + textureUploadTarget, level, static_cast(internalformat), nullptr, + MG_Util::CalculateCompressedTextureImageSize(compressedInfo, {width, height, 1})); + } } if (!originalPixels) { @@ -2965,9 +3023,9 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_TEXTURE_INTERNAL_FORMAT: if (params) { // A level stored compressed must report the token it was given, not the - // uncompressed format backing it (GL 4.6 core 8.11). Only glCompressedTexImage* sets - // that tag, so every level created by glTexImage*D - including one given a compressed - // internalformat - still answers with its resolved storage format. + // uncompressed format backing it (GL 4.6 core 8.11). glCompressedTexImage2D sets + // that tag, and so does a glTexImage2D given a SPECIFIC compressed internalformat; + // every other level answers with its resolved storage format. const GLenum compressedFormat = GetCompressedLevelFormat(textureObject, textureUploadTarget, level); *params = (compressedFormat != GL_NONE) ? (GLint)compressedFormat @@ -3103,9 +3161,9 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_TEXTURE_INTERNAL_FORMAT: if (params) { // A level stored compressed must report the token it was given, not the - // uncompressed format backing it (GL 4.6 core 8.11). Only glCompressedTexImage* sets - // that tag, so every level created by glTexImage*D - including one given a compressed - // internalformat - still answers with its resolved storage format. + // uncompressed format backing it (GL 4.6 core 8.11). glCompressedTexImage2D sets + // that tag, and so does a glTexImage2D given a SPECIFIC compressed internalformat; + // every other level answers with its resolved storage format. const GLenum compressedFormat = GetCompressedLevelFormat(textureObject, textureUploadTarget, level); *params = (GLfloat)((compressedFormat != GL_NONE) ? compressedFormat @@ -3468,10 +3526,169 @@ namespace MobileGL::MG_Impl::GLImpl { RecordUnsupportedCompressedFormat(__func__); } + // Replaces a block-aligned rectangle of the compressed image glCompressedTexImage2D (or a + // compressed glTexImage2D) shadowed for this level. Same deviation as the image call it + // patches: the uncompressed texel shadow beside it is NOT touched, because there is no + // BC/ETC codec here to decode the incoming blocks with - so what changes is the image + // glGetCompressedTexImage hands back, not what the level samples as. Marking the texels + // dirty would therefore only re-upload bytes that did not change. void CompressedTexSubImage2D_State(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLsizei imageSize, const void* data) { - // TODO: implement compressed upload - see CompressedTexImage2D_State. - RecordUnsupportedCompressedFormat(__func__); + // ======================= Converting ================================ + const auto textureUploadTarget = MG_Util::ConvertGLEnumToTextureUploadTarget(target); + const auto textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target); + // Zero block width doubles as "format is not a specific compressed format", the + // INVALID_ENUM case - one lookup answers both questions. + const auto compressedInfo = MG_Util::GetCompressedFormatInfo(format); + + // ===================== Error Checking ============================== + if (!TextureImpl::ValidateTextureUploadTarget(textureUploadTarget)) return; + // A proxy holds no image to modify; only the glTexImage*/glCompressedTexImage* pair + // accepts one. + if (TextureImpl::IsProxyTextureTarget(textureUploadTarget)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + "A proxy target has no texture image to modify.")); + return; + } + if (!TextureImpl::ValidateTextureLevelNumber(level)) return; + if (!TextureImpl::ValidateTextureLevelWithUploadTarget(textureUploadTarget, level)) return; + if (width < 0 || height < 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", __func__, "width and height must be non-negative.")); + return; + } + if (compressedInfo.blockWidth == 0) { + RecordUnsupportedCompressedFormat(__func__); + return; + } + + auto& textureObject = GetTextureObjectByTarget(textureUploadTarget, textureTarget); + if (!TextureImpl::ValidateTextureObject(textureObject)) return; + auto* textureMipmapObject = MG_State::GLState::AsMipmapTexture(textureObject.get()); + if (textureMipmapObject == nullptr) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", __func__, "Texture storage is not mipmap-backed.")); + return; + } + // GL 4.6 core 8.7: INVALID_OPERATION unless the image being modified is stored in + // exactly this compressed format. That is also what makes the block arithmetic below + // sound - the level's grid is measured with THIS format's block size. + const GLenum levelFormat = + textureMipmapObject->GetMipmapCompressedFormat(textureUploadTarget, static_cast(level)); + if (levelFormat != format) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", __func__, + "format does not match the internal format of the texture image.")); + return; + } + + const IntVec3 levelSize = textureMipmapObject->GetMipmapTexelSize(textureUploadTarget, static_cast(level)); + // Written as a subtraction rather than `xoffset + width > levelSize.x()`: both operands + // are application-supplied GLints, so the sum is free to overflow, and a signed overflow + // is undefined behaviour that a compiler may resolve by assuming the check passes. + // levelSize is our own and non-negative, and the offsets are known non-negative by the + // time the subtraction runs, so this form cannot wrap. + if (xoffset < 0 || yoffset < 0 || width > levelSize.x() - xoffset || height > levelSize.y() - yoffset) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", __func__, + "The replaced region does not lie within the texture image.")); + return; + } + // GL 4.6 core 8.7 for block-based formats: the region must start on a block boundary + // and must either be a whole number of blocks wide/high or run to the image's edge. + const Int blockWidth = static_cast(compressedInfo.blockWidth); + const Int blockHeight = static_cast(compressedInfo.blockHeight); + const Bool alignedX = (xoffset % blockWidth == 0) && + (width % blockWidth == 0 || xoffset + width == levelSize.x()); + const Bool alignedY = (yoffset % blockHeight == 0) && + (height % blockHeight == 0 || yoffset + height == levelSize.y()); + if (!alignedX || !alignedY) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", __func__, + "The replaced region is not aligned to the format's compressed blocks.")); + return; + } + // Exactly the size the format and dimensions imply, which is also what keeps the copy + // below in bounds. + const SizeT expectedImageSize = + MG_Util::CalculateCompressedTextureImageSize(compressedInfo, {width, height, 1}); + if (imageSize < 0 || static_cast(imageSize) != expectedImageSize) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", __func__, + "imageSize does not match the compressed image size.")); + return; + } + + // ======================= Processing ================================ + 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 + // this undefined rather than erroring, and dereferencing it is the one answer that + // is never acceptable. + MGLOG_D("%s: null data with no pixel unpack buffer bound, nothing to replace", __func__); + return; + } + + // Once per process: the call is about to succeed, and what it does is narrower than what + // an application has every right to expect from it. Before this existed the call answered + // GL_INVALID_ENUM, which was wrong but at least visible; a silent success that leaves the + // sampled texels untouched is the kind of thing that costs a day to find from the other + // end. MGLOG_I, not _W: warnings are compiled out at the level everything ships at. + static std::atomic announcedNoCodec{false}; + if (!announcedNoCodec.exchange(true)) { + MGLOG_I("%s: the compressed blocks are stored verbatim and returned by " + "glGetCompressedTexImage, but there is no BC/ETC decoder here, so they do not " + "reach the texels this level SAMPLES as. Upload through glTexSubImage2D for " + "that.", + __func__); + } + + // The level's compressed image is stored as one blob, so the rectangle is patched into + // a copy of it and the whole thing handed back. Compressed sub-image uploads are not a + // hot path, and this keeps the storage layer's compressed API to the two calls it has. + const SizeT blobSize = + textureMipmapObject->GetMipmapCompressedByteSize(textureUploadTarget, static_cast(level)); + const void* existing = + textureMipmapObject->MapMipmapCompressedImage(textureUploadTarget, static_cast(level)); + if (blobSize == 0 || existing == nullptr) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", __func__, + "The texture level holds no compressed image to modify.")); + return; + } + Vector blob(blobSize); + Memcpy(blob.data(), existing, blobSize); + + const SizeT blockByteSize = compressedInfo.blockByteSize; + const SizeT levelBlocksX = (static_cast(levelSize.x()) + compressedInfo.blockWidth - 1) / + compressedInfo.blockWidth; + const SizeT levelRowBytes = levelBlocksX * blockByteSize; + const SizeT regionBlocksX = (static_cast(width) + compressedInfo.blockWidth - 1) / + compressedInfo.blockWidth; + const SizeT regionBlocksY = (static_cast(height) + compressedInfo.blockHeight - 1) / + compressedInfo.blockHeight; + const SizeT firstBlockX = static_cast(xoffset) / compressedInfo.blockWidth; + const SizeT firstBlockY = static_cast(yoffset) / compressedInfo.blockHeight; + const SizeT regionRowBytes = regionBlocksX * blockByteSize; + const auto* source = static_cast(compressedBytes); + for (SizeT row = 0; row < regionBlocksY; ++row) { + const SizeT destOffset = (firstBlockY + row) * levelRowBytes + firstBlockX * blockByteSize; + if (destOffset + regionRowBytes > blobSize) break; // a level whose blob predates its size + Memcpy(blob.data() + destOffset, source + row * regionRowBytes, regionRowBytes); + } + textureMipmapObject->SetMipmapCompressedImage(textureUploadTarget, static_cast(level), format, + blob.data(), blobSize); } void CompressedTexSubImage1D_State(GLenum target, GLint level, GLint xoffset, GLsizei width, GLenum format, @@ -3564,28 +3781,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("MG_Impl/GLImpl", __func__, - "Pixel unpack buffer is currently mapped.")); - return; - } - const SizeT offset = reinterpret_cast(data); - const SizeT bufferSize = pixelUnpackBufferObject->GetSize(); - if (offset > bufferSize || expectedImageSize > bufferSize - offset) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", __func__, - "Unpacking would read past the end of the pixel unpack buffer.")); - return; - } - compressedBytes = reinterpret_cast(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); @@ -4095,6 +4292,13 @@ namespace MobileGL::MG_Impl::GLImpl { // core 8.19). Allocating only the primary one left the object cube-incomplete, so every // framebuffer it was attached to reported GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT. Every other // 2D target has exactly one upload target, so this loop is a no-op change for them. + // A specific compressed internalformat commits every level it allocates to that + // format, the same way glTexImage2D does - and here it matters twice over, because + // immutable storage plus glCompressedTexSubImage2D IS the modern way to upload a + // compressed texture: without the tag that sub-image call finds an uncompressed + // level and refuses it. Zero width means a generic (implementation's choice) + // format, which MobileGL answers with uncompressed storage, so it is not tagged. + const auto compressedInfo = MG_Util::GetCompressedFormatInfo(internalformat); for (const auto uploadTarget : textureObject->GetUploadTargets()) { for (GLsizei level = 0; level < levels; ++level) { const GLsizei levelWidth = std::max(1, width >> level); @@ -4103,6 +4307,13 @@ namespace MobileGL::MG_Impl::GLImpl { static_cast(levelWidth) * static_cast(levelHeight) * bytesPerPixel; textureMipmapObject->AllocateStorage(uploadTarget, level, {{levelWidth, levelHeight, 1}, byteSize}); textureMipmapObject->MarkStorageDirty(uploadTarget, level, false); + if (compressedInfo.blockWidth != 0) { + // After AllocateStorage, which clears the tag. + textureMipmapObject->SetMipmapCompressedImage( + uploadTarget, static_cast(level), internalformat, nullptr, + MG_Util::CalculateCompressedTextureImageSize(compressedInfo, + {levelWidth, levelHeight, 1})); + } } // See TextureStorage1D. textureMipmapObject->TruncateMipmapLevels(uploadTarget, static_cast(levels)); @@ -4472,6 +4683,14 @@ namespace MobileGL::MG_Impl::GLImpl { free(processedPixels); } + void CompressedTextureSubImage2D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLsizei width, + GLsizei height, GLenum format, GLsizei imageSize, const void* data) { + auto textureObject = GetTextureObjectByName(texture, __func__); + WithTemporarilyBoundNamedTexture(textureObject, [&](GLenum target) { + CompressedTexSubImage2D_State(target, level, xoffset, yoffset, width, height, format, imageSize, data); + }); + } + void TextureSubImage3D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, const void* pixels) { auto textureObject = GetTextureObjectByName(texture, __func__); diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h index 7f800cf5..604f881b 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h @@ -37,6 +37,8 @@ namespace MobileGL::MG_Impl::GLImpl { GLenum format, GLenum type, const void* pixels); void TextureSubImage3D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, const void* pixels); + void CompressedTextureSubImage2D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLsizei width, + GLsizei height, GLenum format, GLsizei imageSize, const void* data); void TextureParameterf(GLuint texture, GLenum pname, GLfloat param); void TextureParameterfv(GLuint texture, GLenum pname, const GLfloat* params); void TextureParameteri(GLuint texture, GLenum pname, GLint param); diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index b6a520cb..6ca4b79a 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -71,6 +71,8 @@ add_executable(MobileGLIntegrationTest Scenarios/FragmentOutputArrayIndexScenario.cpp Scenarios/BufferTextureScenario.cpp Scenarios/VertexAttribBindingScenario.cpp + Scenarios/XfbCaptureBufferReuseScenario.cpp + Scenarios/VertexArrayEnableDisableScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/VertexArrayEnableDisableScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/VertexArrayEnableDisableScenario.cpp new file mode 100644 index 00000000..938944b9 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/VertexArrayEnableDisableScenario.cpp @@ -0,0 +1,264 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/VertexArrayEnableDisableScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// KHR-GL45.direct_state_access.vertex_arrays_enable_disable_attributes, rebuilt. +// +// The case is small and does one unusual thing twice: it turns half of +// GL_MAX_VERTEX_ATTRIBS attribute arrays on and the other half off with +// glEnableVertexArrayAttrib / glDisableVertexArrayAttrib on a vertex array object +// that is NOT bound (it binds the default one first, on purpose), draws one point +// through a program that reads exactly the enabled half, and checks the sum those +// arrays produced. Then it swaps which half is enabled, draws again through a +// SECOND program, and checks the other sum. +// +// Both draws capture into ONE four-byte transform feedback buffer, allocated once +// with immutable storage and read back with glMapBuffer - so anything that only +// works on the first capture span through a buffer fails the second check while +// leaving the first one green. +// +// It is reassembled here rather than shortened because every one of those details +// is a candidate: the unbound-VAO enables, the two-program swap, the integer +// attributes fetched with glVertexAttribIPointer at a stride wider than one +// element, the second capture span, and the fact that the sums differ ONLY in +// which arrays contributed (a fetch that ignored the enable state, or one that +// read the wrong element, lands on a different number, not on garbage). + +#include +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + GLuint CompileShader(GLenum type, const std::string& source, std::string* log) { + const GLuint shader = glCreateShader(type); + const char* text = source.c_str(); + glShaderSource(shader, 1, &text, nullptr); + glCompileShader(shader); + GLint status = GL_FALSE; + glGetShaderiv(shader, GL_COMPILE_STATUS, &status); + if (status == GL_FALSE) { + GLint length = 0; + glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &length); + std::vector buffer(static_cast(length) + 1, '\0'); + glGetShaderInfoLog(shader, length + 1, nullptr, buffer.data()); + if (log != nullptr) *log = buffer.data(); + glDeleteShader(shader); + return 0; + } + return shader; + } + + // Declares and sums the even (parity 0) or odd (parity 1) attributes only, with the + // locations assigned by glBindAttribLocation rather than a layout qualifier - which is + // what the CTS case does, and which makes the attribute set the program reads a link + // property rather than a source one. + GLuint BuildSumProgram(int parity, int attributeCount, std::string* log) { + std::string declarations; + std::string copies = " sum = 0;\n"; + for (int i = parity; i < attributeCount; i += 2) { + declarations += "in int a_" + std::to_string(i) + ";\n"; + copies += " sum += a_" + std::to_string(i) + ";\n"; + } + // `flat` where the CTS case has none: an integral shader output cannot be + // interpolated, so a driver is within its rights to reject the unqualified form + // even with no matching fragment input. The capture reads the same value either + // way, and the qualifier keeps this scenario portable off llvmpipe. + const std::string vertexSource = "#version 450\n\n" + declarations + + "flat out int sum;\n\nvoid main()\n{\n" + copies + "}\n"; + const std::string fragmentSource = R"(#version 450 + +out vec4 color; + +void main() +{ + color = vec4(1.0); +} +)"; + const GLuint vertexShader = CompileShader(GL_VERTEX_SHADER, vertexSource, log); + if (vertexShader == 0) return 0; + const GLuint fragmentShader = CompileShader(GL_FRAGMENT_SHADER, fragmentSource, log); + if (fragmentShader == 0) { + glDeleteShader(vertexShader); + return 0; + } + + const GLuint program = glCreateProgram(); + glAttachShader(program, vertexShader); + glAttachShader(program, fragmentShader); + const char* varying = "sum"; + glTransformFeedbackVaryings(program, 1, &varying, GL_INTERLEAVED_ATTRIBS); + for (int i = parity; i < attributeCount; i += 2) { + const std::string name = "a_" + std::to_string(i); + glBindAttribLocation(program, static_cast(i), name.c_str()); + } + glLinkProgram(program); + glDeleteShader(vertexShader); + glDeleteShader(fragmentShader); + + GLint status = GL_FALSE; + glGetProgramiv(program, GL_LINK_STATUS, &status); + if (status == GL_FALSE) { + GLint length = 0; + glGetProgramiv(program, GL_INFO_LOG_LENGTH, &length); + std::vector buffer(static_cast(length) + 1, '\0'); + glGetProgramInfoLog(program, length + 1, nullptr, buffer.data()); + if (log != nullptr) *log = buffer.data(); + glDeleteProgram(program); + return 0; + } + return program; + } + + class VertexArrayEnableDisableScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + + glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &m_attributeCount); + ASSERT_GE(m_attributeCount, 16); + + std::string log; + m_even = BuildSumProgram(0, m_attributeCount, &log); + ASSERT_NE(m_even, 0u) << "even program failed to build: " << log; + m_odd = BuildSumProgram(1, m_attributeCount, &log); + ASSERT_NE(m_odd, 0u) << "odd program failed to build: " << log; + + // One element per attribute, read as one vertex whose stride spans them all. + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + glGenBuffers(1, &m_vbo); + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + std::vector reference(static_cast(m_attributeCount)); + for (int i = 0; i < m_attributeCount; ++i) reference[static_cast(i)] = i; + glBufferData(GL_ARRAY_BUFFER, static_cast(reference.size() * sizeof(GLint)), + reference.data(), GL_STATIC_DRAW); + for (int i = 0; i < m_attributeCount; ++i) { + glVertexAttribIPointer(static_cast(i), 1, GL_INT, + static_cast(sizeof(GLint) * m_attributeCount), + reinterpret_cast(static_cast(i) * sizeof(GLint))); + } + glBindBuffer(GL_ARRAY_BUFFER, 0); + + // Immutable storage, allocated once, read back with glMapBuffer - the capture + // buffer is never respecified between the two spans. + glGenBuffers(1, &m_xfb); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, m_xfb); + glBufferStorage(GL_TRANSFORM_FEEDBACK_BUFFER, sizeof(GLint), nullptr, GL_MAP_READ_BIT); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, m_xfb); + ASSERT_EQ(glGetError(), GL_NO_ERROR) << "capture buffer setup"; + } + + void TearDown() override { + if (!Ready()) return; + glUseProgram(0); + glBindVertexArray(0); + if (m_xfb != 0) glDeleteBuffers(1, &m_xfb); + if (m_vbo != 0) glDeleteBuffers(1, &m_vbo); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + if (m_even != 0) glDeleteProgram(m_even); + if (m_odd != 0) glDeleteProgram(m_odd); + ScenarioTest::TearDown(); + } + + // Enables one parity's arrays and disables the other's, THROUGH THE OBJECT NAME + // while a different vertex array object is bound. + void TurnOnAttributes(int enabledParity) { + glBindVertexArray(0); + for (int i = 0; i < m_attributeCount; ++i) { + if (i % 2 == enabledParity % 2) { + glEnableVertexArrayAttrib(m_vao, static_cast(i)); + } else { + glDisableVertexArrayAttrib(m_vao, static_cast(i)); + } + ASSERT_EQ(glGetError(), GL_NO_ERROR) << "attribute " << i << ", parity " << enabledParity; + } + glBindVertexArray(m_vao); + } + + int ExpectedSum(int parity) const { + int sum = 0; + for (int i = parity; i < m_attributeCount; i += 2) sum += i; + return sum; + } + + // One capture span, read back the way the CTS case does. + int DrawAndRead(int parity) { + glUseProgram(parity == 0 ? m_even : m_odd); + glBindVertexArray(m_vao); + glBeginTransformFeedback(GL_POINTS); + glDrawArrays(GL_POINTS, 0, 1); + glEndTransformFeedback(); + + const void* mapped = glMapBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, GL_READ_ONLY); + if (mapped == nullptr) { + ADD_FAILURE() << "glMapBuffer returned null for parity " << parity; + return -1; + } + GLint result = -1; + std::memcpy(&result, mapped, sizeof(result)); + glUnmapBuffer(GL_TRANSFORM_FEEDBACK_BUFFER); + return result; + } + + GLint m_attributeCount = 16; + GLuint m_even = 0; + GLuint m_odd = 0; + GLuint m_vao = 0; + GLuint m_vbo = 0; + GLuint m_xfb = 0; + }; + + // The case verbatim: even half on, draw, check; odd half on, draw, check. + TEST_F(VertexArrayEnableDisableScenario, EitherHalfOfTheAttributesInTurn) { + if (!Ready()) GTEST_SKIP(); + + TurnOnAttributes(0); + EXPECT_EQ(DrawAndRead(0), ExpectedSum(0)) << "even attributes"; + + TurnOnAttributes(1); + EXPECT_EQ(DrawAndRead(1), ExpectedSum(1)) << "odd attributes"; + + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // The first span on its own, so a failure of the case above can be read as "the second + // span" rather than "the enables". + TEST_F(VertexArrayEnableDisableScenario, TheEvenHalfAlone) { + if (!Ready()) GTEST_SKIP(); + + TurnOnAttributes(0); + EXPECT_EQ(DrawAndRead(0), ExpectedSum(0)); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // And the odd half as the FIRST span, which separates "the odd program/arrays are + // wrong" from "the second span is wrong". + TEST_F(VertexArrayEnableDisableScenario, TheOddHalfAlone) { + if (!Ready()) GTEST_SKIP(); + + TurnOnAttributes(1); + EXPECT_EQ(DrawAndRead(1), ExpectedSum(1)); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + } // namespace +} // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp index 6cac2fc7..ec8c5ada 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/VertexAttribBindingScenario.cpp @@ -139,16 +139,18 @@ void main() { // As CapturePoints, but through the baseInstance entry point, and on a capture buffer // of its own. // - // Kept separate from CapturePoints rather than defaulting a parameter, for two - // reasons. Every existing caller stays on the draw command that carries no - // baseInstance at all, so the negative control is a DIFFERENT command rather than - // the same one passed a zero. And baseInstance is the first thing here that needs - // several captures in ONE test, which the shared helper cannot currently do: a - // second capture into the same buffer object comes back empty on DirectVulkan - // (respecifying a buffer that is bound to a transform-feedback binding point does - // not reach that binding - reproduced with two plain CapturePoints calls, so it is - // neither about baseInstance nor about this helper). A fresh buffer per capture - // sidesteps it; without that, this scenario would be pinning that bug instead. + // Kept separate from CapturePoints rather than defaulting a parameter, so that every + // existing caller stays on the draw command that carries no baseInstance at all: the + // negative control is then a DIFFERENT command rather than the same one passed a zero. + // + // The buffer per capture is a leftover. baseInstance was the first thing here that + // needed several captures in ONE test, and at the time a second capture into the same + // buffer object came back empty on DirectVulkan - respecifying a buffer whose bytes the + // backend had handed the frontend a pointer into replaced the storage under that + // pointer, so the capture wrote one store and the readback read another. That is fixed + // and pinned by XfbCaptureBufferReuseScenario, which owns the shape now; a buffer per + // capture is simply the cheapest thing that still isolates these three draws from each + // other. std::vector CaptureOwnBufferBaseInstance(GLuint program, int vertexCount, int instanceCount, GLuint baseInstance, bool useBaseInstanceCommand) { const std::size_t floats = static_cast(vertexCount) * instanceCount * 16; diff --git a/MobileGL/MG_IntegrationTest/Scenarios/XfbCaptureBufferReuseScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/XfbCaptureBufferReuseScenario.cpp new file mode 100644 index 00000000..7d44c3fb --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/XfbCaptureBufferReuseScenario.cpp @@ -0,0 +1,317 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/XfbCaptureBufferReuseScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// ONE capture buffer, SEVERAL capture spans - the shape most KHR-GL4x cases that +// use transform feedback as a readback channel are built on. They allocate the +// capture buffer once in a setup step and then run span after span through it, +// so a defect that only shows from the second span onwards fails the whole case +// while the first span (and every single-span scenario in this suite) stays +// green. The first thing checked here is therefore not the capture itself but +// that the bytes the capture wrote are the bytes the readback reads. +// +// Two ways of reusing the buffer, because they exercise different machinery: +// +// * respecified between spans (glBufferData while the buffer is still bound to +// the transform-feedback binding point), which is what a test helper that +// poisons its capture buffer before every span does; +// * allocated ONCE with immutable storage and never touched again, which is +// what KHR-GL45.direct_state_access.vertex_arrays_enable_disable_attributes +// does - glBufferStorage(4 bytes) in its setup, then two draws. +// +// The negative control (a fresh buffer object per span) is a separate case +// rather than a parameter: it is the configuration that already worked, so it +// has to keep working for the others to mean anything. + +#include +#include +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr float kPoison = -1234.0f; + // One vec4 per point, one point per draw. + constexpr std::size_t kCaptureFloats = 4; + constexpr std::size_t kCaptureBytes = kCaptureFloats * sizeof(float); + + GLuint CompileShader(GLenum type, const std::string& source, std::string* log) { + const GLuint shader = glCreateShader(type); + const char* text = source.c_str(); + glShaderSource(shader, 1, &text, nullptr); + glCompileShader(shader); + GLint status = GL_FALSE; + glGetShaderiv(shader, GL_COMPILE_STATUS, &status); + if (status == GL_FALSE) { + GLint length = 0; + glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &length); + std::vector buffer(static_cast(length) + 1, '\0'); + glGetShaderInfoLog(shader, length + 1, nullptr, buffer.data()); + if (log != nullptr) *log = buffer.data(); + glDeleteShader(shader); + return 0; + } + return shader; + } + + // Vertex-only capture program: whatever the draw fetched at location 0 comes + // straight back out through the capture. Runs under GL_RASTERIZER_DISCARD, so + // there is no fragment stage. + GLuint BuildCaptureProgram(std::string* log) { + const std::string vertexSource = R"(#version 430 core +layout(location = 0) in vec4 vs_in_value; +out vec4 vs_out_value; +void main() { + vs_out_value = vs_in_value; +} +)"; + const GLuint vertexShader = CompileShader(GL_VERTEX_SHADER, vertexSource, log); + if (vertexShader == 0) return 0; + const GLuint program = glCreateProgram(); + glAttachShader(program, vertexShader); + const char* varying = "vs_out_value"; + glTransformFeedbackVaryings(program, 1, &varying, GL_INTERLEAVED_ATTRIBS); + glLinkProgram(program); + glDeleteShader(vertexShader); + GLint status = GL_FALSE; + glGetProgramiv(program, GL_LINK_STATUS, &status); + if (status == GL_FALSE) { + GLint length = 0; + glGetProgramiv(program, GL_INFO_LOG_LENGTH, &length); + std::vector buffer(static_cast(length) + 1, '\0'); + glGetProgramInfoLog(program, length + 1, nullptr, buffer.data()); + if (log != nullptr) *log = buffer.data(); + glDeleteProgram(program); + return 0; + } + return program; + } + + class XfbCaptureBufferReuseScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + + std::string log; + m_program = BuildCaptureProgram(&log); + ASSERT_NE(m_program, 0u) << "capture program failed to build: " << log; + + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + glGenBuffers(1, &m_vbo); + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + glBufferData(GL_ARRAY_BUFFER, kCaptureBytes, nullptr, GL_DYNAMIC_DRAW); + glVertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, nullptr); + glEnableVertexAttribArray(0); + glBindBuffer(GL_ARRAY_BUFFER, 0); + } + + void TearDown() override { + if (!Ready()) return; + glBindVertexArray(0); + if (m_vbo != 0) glDeleteBuffers(1, &m_vbo); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + if (m_program != 0) glDeleteProgram(m_program); + glUseProgram(0); + ScenarioTest::TearDown(); + } + + // The vertex the next span will fetch and capture. + void SetVertex(float value) { + const float data[kCaptureFloats] = {value, value + 1.0f, value + 2.0f, value + 3.0f}; + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + glBufferSubData(GL_ARRAY_BUFFER, 0, kCaptureBytes, data); + glBindBuffer(GL_ARRAY_BUFFER, 0); + } + + // One capture span over the buffer currently bound to capture point 0. + void RunSpan() { + glEnable(GL_RASTERIZER_DISCARD); + glUseProgram(m_program); + glBindVertexArray(m_vao); + glBeginTransformFeedback(GL_POINTS); + glDrawArrays(GL_POINTS, 0, 1); + glEndTransformFeedback(); + glDisable(GL_RASTERIZER_DISCARD); + glUseProgram(0); + } + + static ::testing::AssertionResult CapturedIs(const float* data, float value) { + for (std::size_t i = 0; i < kCaptureFloats; ++i) { + const float expected = value + static_cast(i); + const float got = data[i]; + // isfinite first: every ordered comparison against a NaN is false, so a + // pair of one-sided range tests REPORTS SUCCESS for uninitialised + // storage that happens to read as NaN - which is exactly the failure + // these scenarios exist to catch. + if (!std::isfinite(got) || std::fabs(got - expected) > 0.01f) { + return ::testing::AssertionFailure() + << "component " << i << " is " << got << ", expected " << expected + << (got == kPoison ? " (the capture never reached these bytes)" : ""); + } + } + return ::testing::AssertionSuccess(); + } + + GLuint m_program = 0; + GLuint m_vao = 0; + GLuint m_vbo = 0; + }; + + // The negative control: one buffer object per span. This is the configuration + // every multi-span scenario in this suite works around the others with, so it + // has to hold or nothing below is interpretable. + TEST_F(XfbCaptureBufferReuseScenario, EverySpanIntoABufferObjectOfItsOwn) { + if (!Ready()) GTEST_SKIP(); + + for (int span = 0; span < 3; ++span) { + const float value = 10.0f * static_cast(span + 1); + const std::vector poison(kCaptureFloats, kPoison); + + GLuint xfbBuffer = 0; + glGenBuffers(1, &xfbBuffer); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer); + glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, kCaptureBytes, poison.data(), GL_DYNAMIC_DRAW); + + SetVertex(value); + RunSpan(); + + float readback[kCaptureFloats] = {kPoison, kPoison, kPoison, kPoison}; + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, kCaptureBytes, readback); + EXPECT_TRUE(CapturedIs(readback, value)) << "span " << span; + + glDeleteBuffers(1, &xfbBuffer); + } + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // The same three spans through ONE buffer object, respecified before each of + // them WHILE it is bound to capture point 0 - a helper poisoning its capture + // buffer, which is what makes "captured nothing" legible in the first place. + // + // A respecification is free to replace the storage underneath (that is what + // orphaning is), and on a buffer whose bytes the backend has already handed + // the frontend a pointer into, the replacement has to reach that pointer too. + // It did not: the capture wrote the new storage and the readback kept reading + // the old one, so every span after the first came back poison. + TEST_F(XfbCaptureBufferReuseScenario, EverySpanIntoOneRespecifiedBufferObject) { + if (!Ready()) GTEST_SKIP(); + + GLuint xfbBuffer = 0; + glGenBuffers(1, &xfbBuffer); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer); + + for (int span = 0; span < 3; ++span) { + const float value = 10.0f * static_cast(span + 1); + const std::vector poison(kCaptureFloats, kPoison); + glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, kCaptureBytes, poison.data(), GL_DYNAMIC_DRAW); + + SetVertex(value); + RunSpan(); + + float readback[kCaptureFloats] = {kPoison, kPoison, kPoison, kPoison}; + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, kCaptureBytes, readback); + EXPECT_TRUE(CapturedIs(readback, value)) << "span " << span; + } + + glDeleteBuffers(1, &xfbBuffer); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // A respecification that CHANGES the size, which is the case a re-pointing + // that only handled same-size storage would still get wrong - and, before the + // fix, the case that wrote the new (larger) contents through a mapping sized + // for the old ones. + TEST_F(XfbCaptureBufferReuseScenario, ARespecificationMayChangeTheCaptureBufferSize) { + if (!Ready()) GTEST_SKIP(); + + GLuint xfbBuffer = 0; + glGenBuffers(1, &xfbBuffer); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer); + + // Sized for one point, then for four, then back down to one. + const std::size_t pointCapacity[] = {1, 4, 1}; + for (int span = 0; span < 3; ++span) { + const float value = 10.0f * static_cast(span + 1); + const std::size_t floats = kCaptureFloats * pointCapacity[span]; + const std::vector poison(floats, kPoison); + glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, static_cast(floats * sizeof(float)), + poison.data(), GL_DYNAMIC_DRAW); + + SetVertex(value); + RunSpan(); + + std::vector readback(floats, kPoison); + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, + static_cast(floats * sizeof(float)), readback.data()); + EXPECT_TRUE(CapturedIs(readback.data(), value)) << "span " << span; + // The bytes past the one point the draw produced must still be the + // poison the respecification put there, not whatever the previous + // (differently sized) storage held. + for (std::size_t i = kCaptureFloats; i < floats; ++i) { + EXPECT_FLOAT_EQ(readback[i], kPoison) << "span " << span << " float " << i; + } + } + + glDeleteBuffers(1, &xfbBuffer); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // The KHR-GL45.direct_state_access.vertex_arrays_enable_disable_attributes + // shape: the capture buffer gets IMMUTABLE storage once, in a setup step, and + // is never respecified - two spans simply run through it, each read back with + // glMapBuffer. Nothing here may depend on a respecification to reset the + // capture: glBeginTransformFeedback does that on its own. + TEST_F(XfbCaptureBufferReuseScenario, EverySpanIntoOneImmutableStorageBuffer) { + if (!Ready()) GTEST_SKIP(); + + GLuint xfbBuffer = 0; + glGenBuffers(1, &xfbBuffer); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, xfbBuffer); + // Poisoned at creation - the storage is immutable, so this is the only chance to + // put a recognisable value there, and without it a span that captured nothing + // would be indistinguishable from one that captured the right thing whenever the + // untouched bytes happened to read back as the expected number. + const std::vector poison(kCaptureFloats, kPoison); + glBufferStorage(GL_TRANSFORM_FEEDBACK_BUFFER, kCaptureBytes, poison.data(), GL_MAP_READ_BIT); + ASSERT_EQ(glGetError(), GL_NO_ERROR) << "glBufferStorage on the capture buffer"; + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer); + + for (int span = 0; span < 3; ++span) { + const float value = 10.0f * static_cast(span + 1); + SetVertex(value); + RunSpan(); + + const void* mapped = glMapBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, GL_READ_ONLY); + ASSERT_NE(mapped, nullptr) << "span " << span << ": glMapBuffer returned null"; + float readback[kCaptureFloats] = {kPoison, kPoison, kPoison, kPoison}; + std::memcpy(readback, mapped, kCaptureBytes); + glUnmapBuffer(GL_TRANSFORM_FEEDBACK_BUFFER); + EXPECT_TRUE(CapturedIs(readback, value)) << "span " << span; + } + + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, 0); + glDeleteBuffers(1, &xfbBuffer); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + } // namespace +} // namespace MGITest diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp index 5ec71a9c..6e6aa649 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp +++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp @@ -75,10 +75,42 @@ namespace MobileGL::MG_State::GLState { NotifySubData(offset, size); } - void BufferObject::Respecify(SizeT size, const void* data) { - ReleaseMemory(); + // A (re)definition of the store is about to write `size` bytes through Bytes(). + // Sizing the shadow is all that takes for a shadow-backed buffer. A buffer whose + // bytes were adopted into backend GPU memory has to give the adoption back first, + // because the mapping it holds describes exactly the OLD store: writing the new + // contents through it runs past its end the moment the store grows, and a backend + // that replaces the storage for the new store - which is what an orphaning + // respecification asks for - would leave that mapping, and therefore every later + // read of this buffer, addressing storage nothing writes to any more. That was the + // transform feedback capture that wrote one buffer while the readback read another. + // + // Given back rather than renewed here, deliberately. Renewing in place would mean + // memcpying the new contents into storage that submitted-but-unretired draws may + // still be reading, which is precisely what the orphaning idiom exists to avoid; + // avoiding THAT would mean either stalling on a fence in the middle of a frame or + // teaching the persistent-map op to orphan, and the op must never orphan for the + // other kind of caller (an application-held GL_MAP_PERSISTENT_BIT mapping, whose + // pointer has to stay valid for the buffer's whole life). Handing the store back to + // the CPU shadow needs none of that: the backend's ordinary respecification path + // then does the busy-tracking and the conditional orphan it has always done, and the + // next binding that wants GPU residency takes a fresh mapping of the new store. + void BufferObject::RedefineStorage(SizeT size) { + if (m_resource.IsGpuResident()) { + m_resource.ReleasePersistentMap(); + // Whatever a shader or a capture wrote is in the store being replaced, so + // there is nothing left to reconcile - and leaving the flag set would make + // the next read of this buffer wait for GPU work on behalf of bytes the + // application has just thrown away. + m_gpuWritePending = false; + } m_size = size; m_resource.ResizeShadow(size); + } + + void BufferObject::Respecify(SizeT size, const void* data) { + ReleaseMemory(); + RedefineStorage(size); if (data && size > 0) { Memcpy(m_resource.Bytes(), data, size); } @@ -96,8 +128,7 @@ namespace MobileGL::MG_State::GLState { void BufferObject::AllocateImmutableStorage(SizeT size, const void* data, GLbitfield storageFlags) { ReleaseMemory(); - m_size = size; - m_resource.ResizeShadow(size); + RedefineStorage(size); if (data) { Memcpy(m_resource.Bytes(), data, size); } else if (size > 0) { diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.h b/MobileGL/MG_State/GLState/BufferState/BufferObject.h index a1d366eb..425dd80f 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.h +++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.h @@ -205,6 +205,9 @@ namespace MobileGL { void SetBackendResource(SharedPtr resource); private: + // Sizes the store for a (re)definition, renewing an adopted GPU-resident + // mapping across it. See the definition for why the renewal is not optional. + void RedefineStorage(SizeT size); void NotifyRespecify(); void NotifySubData(SizeT offset, SizeT size); void NotifyFlushMappedRange(Range1D range, Flags appAccess); diff --git a/MobileGL/MG_State/GLState/BufferState/PipeResource.h b/MobileGL/MG_State/GLState/BufferState/PipeResource.h index 235efc0a..3fcdc0b4 100644 --- a/MobileGL/MG_State/GLState/BufferState/PipeResource.h +++ b/MobileGL/MG_State/GLState/BufferState/PipeResource.h @@ -70,6 +70,15 @@ namespace MobileGL::MG_State::GLState { m_shadow->shrink_to_fit(); } + // Give the adoption back: the bytes resolve against the shadow again (which + // the caller must (re)size, it was released on adoption). Used when the store + // itself is redefined - the mapping describes exactly the store that is going + // away, so it may neither be written through nor kept. It is NOT a general + // "unmap": a persistent map the application holds outlives every unmap by + // definition, and the calls that could redefine such a buffer's store are + // errors the frontend refuses before reaching here. + void ReleasePersistentMap() { m_gpuMapped = nullptr; } + // Backend GPU resource, owned here in both modes. const SharedPtr& Backend() const { return m_backend; } void SetBackend(SharedPtr backend) { m_backend = std::move(backend); } diff --git a/MobileGL/MG_Test/Buffer/BufferTest.cpp b/MobileGL/MG_Test/Buffer/BufferTest.cpp index 04084a20..a5312565 100644 --- a/MobileGL/MG_Test/Buffer/BufferTest.cpp +++ b/MobileGL/MG_Test/Buffer/BufferTest.cpp @@ -1610,3 +1610,188 @@ TEST_F(GeneralBufferTest, General_CoherentAsFlush_PersistentMapAdoptsZeroCopyBac EXPECT_EQ(GetError(), GL_NO_ERROR); g_zeroCopyMock = nullptr; } + +// A buffer whose bytes the backend adopted into its own GPU memory - which is what +// EnsureGpuResidentStorage does for a transform-feedback capture target or a shader +// storage binding, so that MapBuffer/GetBufferSubData read real GPU results - and which +// the application then REDEFINES. +// +// The store the adopted mapping describes is the one being thrown away. Keeping that +// mapping across the redefinition is what let a transform feedback capture be written to +// one buffer and read back out of another: the backend replaced the storage (a +// respecification is the orphaning point) while the frontend went on resolving every read +// through a mapping of the storage it had just released. Two capture spans into one +// re-specified buffer came back empty from the second one onwards. +// +// So the mapping is handed back and the buffer returns to the CPU-shadow model until +// something asks for residency again. These pin all three parts of that: the adoption +// really is dropped, the new contents really do land where later reads resolve, and the +// backend really is told to respecify - it must not skip the storage, or its copy would +// keep the old bytes. +TEST_F(BufferTest, RedefiningAnAdoptedBufferHandsTheMappingBack) { + ZeroCopyMockBackend mock; + g_zeroCopyMock = &mock; + ScopedBackendOps scopedOps(&kZeroCopyMockOps); + + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(GL_ARRAY_BUFFER, buffer); + const GLint before[4] = {1, 2, 3, 4}; + BufferData(GL_ARRAY_BUFFER, sizeof(before), before, GL_DYNAMIC_DRAW); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + // The backend adopts the bytes, exactly as a capture target or an SSBO binding does. + ASSERT_TRUE(bufferObject->EnsureGpuResidentStorage()); + ASSERT_TRUE(bufferObject->IsBackendPersistentMapped()); + ASSERT_EQ(static_cast(bufferObject->MappedData()), + static_cast(mock.gpu.data())); + mock.respecifyCalls = 0; + + const GLint after[4] = {10, 20, 30, 40}; + BufferData(GL_ARRAY_BUFFER, sizeof(after), after, GL_DYNAMIC_DRAW); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + EXPECT_FALSE(bufferObject->IsBackendPersistentMapped()); + EXPECT_NE(static_cast(bufferObject->MappedData()), + static_cast(mock.gpu.data())); + EXPECT_EQ(std::memcmp(bufferObject->MappedData(), after, sizeof(after)), 0); + // The backend has a separate copy again, so it must have been told to refresh it. + EXPECT_EQ(mock.respecifyCalls, 1); + + g_zeroCopyMock = nullptr; +} + +// The same redefinition at a LARGER size, which is the case nothing could paper over: the +// adopted mapping is exactly as big as the old store, so writing the new contents through +// it ran past the end of the backend allocation. +TEST_F(BufferTest, RedefiningAnAdoptedBufferAtANewSizeStaysInBounds) { + ZeroCopyMockBackend mock; + g_zeroCopyMock = &mock; + ScopedBackendOps scopedOps(&kZeroCopyMockOps); + + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(GL_ARRAY_BUFFER, buffer); + const GLint small[2] = {1, 2}; + BufferData(GL_ARRAY_BUFFER, sizeof(small), small, GL_DYNAMIC_DRAW); + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + ASSERT_TRUE(bufferObject->EnsureGpuResidentStorage()); + ASSERT_EQ(mock.gpu.size(), sizeof(small)); + + const GLint large[8] = {1, 2, 3, 4, 5, 6, 7, 8}; + BufferData(GL_ARRAY_BUFFER, sizeof(large), large, GL_DYNAMIC_DRAW); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + EXPECT_EQ(bufferObject->GetSize(), sizeof(large)); + EXPECT_FALSE(bufferObject->IsBackendPersistentMapped()); + EXPECT_EQ(std::memcmp(bufferObject->MappedData(), large, sizeof(large)), 0); + // The old, smaller GPU block was not written through: still the old size, still the + // old bytes. + EXPECT_EQ(mock.gpu.size(), sizeof(small)); + EXPECT_EQ(std::memcmp(mock.gpu.data(), small, sizeof(small)), 0); + + // And residency can be taken again, now over the new store. + ASSERT_TRUE(bufferObject->EnsureGpuResidentStorage()); + EXPECT_TRUE(bufferObject->IsBackendPersistentMapped()); + EXPECT_EQ(mock.gpu.size(), sizeof(large)); + EXPECT_EQ(std::memcmp(bufferObject->MappedData(), large, sizeof(large)), 0); + + g_zeroCopyMock = nullptr; +} + +// glBufferStorage is the other way into a redefinition, and an adopted buffer can reach +// it: the adoption came from a binding rather than from an application map, so the buffer +// is still mutable and glBufferStorage is still legal on it. +TEST_F(BufferTest, ImmutableStorageOnAnAdoptedBufferHandsTheMappingBackToo) { + ZeroCopyMockBackend mock; + g_zeroCopyMock = &mock; + ScopedBackendOps scopedOps(&kZeroCopyMockOps); + + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(GL_ARRAY_BUFFER, buffer); + const GLint before[4] = {1, 2, 3, 4}; + BufferData(GL_ARRAY_BUFFER, sizeof(before), before, GL_DYNAMIC_DRAW); + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + ASSERT_TRUE(bufferObject->EnsureGpuResidentStorage()); + ASSERT_TRUE(bufferObject->IsBackendPersistentMapped()); + + const GLint after[6] = {9, 8, 7, 6, 5, 4}; + BufferStorage(GL_ARRAY_BUFFER, sizeof(after), after, GL_MAP_READ_BIT); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + EXPECT_TRUE(bufferObject->IsImmutableStorage()); + EXPECT_FALSE(bufferObject->IsBackendPersistentMapped()); + EXPECT_EQ(bufferObject->GetSize(), sizeof(after)); + EXPECT_EQ(std::memcmp(bufferObject->MappedData(), after, sizeof(after)), 0); + + g_zeroCopyMock = nullptr; +} + +// A redefinition to nothing. The backend declines residency for an empty store, so this +// is also the path where the mapping is given back and never retaken. +TEST_F(BufferTest, RedefiningAnAdoptedBufferToZeroBytesLeavesItOnTheShadow) { + ZeroCopyMockBackend mock; + g_zeroCopyMock = &mock; + ScopedBackendOps scopedOps(&kZeroCopyMockOps); + + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(GL_ARRAY_BUFFER, buffer); + const GLint before[4] = {1, 2, 3, 4}; + BufferData(GL_ARRAY_BUFFER, sizeof(before), before, GL_DYNAMIC_DRAW); + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + ASSERT_TRUE(bufferObject->EnsureGpuResidentStorage()); + ASSERT_TRUE(bufferObject->IsBackendPersistentMapped()); + + BufferData(GL_ARRAY_BUFFER, 0, nullptr, GL_DYNAMIC_DRAW); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + EXPECT_EQ(bufferObject->GetSize(), 0u); + EXPECT_FALSE(bufferObject->IsBackendPersistentMapped()); + EXPECT_FALSE(bufferObject->EnsureGpuResidentStorage()); // nothing to make resident + + // ...and it comes back to life on the next non-empty store. + const GLint again[3] = {5, 6, 7}; + BufferData(GL_ARRAY_BUFFER, sizeof(again), again, GL_DYNAMIC_DRAW); + ASSERT_EQ(GetError(), GL_NO_ERROR); + EXPECT_TRUE(bufferObject->EnsureGpuResidentStorage()); + EXPECT_EQ(std::memcmp(bufferObject->MappedData(), again, sizeof(again)), 0); + + g_zeroCopyMock = nullptr; +} + +// The negative control for the four above: a backend that DECLINES to hand out a mapping +// leaves the buffer shadow-backed throughout, so a redefinition is just a redefinition - +// no adoption to give back, and the backend still gets its Respecify. +TEST_F(BufferTest, RedefiningANonAdoptedBufferIsUnchanged) { + ZeroCopyMockBackend mock; + mock.provideMap = false; + g_zeroCopyMock = &mock; + ScopedBackendOps scopedOps(&kZeroCopyMockOps); + + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(GL_ARRAY_BUFFER, buffer); + const GLint before[4] = {1, 2, 3, 4}; + BufferData(GL_ARRAY_BUFFER, sizeof(before), before, GL_DYNAMIC_DRAW); + auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer); + ASSERT_NE(bufferObject, nullptr); + EXPECT_FALSE(bufferObject->EnsureGpuResidentStorage()); + EXPECT_FALSE(bufferObject->IsBackendPersistentMapped()); + mock.respecifyCalls = 0; + + const GLint after[4] = {10, 20, 30, 40}; + BufferData(GL_ARRAY_BUFFER, sizeof(after), after, GL_DYNAMIC_DRAW); + ASSERT_EQ(GetError(), GL_NO_ERROR); + EXPECT_FALSE(bufferObject->IsBackendPersistentMapped()); + EXPECT_EQ(std::memcmp(bufferObject->MappedData(), after, sizeof(after)), 0); + EXPECT_EQ(mock.respecifyCalls, 1); + + g_zeroCopyMock = nullptr; +} diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index e215c11e..fc07caf3 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -1572,6 +1573,378 @@ TEST_F(TextureTest, CompressedInternalFormatsResolveToTheirUncompressedStorage) } } +// Resolving to uncompressed storage is a storage decision, not a licence to answer the level +// queries as if the application had asked for an uncompressed format. GL 4.6 core 8.5 lets the +// implementation choose for the GENERIC formats (GL_COMPRESSED_RED and friends), but a SPECIFIC +// one commits the level: GL_TEXTURE_COMPRESSED is true, GL_TEXTURE_INTERNAL_FORMAT is the token +// that was passed, and GL_TEXTURE_COMPRESSED_IMAGE_SIZE answers instead of erroring - which is +// exactly the three-query sequence KHR-GL44.buffer_storage.map_persistent_texture opens with to +// size the image it then uploads through glCompressedTexSubImage2D. +TEST_F(TextureTest, ASpecificCompressedInternalFormatTagsTheLevelCompressed) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_COMPRESSED_RED_RGTC1, 8, 8, 0, GL_RED, GL_UNSIGNED_BYTE, nullptr); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + GLint compressed = GL_FALSE; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_COMPRESSED, &compressed); + EXPECT_EQ(compressed, GL_TRUE); + + GLint internalFormat = 0; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_INTERNAL_FORMAT, &internalFormat); + EXPECT_EQ(internalFormat, static_cast(GL_COMPRESSED_RED_RGTC1)); + + // 8x8 in 4x4 blocks of 8 bytes each. + GLint imageSize = 0; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_COMPRESSED_IMAGE_SIZE, &imageSize); + EXPECT_EQ(imageSize, 32); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + // The texel shadow behind the tag still carries the uncompressed storage the format resolves + // to - which is what lets the level sample, and what every size computation downstream + // divides by. + const auto textureObject = MG_State::pGLContext->GetTextureObject(texture); + ASSERT_NE(textureObject, nullptr); + EXPECT_EQ(textureObject->GetFormat(), TextureInternalFormat::R8); +} + +// The negative control for the case above, and the reason it cannot simply tag every +// GL_COMPRESSED_* token: for a generic format the implementation's choice IS the answer, and +// MobileGL chooses uncompressed - so the level is not compressed and the size query is the +// INVALID_OPERATION GL 4.6 core 8.11 prescribes for an uncompressed image. +TEST_F(TextureTest, AGenericCompressedInternalFormatLeavesTheLevelUncompressed) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_COMPRESSED_RED, 8, 8, 0, GL_RED, GL_UNSIGNED_BYTE, nullptr); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + GLint compressed = GL_TRUE; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_COMPRESSED, &compressed); + EXPECT_EQ(compressed, GL_FALSE); + + GLint internalFormat = 0; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_INTERNAL_FORMAT, &internalFormat); + EXPECT_EQ(internalFormat, static_cast(GL_R8)); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + GLint imageSize = 0; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_COMPRESSED_IMAGE_SIZE, &imageSize); + ExpectSingleGlError(GL_INVALID_OPERATION); +} + +// A plain glTexImage2D over a level that was tagged compressed has to un-tag it, the same way it +// does for a level a glCompressedTexImage2D shadowed - otherwise the size query would keep +// answering for an image that no longer exists. +TEST_F(TextureTest, AnUncompressedRespecificationClearsTheCompressedTag) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_COMPRESSED_RED_RGTC1, 8, 8, 0, GL_RED, GL_UNSIGNED_BYTE, nullptr); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_R8, 8, 8, 0, GL_RED, GL_UNSIGNED_BYTE, nullptr); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + GLint compressed = GL_TRUE; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_COMPRESSED, &compressed); + EXPECT_EQ(compressed, GL_FALSE); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + +namespace { + // 8x8 RGTC1: 2x2 blocks of 8 bytes, so the stored image is 32 bytes and one block row is 16. + constexpr GLsizei kRgtc1Size8x8 = 32; + + GLuint MakeCompressedRgtc1Texture8x8() { + 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); + return texture; + } +} // namespace + +// glCompressedTexSubImage2D was a stub that answered GL_INVALID_ENUM to every call. It replaces a +// block-aligned rectangle of the stored image, and the arithmetic that places the incoming blocks +// is what the partial write below pins: a full-width write would pass with the rows concatenated +// in either order. +TEST_F(TextureTest, CompressedTexSubImage2DReplacesTheStoredBlocks) { + const GLuint texture = MakeCompressedRgtc1Texture8x8(); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 whole[kRgtc1Size8x8]; + for (Int i = 0; i < kRgtc1Size8x8; ++i) whole[i] = static_cast(i + 1); + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8, + whole); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 stored[kRgtc1Size8x8] = {}; + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, whole, sizeof(whole)), 0); + + // The right-hand block column only: one block wide, two block rows high. Its two blocks land + // at byte 8 and byte 24, not at bytes 0 and 8. + const Uint8 column[16] = {0xA0, 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7, + 0xB0, 0xB1, 0xB2, 0xB3, 0xB4, 0xB5, 0xB6, 0xB7}; + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 4, 0, 4, 8, GL_COMPRESSED_RED_RGTC1, + static_cast(sizeof(column)), column); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 expected[kRgtc1Size8x8]; + std::memcpy(expected, whole, sizeof(expected)); + std::memcpy(expected + 8, column, 8); + std::memcpy(expected + 24, column + 8, 8); + + std::memset(stored, 0, sizeof(stored)); + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, expected, sizeof(expected)), 0); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + +// The Y axis of the placement, which the whole-image and single-column cases above cannot see: an +// implementation that dropped the first-block-row term, or that divided yoffset by the block WIDTH, +// passes every one of them. The region here starts at block row 1, so its two blocks belong at +// bytes 16 and 24 and nowhere else. +TEST_F(TextureTest, CompressedTexSubImage2DPlacesTheFirstBlockRow) { + const GLuint texture = MakeCompressedRgtc1Texture8x8(); + Uint8 whole[kRgtc1Size8x8]; + for (Int i = 0; i < kRgtc1Size8x8; ++i) whole[i] = static_cast(i + 1); + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8, + whole); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + // The bottom block row only: 8 texels wide, 4 high, starting at y = 4. + const Uint8 bottom[16] = {0xC0, 0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7, + 0xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, 0xD7}; + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 4, 8, 4, GL_COMPRESSED_RED_RGTC1, + static_cast(sizeof(bottom)), bottom); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 expected[kRgtc1Size8x8]; + std::memcpy(expected, whole, sizeof(expected)); + std::memcpy(expected + 16, bottom, sizeof(bottom)); + + Uint8 stored[kRgtc1Size8x8] = {}; + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, expected, sizeof(expected)), 0); + + // And one block in the far corner, which needs both terms at once. + const Uint8 corner[8] = {0xE0, 0xE1, 0xE2, 0xE3, 0xE4, 0xE5, 0xE6, 0xE7}; + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 4, 4, 4, 4, GL_COMPRESSED_RED_RGTC1, + static_cast(sizeof(corner)), corner); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + std::memcpy(expected + 24, corner, sizeof(corner)); + std::memset(stored, 0, sizeof(stored)); + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, expected, sizeof(expected)), 0); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + (void)texture; +} + +// A level whose size is neither square nor a multiple of the block size, at a level above the +// base, in a format with SIXTEEN bytes per block. Between them these pin the row stride (which a +// square level cannot distinguish from the column count), the rounding-up of a partial edge block, +// the run-to-the-edge exemption from the whole-blocks rule, and the block size actually coming from +// the format rather than from a constant. +TEST_F(TextureTest, CompressedTexSubImage2DHandlesPartialBlocksAndAMipLevel) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + // 6x10 BPTC: 2 block columns x 3 block rows of 16 bytes = 96, one block row = 32. + constexpr GLsizei kBptcSize6x10 = 96; + MG_Impl::GLImpl::CompressedTexImage2D(GL_TEXTURE_2D, 1, GL_COMPRESSED_RGBA_BPTC_UNORM, 6, 10, 0, kBptcSize6x10, + nullptr); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + GLint imageSize = 0; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 1, GL_TEXTURE_COMPRESSED_IMAGE_SIZE, &imageSize); + EXPECT_EQ(imageSize, kBptcSize6x10); + + Uint8 whole[kBptcSize6x10]; + for (Int i = 0; i < kBptcSize6x10; ++i) whole[i] = static_cast(i + 1); + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 1, 0, 0, 6, 10, GL_COMPRESSED_RGBA_BPTC_UNORM, + kBptcSize6x10, whole); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + // The right-hand column (2 texels wide - a partial block that runs to the edge) of the middle + // block row: one block, at byte 32 + 16. + Uint8 patch[16]; + for (Int i = 0; i < 16; ++i) patch[i] = static_cast(0xF0 + i); + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 1, 4, 4, 2, 4, GL_COMPRESSED_RGBA_BPTC_UNORM, + static_cast(sizeof(patch)), patch); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 expected[kBptcSize6x10]; + std::memcpy(expected, whole, sizeof(expected)); + std::memcpy(expected + 48, patch, sizeof(patch)); + + Uint8 stored[kBptcSize6x10] = {}; + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 1, stored); + EXPECT_EQ(std::memcmp(stored, expected, sizeof(expected)), 0); + + // The partial edge block is only exempt from the whole-blocks rule AT the edge: the same + // 2-texel width one block to the left is not. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 1, 0, 4, 2, 4, GL_COMPRESSED_RGBA_BPTC_UNORM, + static_cast(sizeof(patch)), patch); + ExpectSingleGlError(GL_INVALID_OPERATION); +} + +// The same call sourcing its blocks from a buffer bound to GL_PIXEL_UNPACK_BUFFER, where `data` is +// an offset into that buffer rather than a client pointer - which is the form +// KHR-GL44.buffer_storage.map_persistent_texture uses for every one of its operations. +TEST_F(TextureTest, CompressedTexSubImage2DUnpacksFromAPixelUnpackBuffer) { + Uint8 source[256]; + for (Int i = 0; i < 256; ++i) source[i] = static_cast(i); + GLuint buffer = 0; + MG_Impl::GLImpl::GenBuffers(1, &buffer); + MG_Impl::GLImpl::BindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer); + MG_Impl::GLImpl::BufferData(GL_PIXEL_UNPACK_BUFFER, sizeof(source), source, GL_STATIC_DRAW); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + const GLuint texture = MakeCompressedRgtc1Texture8x8(); + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8, + reinterpret_cast(static_cast(64))); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 stored[kRgtc1Size8x8] = {}; + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, source + 64, sizeof(stored)), 0); + + // Reading past the end of the buffer is the unpack-buffer error, not a read out of bounds. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8, + reinterpret_cast(static_cast(sizeof(source) - 8))); + ExpectSingleGlError(GL_INVALID_OPERATION); + + MG_Impl::GLImpl::BindBuffer(GL_PIXEL_UNPACK_BUFFER, 0); + (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(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); + // The image call takes offset 0 and the sub-image call offset 128, so the readback can only + // match if the SUB-IMAGE call ran: were it refused (or a no-op), the level would still hold + // the image call's bytes. + MG_Impl::GLImpl::CompressedTexImage2D(GL_TEXTURE_2D, 0, GL_COMPRESSED_RED_RGTC1, 8, 8, 0, kRgtc1Size8x8, + reinterpret_cast(static_cast(0))); + 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(static_cast(128))); + 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 + 128, 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(static_cast(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. +TEST_F(TextureTest, CompressedTextureSubImage2DModifiesTheNamedTextureOnly) { + const GLuint bound = MakeCompressedRgtc1Texture8x8(); + Uint8 boundImage[kRgtc1Size8x8]; + for (Int i = 0; i < kRgtc1Size8x8; ++i) boundImage[i] = 0x11; + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8, + boundImage); + + const GLuint named = MakeCompressedRgtc1Texture8x8(); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, bound); // `named` is NOT the bound texture + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 namedImage[kRgtc1Size8x8]; + for (Int i = 0; i < kRgtc1Size8x8; ++i) namedImage[i] = 0x22; + MG_Impl::GLImpl::CompressedTextureSubImage2D(named, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8, + namedImage); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + // The borrowed binding is back, and it kept its own image. + Uint8 stored[kRgtc1Size8x8] = {}; + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, boundImage, sizeof(stored)), 0); + + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, named); + std::memset(stored, 0, sizeof(stored)); + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, namedImage, sizeof(stored)), 0); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + +TEST_F(TextureTest, CompressedTexSubImage2DRejectsTheRegionsGLForbids) { + const GLuint texture = MakeCompressedRgtc1Texture8x8(); + Uint8 blocks[kRgtc1Size8x8] = {}; + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + // A format that is not the one the image is stored in. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RG_RGTC2, 64, blocks); + ExpectSingleGlError(GL_INVALID_OPERATION); + + // A start that is not on a block boundary. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 2, 0, 4, 8, GL_COMPRESSED_RED_RGTC1, 16, blocks); + ExpectSingleGlError(GL_INVALID_OPERATION); + + // A width that is neither a whole number of blocks nor a run to the image's edge. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 2, 8, GL_COMPRESSED_RED_RGTC1, 16, blocks); + ExpectSingleGlError(GL_INVALID_OPERATION); + + // A region that runs off the image. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 4, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, 32, blocks); + ExpectSingleGlError(GL_INVALID_VALUE); + + // An imageSize that does not match the region. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, 16, blocks); + ExpectSingleGlError(GL_INVALID_VALUE); + + // A format with no defined block layout here. + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_RGBA8, 32, blocks); + ExpectSingleGlError(GL_INVALID_ENUM); + + // An uncompressed image has nothing for it to replace. + GLuint plain = 0; + MG_Impl::GLImpl::GenTextures(1, &plain); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, plain); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_R8, 8, 8, 0, GL_RED, GL_UNSIGNED_BYTE, 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, + blocks); + ExpectSingleGlError(GL_INVALID_OPERATION); + (void)texture; +} + // RGTC compresses 4x4 blocks of a 2D image and has no 3D form, so glTexImage3D must reject it even // though the same enum is accepted on a 2D target. The generic compressed formats carry no such // restriction and stay legal in 3D. @@ -3288,3 +3661,57 @@ TEST_F(TextureTest, GetTexLevelParameterOnBufferStorageReportsErrorInsteadOfTerm ExpectSingleGlError(GL_INVALID_OPERATION); } } + +// Immutable storage plus glCompressedTexSubImage2D is the modern way to upload a compressed +// texture, so glTexStorage2D has to commit its levels to a specific compressed internalformat +// exactly as glTexImage2D does. When it did not, the sub-image call found an uncompressed level +// and refused it, and glTexImage2D and glTexStorage2D disagreed about the same token. +TEST_F(TextureTest, TexStorage2DTagsEveryLevelForASpecificCompressedFormat) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + MG_Impl::GLImpl::TexStorage2D(GL_TEXTURE_2D, 2, GL_COMPRESSED_RED_RGTC1, 8, 8); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + for (GLint level = 0; level < 2; ++level) { + GLint compressed = GL_FALSE; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, level, GL_TEXTURE_COMPRESSED, &compressed); + EXPECT_EQ(compressed, GL_TRUE) << "level " << level; + GLint internalFormat = 0; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, level, GL_TEXTURE_INTERNAL_FORMAT, &internalFormat); + EXPECT_EQ(internalFormat, static_cast(GL_COMPRESSED_RED_RGTC1)) << "level " << level; + GLint imageSize = 0; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, level, GL_TEXTURE_COMPRESSED_IMAGE_SIZE, &imageSize); + // 8x8 -> 2x2 blocks -> 32 bytes; 4x4 -> 1 block -> 8 bytes. + EXPECT_EQ(imageSize, level == 0 ? 32 : 8) << "level " << level; + } + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + // ...and the sub-image call the whole arrangement exists for now reaches both levels. + Uint8 blocks[kRgtc1Size8x8]; + for (Int i = 0; i < kRgtc1Size8x8; ++i) blocks[i] = static_cast(0x40 + i); + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 8, 8, GL_COMPRESSED_RED_RGTC1, kRgtc1Size8x8, + blocks); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + Uint8 stored[kRgtc1Size8x8] = {}; + MG_Impl::GLImpl::GetCompressedTexImage(GL_TEXTURE_2D, 0, stored); + EXPECT_EQ(std::memcmp(stored, blocks, sizeof(stored)), 0); + + MG_Impl::GLImpl::CompressedTexSubImage2D(GL_TEXTURE_2D, 1, 0, 0, 4, 4, GL_COMPRESSED_RED_RGTC1, 8, blocks); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + +// The negative control: a generic compressed token leaves glTexStorage2D's levels uncompressed, +// because for those the implementation's choice IS the answer and MobileGL chooses uncompressed. +TEST_F(TextureTest, TexStorage2DLeavesAGenericCompressedFormatUncompressed) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + MG_Impl::GLImpl::TexStorage2D(GL_TEXTURE_2D, 1, GL_COMPRESSED_RED, 8, 8); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + GLint compressed = GL_TRUE; + MG_Impl::GLImpl::GetTexLevelParameteriv(GL_TEXTURE_2D, 0, GL_TEXTURE_COMPRESSED, &compressed); + EXPECT_EQ(compressed, GL_FALSE); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +}