From f39e6eb82d7d50c4d3c6be0ad09efbe50026c234 Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Tue, 4 Aug 2026 20:33:14 -0400 Subject: [PATCH] [Feat] (MG_Impl): implement glReadnPixels It was exported as a stub: it logged a warning and returned, leaving the caller's buffer untouched. Anything reading back through it saw whatever the destination already held, which for a freshly allocated vector is zeros -- so every direct_state_access texture test comparing a readback against reference data failed without a GL error to explain it. glReadnPixels is glReadPixels with a bound on how much it may write (GL 4.6 core 18.2.8, originally GL_ARB_robustness) and is identical in every other respect, so it validates and reads through exactly the same path once the destination is known to be big enough. Sizing the read honours the GL_PACK_* state: rows are padded to GL_PACK_ALIGNMENT and laid out GL_PACK_ROW_LENGTH wide, with the skip parameters offsetting the first texel. The last row is deliberately not padded -- nothing follows it to align -- which is what makes a tightly-sized destination legal. --- .../MG_Impl/GLImpl/Exporting/Definitions.cpp | 2 +- .../GLImpl/Framebuffer/GL_Framebuffer.cpp | 50 +++++++++++++++++++ .../GLImpl/Framebuffer/GL_Framebuffer.h | 2 + 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index 1732fbbe..f8dfe687 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -419,7 +419,7 @@ DECLARE_GL_FUNCTION_HEAD(void, DrawElementsInstancedBaseVertex, GLenum mode, GLs DECLARE_GL_FUNCTION_HEAD(void, FramebufferTexture, GLenum target, GLenum attachment, GLuint texture, GLint level) DECLARE_GL_FUNCTION_END_NO_RETURN(void, FramebufferTexture, target, attachment, texture, level) DECLARE_GL_FUNCTION_STUB_HEAD(void, PrimitiveBoundingBox, GLfloat minX, GLfloat minY, GLfloat minZ, GLfloat minW, GLfloat maxX, GLfloat maxY, GLfloat maxZ, GLfloat maxW) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, PrimitiveBoundingBox, minX, minY, minZ, minW, maxX, maxY, maxZ, maxW) DECLARE_GL_FUNCTION_HEAD(GLenum, GetGraphicsResetStatus) DECLARE_GL_FUNCTION_END(GLenum, GetGraphicsResetStatus) -DECLARE_GL_FUNCTION_STUB_HEAD(void, ReadnPixels, GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, GLsizei bufSize, void* data) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, ReadnPixels, x, y, width, height, format, type, bufSize, data) +DECLARE_GL_FUNCTION_HEAD(void, ReadnPixels, GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, GLsizei bufSize, void* data) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ReadnPixels, x, y, width, height, format, type, bufSize, data) DECLARE_GL_FUNCTION_STUB_HEAD(void, GetnUniformfv, GLuint program, GLint location, GLsizei bufSize, GLfloat* params) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetnUniformfv, program, location, bufSize, params) DECLARE_GL_FUNCTION_STUB_HEAD(void, GetnUniformiv, GLuint program, GLint location, GLsizei bufSize, GLint* params) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetnUniformiv, program, location, bufSize, params) DECLARE_GL_FUNCTION_STUB_HEAD(void, GetnUniformuiv, GLuint program, GLint location, GLsizei bufSize, GLuint* params) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetnUniformuiv, program, location, bufSize, params) diff --git a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp index da6dc6c9..2f9c8632 100644 --- a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp @@ -2199,6 +2199,56 @@ namespace MobileGL::MG_Impl::GLImpl { ReadPixels_Backend(x, y, width, height, format, type, pixels); } + // Bytes glReadPixels would write for this rectangle under the current GL_PACK_* state + // (GL 4.6 core 18.2.8): rows are padded to GL_PACK_ALIGNMENT and laid out GL_PACK_ROW_LENGTH + // wide, and the skip parameters offset the first texel. The last row is not padded - nothing + // follows it to align - which is what makes a tightly-sized destination legal. + static SizeT ComputePackedReadSizeInBytes(GLsizei width, GLsizei height, GLenum format, GLenum type) { + const SizeT bytesPerPixel = + MG_Util::GetInputBytesPerPixel(MG_Util::ConvertGLEnumToTextureInputFormat(format), + MG_Util::ConvertGLEnumToTexturePixelDataType(type)); + if (bytesPerPixel == 0 || width <= 0 || height <= 0) return 0; + + const auto packParam = [](PixelStoreParam param) { + return static_cast(std::max(0, MG_State::pGLContext->GetPixelStoreParam(param))); + }; + const SizeT rowLengthInPixels = + packParam(PixelStoreParam::PackRowLength) != 0 + ? packParam(PixelStoreParam::PackRowLength) + : static_cast(width); + const SizeT alignment = std::max(1, packParam(PixelStoreParam::PackAlignment)); + + const SizeT unalignedRowBytes = rowLengthInPixels * bytesPerPixel; + const SizeT paddedRowBytes = ((unalignedRowBytes + alignment - 1) / alignment) * alignment; + const SizeT skipBytes = packParam(PixelStoreParam::PackSkipRows) * paddedRowBytes + + packParam(PixelStoreParam::PackSkipPixels) * bytesPerPixel; + + return skipBytes + paddedRowBytes * (static_cast(height) - 1) + + static_cast(width) * bytesPerPixel; + } + + // glReadnPixels is glReadPixels with a bound on how much it may write (GL 4.6 core 18.2.8, + // originally GL_ARB_robustness). It is identical in every other respect, so it validates and + // reads through exactly the same path once the destination is known to be big enough. + void ReadnPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, GLsizei bufSize, + void* data) { + if (bufSize < 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", __func__, "bufSize must be non-negative.")); + return; + } + if (!ReadPixels_State(x, y, width, height, format, type, data)) return; + if (ComputePackedReadSizeInBytes(width, height, format, type) > static_cast(bufSize)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", __func__, + "the data required for this read does not fit in bufSize.")); + return; + } + ReadPixels_Backend(x, y, width, height, format, type, data); + } + void ClearBufferfi(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) { if (!ValidateClearBufferfi_State(buffer, drawbuffer)) return; ClearBufferfi_Backend(buffer, drawbuffer, depth, stencil); diff --git a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h index 8e843d49..33b58f26 100644 --- a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h +++ b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h @@ -14,6 +14,8 @@ namespace MobileGL::MG_Impl::GLImpl { /* @INSERTION_POINT:FUNCTION_DECLARATION@ */ void ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels); + void ReadnPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, GLsizei bufSize, + void* data); void ClearBufferfi(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil); void ClearBufferfv(GLenum buffer, GLint drawbuffer, const GLfloat* value); void ClearBufferuiv(GLenum buffer, GLint drawbuffer, const GLuint* value);