From 66ac3486e1809a3ae0c61b7badfb5e9f65384704 Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Wed, 5 Aug 2026 00:31:46 -0400 Subject: [PATCH] [Feat] (MG_Impl): validate the framebuffer invalidation entry points glInvalidateFramebuffer, glInvalidateSubFramebuffer and their two by-name forms were all stubs, so every call - including the malformed ones - returned quietly with no error. These four only grant permission to throw the named attachments' contents away, and keeping them satisfies "the contents become undefined", so the frontend validates the call and leaves the contents alone. Actually discarding is a bandwidth optimisation that would need a backend dependency; it can be added later without changing what any of these promise. The validation is where the real content is. Which tokens name an attachment depends on which framebuffer is affected: the default framebuffer has buffers (FRONT_LEFT and company) and a framebuffer object has attachment points, so a token from the wrong set is INVALID_ENUM. A COLOR_ATTACHMENTm past GL_MAX_COLOR_ATTACHMENTS is different in kind - a well-formed enum naming a point that does not exist - and is INVALID_OPERATION, which the existing colour-attachment range validator already expresses. Negative counts and negative sub-region extents are INVALID_VALUE. direct_state_access.invalidate_data_and_subdata_errors passes on both backends. --- .../MG_Impl/GLImpl/Exporting/Definitions.cpp | 8 +- .../GLImpl/Framebuffer/GL_Framebuffer.cpp | 150 ++++++++++++++++++ .../GLImpl/Framebuffer/GL_Framebuffer.h | 6 + 3 files changed, 160 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index 97798a54..4ef984f6 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -301,8 +301,8 @@ DECLARE_GL_FUNCTION_HEAD(void, ResumeTransformFeedback) DECLARE_GL_FUNCTION_END_ DECLARE_GL_FUNCTION_HEAD(void, GetProgramBinary, GLuint program, GLsizei bufSize, GLsizei* length, GLenum* binaryFormat, void* binary) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetProgramBinary, program, bufSize, length, binaryFormat, binary) DECLARE_GL_FUNCTION_HEAD(void, ProgramBinary, GLuint program, GLenum binaryFormat, const void* binary, GLsizei length) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ProgramBinary, program, binaryFormat, binary, length) DECLARE_GL_FUNCTION_HEAD(void, ProgramParameteri, GLuint program, GLenum pname, GLint value) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ProgramParameteri, program, pname, value) -DECLARE_GL_FUNCTION_STUB_HEAD(void, InvalidateFramebuffer, GLenum target, GLsizei numAttachments, const GLenum* attachments) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, InvalidateFramebuffer, target, numAttachments, attachments) -DECLARE_GL_FUNCTION_STUB_HEAD(void, InvalidateSubFramebuffer, GLenum target, GLsizei numAttachments, const GLenum* attachments, GLint x, GLint y, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, InvalidateSubFramebuffer, target, numAttachments, attachments, x, y, width, height) +DECLARE_GL_FUNCTION_HEAD(void, InvalidateFramebuffer, GLenum target, GLsizei numAttachments, const GLenum* attachments) DECLARE_GL_FUNCTION_END_NO_RETURN(void, InvalidateFramebuffer, target, numAttachments, attachments) +DECLARE_GL_FUNCTION_HEAD(void, InvalidateSubFramebuffer, GLenum target, GLsizei numAttachments, const GLenum* attachments, GLint x, GLint y, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_END_NO_RETURN(void, InvalidateSubFramebuffer, target, numAttachments, attachments, x, y, width, height) DECLARE_GL_FUNCTION_HEAD(void, TexStorage2D, GLenum target, GLsizei levels, GLenum internalformat, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TexStorage2D, target, levels, internalformat, width, height) DECLARE_GL_FUNCTION_HEAD(void, TexStorage3D, GLenum target, GLsizei levels, GLenum internalformat, GLsizei width, GLsizei height, GLsizei depth) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TexStorage3D, target, levels, internalformat, width, height, depth) DECLARE_GL_FUNCTION_HEAD(void, GetInternalformativ, GLenum target, GLenum internalformat, GLenum pname, GLsizei bufSize, GLint* params) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetInternalformativ, target, internalformat, pname, bufSize, params) @@ -1034,8 +1034,8 @@ DECLARE_GL_FUNCTION_HEAD(void, NamedFramebufferTextureLayer, GLuint framebuffer, DECLARE_GL_FUNCTION_HEAD(void, NamedFramebufferDrawBuffer, GLuint framebuffer, GLenum buf) DECLARE_GL_FUNCTION_END_NO_RETURN(void, NamedFramebufferDrawBuffer, framebuffer, buf) DECLARE_GL_FUNCTION_HEAD(void, NamedFramebufferDrawBuffers, GLuint framebuffer, GLsizei n, const GLenum* bufs) DECLARE_GL_FUNCTION_END_NO_RETURN(void, NamedFramebufferDrawBuffers, framebuffer, n, bufs) DECLARE_GL_FUNCTION_HEAD(void, NamedFramebufferReadBuffer, GLuint framebuffer, GLenum src) DECLARE_GL_FUNCTION_END_NO_RETURN(void, NamedFramebufferReadBuffer, framebuffer, src) -DECLARE_GL_FUNCTION_STUB_HEAD(void, InvalidateNamedFramebufferData, GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, InvalidateNamedFramebufferData, framebuffer, numAttachments, attachments) -DECLARE_GL_FUNCTION_STUB_HEAD(void, InvalidateNamedFramebufferSubData, GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments, GLint x, GLint y, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, InvalidateNamedFramebufferSubData, framebuffer, numAttachments, attachments, x, y, width, height) +DECLARE_GL_FUNCTION_HEAD(void, InvalidateNamedFramebufferData, GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments) DECLARE_GL_FUNCTION_END_NO_RETURN(void, InvalidateNamedFramebufferData, framebuffer, numAttachments, attachments) +DECLARE_GL_FUNCTION_HEAD(void, InvalidateNamedFramebufferSubData, GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments, GLint x, GLint y, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_END_NO_RETURN(void, InvalidateNamedFramebufferSubData, framebuffer, numAttachments, attachments, x, y, width, height) DECLARE_GL_FUNCTION_HEAD(void, ClearNamedFramebufferiv, GLuint framebuffer, GLenum buffer, GLint drawbuffer, const GLint* value) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ClearNamedFramebufferiv, framebuffer, buffer, drawbuffer, value) DECLARE_GL_FUNCTION_HEAD(void, ClearNamedFramebufferuiv, GLuint framebuffer, GLenum buffer, GLint drawbuffer, const GLuint* value) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ClearNamedFramebufferuiv, framebuffer, buffer, drawbuffer, value) DECLARE_GL_FUNCTION_HEAD(void, ClearNamedFramebufferfv, GLuint framebuffer, GLenum buffer, GLint drawbuffer, const GLfloat* value) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ClearNamedFramebufferfv, framebuffer, buffer, drawbuffer, value) diff --git a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp index ae6083a3..90bf9b44 100644 --- a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp @@ -1666,6 +1666,138 @@ namespace MobileGL::MG_Impl::GLImpl { ClearNamedFramebufferuiv_Backend(framebufferObject, buffer, drawbuffer, value); } + // glInvalidateFramebuffer and its three siblings only grant the implementation permission to + // throw the named attachments' contents away - "become undefined" is satisfied by keeping them + // - so MobileGL validates the call and leaves the contents alone. Discarding is a bandwidth + // optimisation that would need a backend dependency; it can be added later without changing + // what any of these entry points promise. The conformance tests exercise the validation, which + // is the part that was missing. + Bool ValidateInvalidateAttachments_State(const SharedPtr& framebuffer, + GLsizei numAttachments, const GLenum* attachments, const char* caller) { + if (numAttachments < 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", caller, "numAttachments cannot be negative.")); + return false; + } + if (numAttachments > 0 && attachments == nullptr) { + return false; + } + + // Which tokens name an attachment depends on which framebuffer is affected: the default one + // has buffers, a framebuffer object has attachment points (GL 4.6 core 9.2.7). + const Bool isDefaultFramebuffer = framebuffer->IsDefaultFramebuffer(); + for (GLsizei at = 0; at < numAttachments; ++at) { + const GLenum attachment = attachments[at]; + if (isDefaultFramebuffer) { + switch (attachment) { + case GL_FRONT_LEFT: + case GL_FRONT_RIGHT: + case GL_BACK_LEFT: + case GL_BACK_RIGHT: + case GL_DEPTH: + case GL_STENCIL: + continue; + default: + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique( + "MG_Impl/GLImpl", caller, + std::format("{} does not name a buffer of the default framebuffer.", + MG_Util::ConvertGLEnumToString(attachment)))); + return false; + } + } + + if (attachment == GL_DEPTH_ATTACHMENT || attachment == GL_STENCIL_ATTACHMENT || + attachment == GL_DEPTH_STENCIL_ATTACHMENT) { + continue; + } + if (attachment < GL_COLOR_ATTACHMENT0 || attachment > GL_COLOR_ATTACHMENT31) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique( + "MG_Impl/GLImpl", caller, + std::format("{} does not name an attachment point of a framebuffer object.", + MG_Util::ConvertGLEnumToString(attachment)))); + return false; + } + // A COLOR_ATTACHMENTm token past the limit is a well-formed enum naming a point that + // does not exist, which is INVALID_OPERATION rather than INVALID_ENUM. + const auto attachmentType = MG_Util::ConvertGLEnumToFramebufferAttachmentType(attachment); + if (!FramebufferImpl::ValidateColorAttachmentInRange(attachmentType, caller)) return false; + } + return true; + } + + Bool ValidateInvalidateSubRegion_State(GLsizei width, GLsizei height, const char* caller) { + if (width < 0 || height < 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", caller, "width and height cannot be negative.")); + return false; + } + return true; + } + + SharedPtr GetFramebufferObjectForInvalidate_State(GLenum target, + const char* caller) { + const FramebufferTarget framebufferTarget = + MG_Util::ConvertGLEnumToFramebufferTarget(target == GL_FRAMEBUFFER ? GL_DRAW_FRAMEBUFFER : target); + if (!FramebufferImpl::ValidateFramebufferTarget(framebufferTarget)) return nullptr; + auto framebufferObject = MG_State::pGLContext->GetFramebufferBindingSlot(framebufferTarget).GetBoundObject(); + if (!framebufferObject) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", caller, + "Framebuffer target is bound to no framebuffer object.")); + } + return framebufferObject; + } + + void InvalidateNamedFramebufferData_State(GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments) { + auto framebufferObject = + GetFramebufferObjectForNamedClear(framebuffer, "InvalidateNamedFramebufferData_State"); + if (!framebufferObject) return; + ValidateInvalidateAttachments_State(framebufferObject, numAttachments, attachments, + "InvalidateNamedFramebufferData_State"); + } + + void InvalidateNamedFramebufferSubData_State(GLuint framebuffer, GLsizei numAttachments, + const GLenum* attachments, GLint x, GLint y, GLsizei width, + GLsizei height) { + (void)x; + (void)y; + auto framebufferObject = + GetFramebufferObjectForNamedClear(framebuffer, "InvalidateNamedFramebufferSubData_State"); + if (!framebufferObject) return; + if (!ValidateInvalidateAttachments_State(framebufferObject, numAttachments, attachments, + "InvalidateNamedFramebufferSubData_State")) { + return; + } + ValidateInvalidateSubRegion_State(width, height, "InvalidateNamedFramebufferSubData_State"); + } + + void InvalidateFramebuffer_State(GLenum target, GLsizei numAttachments, const GLenum* attachments) { + auto framebufferObject = GetFramebufferObjectForInvalidate_State(target, "InvalidateFramebuffer_State"); + if (!framebufferObject) return; + ValidateInvalidateAttachments_State(framebufferObject, numAttachments, attachments, + "InvalidateFramebuffer_State"); + } + + void InvalidateSubFramebuffer_State(GLenum target, GLsizei numAttachments, const GLenum* attachments, GLint x, + GLint y, GLsizei width, GLsizei height) { + (void)x; + (void)y; + auto framebufferObject = GetFramebufferObjectForInvalidate_State(target, "InvalidateSubFramebuffer_State"); + if (!framebufferObject) return; + if (!ValidateInvalidateAttachments_State(framebufferObject, numAttachments, attachments, + "InvalidateSubFramebuffer_State")) { + return; + } + ValidateInvalidateSubRegion_State(width, height, "InvalidateSubFramebuffer_State"); + } + void DeleteRenderbuffers_State(GLsizei n, const GLuint* renderbuffers) { if (n < 0) { MG_State::pGLContext->RecordError( @@ -2353,6 +2485,24 @@ namespace MobileGL::MG_Impl::GLImpl { ClearBufferiv_Backend(buffer, drawbuffer, value); } + void InvalidateNamedFramebufferData(GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments) { + InvalidateNamedFramebufferData_State(framebuffer, numAttachments, attachments); + } + + void InvalidateNamedFramebufferSubData(GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments, + GLint x, GLint y, GLsizei width, GLsizei height) { + InvalidateNamedFramebufferSubData_State(framebuffer, numAttachments, attachments, x, y, width, height); + } + + void InvalidateFramebuffer(GLenum target, GLsizei numAttachments, const GLenum* attachments) { + InvalidateFramebuffer_State(target, numAttachments, attachments); + } + + void InvalidateSubFramebuffer(GLenum target, GLsizei numAttachments, const GLenum* attachments, GLint x, GLint y, + GLsizei width, GLsizei height) { + InvalidateSubFramebuffer_State(target, numAttachments, attachments, x, y, width, height); + } + void ClearNamedFramebufferiv(GLuint framebuffer, GLenum buffer, GLint drawbuffer, const GLint* value) { ClearNamedFramebufferiv_State(framebuffer, buffer, drawbuffer, value); } diff --git a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h index 4b3d90cc..696132b2 100644 --- a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h +++ b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h @@ -58,6 +58,12 @@ namespace MobileGL::MG_Impl::GLImpl { void NamedFramebufferReadBuffer(GLuint framebuffer, GLenum src); void ClearNamedFramebufferfv(GLuint framebuffer, GLenum buffer, GLint drawbuffer, const GLfloat* value); void ClearNamedFramebufferfi(GLuint framebuffer, GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil); + void InvalidateNamedFramebufferData(GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments); + void InvalidateNamedFramebufferSubData(GLuint framebuffer, GLsizei numAttachments, const GLenum* attachments, + GLint x, GLint y, GLsizei width, GLsizei height); + void InvalidateFramebuffer(GLenum target, GLsizei numAttachments, const GLenum* attachments); + void InvalidateSubFramebuffer(GLenum target, GLsizei numAttachments, const GLenum* attachments, GLint x, GLint y, + GLsizei width, GLsizei height); void ClearNamedFramebufferiv(GLuint framebuffer, GLenum buffer, GLint drawbuffer, const GLint* value); void ClearNamedFramebufferuiv(GLuint framebuffer, GLenum buffer, GLint drawbuffer, const GLuint* value); GLenum CheckNamedFramebufferStatus(GLuint framebuffer, GLenum target);