[Feat] (MG_Impl, MG_Test): validate the direct-state-access texture copies

CopyTextureSubImage1D and 3D were do-nothing stubs and the 2D form checked only
its effective target, so all 28 conditions in
direct_state_access.textures_copy_errors went unreported: level and region
bounds, and every read-framebuffer precondition.

The read-framebuffer half lands in FramebufferImpl as ValidateReadFramebufferForCopy -
incomplete read framebuffer (INVALID_FRAMEBUFFER_OPERATION), a read buffer that
names no attachment, and a multisampled read buffer (both INVALID_OPERATION). It
decides multisampledness by attachment kind rather than by sample count alone,
because a TEXTURE_2D_MULTISAMPLE attachment sets SAMPLE_BUFFERS even when its
sample count is one - which is exactly what the CTS attaches, and what a
renderbuffer-only check would have missed.

The texture half is ValidateCopyTextureSubImage, shared by all three forms; 1D
and 3D also get the effective-target rule their form specifies.

NOTE: the copy itself is still not implemented for 1D and 3D - CopyTexSubImage1D_State
and CopyTexSubImage3D_State remain TODOs and no backend exposes anything but a
2D blit - so direct_state_access.textures_copy stays red. Only the errors are
complete, which is what un-stubbing these two entry points buys; both carry a
comment saying so.

CopyTextureSubImage2DUsesNamedObjectAndRestoresBinding had been passing a
storage-less texture and no read framebuffer, which the new validation correctly
rejects. It now sets up a legal copy, so it still measures the by-name plumbing
it was written for.

Takes direct_state_access.textures_copy_errors from failing to passing on both
backends.
This commit is contained in:
BZLZHH
2026-08-05 01:54:39 -04:00
parent 31ea6aa5a3
commit 81604d5596
6 changed files with 133 additions and 2 deletions
@@ -1062,9 +1062,9 @@ DECLARE_GL_FUNCTION_HEAD(void, TextureSubImage3D, GLuint texture, GLint level, G
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, 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_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_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_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_STUB_HEAD(void, CopyTextureSubImage1D, GLuint texture, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CopyTextureSubImage1D, texture, level, xoffset, x, y, width) 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) 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)
DECLARE_GL_FUNCTION_STUB_HEAD(void, CopyTextureSubImage3D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLint x, GLint y, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, CopyTextureSubImage3D, texture, level, xoffset, yoffset, zoffset, x, y, width, height) DECLARE_GL_FUNCTION_HEAD(void, CopyTextureSubImage3D, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLint x, GLint y, GLsizei width, GLsizei height) DECLARE_GL_FUNCTION_END_NO_RETURN(void, CopyTextureSubImage3D, texture, level, xoffset, yoffset, zoffset, x, y, width, height)
DECLARE_GL_FUNCTION_HEAD(void, TextureParameterf, GLuint texture, GLenum pname, GLfloat param) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureParameterf, texture, pname, param) DECLARE_GL_FUNCTION_HEAD(void, TextureParameterf, GLuint texture, GLenum pname, GLfloat param) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureParameterf, texture, pname, param)
DECLARE_GL_FUNCTION_HEAD(void, TextureParameterfv, GLuint texture, GLenum pname, const GLfloat* param) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureParameterfv, texture, pname, param) DECLARE_GL_FUNCTION_HEAD(void, TextureParameterfv, GLuint texture, GLenum pname, const GLfloat* param) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureParameterfv, texture, pname, param)
DECLARE_GL_FUNCTION_HEAD(void, TextureParameteri, GLuint texture, GLenum pname, GLint param) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureParameteri, texture, pname, param) DECLARE_GL_FUNCTION_HEAD(void, TextureParameteri, GLuint texture, GLenum pname, GLint param) DECLARE_GL_FUNCTION_END_NO_RETURN(void, TextureParameteri, texture, pname, param)
@@ -118,4 +118,48 @@ namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl {
std::format("Renderbuffer name {} is not valid.", index))); std::format("Renderbuffer name {} is not valid.", index)));
return false; return false;
} }
Bool ValidateReadFramebufferForCopy(const char* caller) {
auto& framebufferObject =
MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Read).GetBoundObject();
if (!framebufferObject || !framebufferObject->CheckCompleteness()) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidFramebufferOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl/FramebufferImpl", caller,
"Read framebuffer is not framebuffer complete."));
return false;
}
const FramebufferAttachmentType readBuffer = framebufferObject->GetReadBuffer();
if (readBuffer == FramebufferAttachmentType::None ||
!framebufferObject->GetAttachment(readBuffer).IsValid()) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl/FramebufferImpl", caller,
"Read buffer names no attachment of the read framebuffer."));
return false;
}
// SAMPLE_BUFFERS is one whenever the read buffer resolves to multisample storage. A
// multisample texture says so by its target - its sample count can legally be one - while a
// renderbuffer says so by having been given a non-zero sample count.
const auto& readAttachment = framebufferObject->GetAttachment(readBuffer);
Bool isMultisampled = false;
if (readAttachment.IsRenderbuffer() && readAttachment.GetRenderbuffer()) {
isMultisampled = readAttachment.GetRenderbuffer()->GetSamples() > 0;
} else if (readAttachment.IsTexture() && readAttachment.GetTexture()) {
const auto target = readAttachment.GetTexture()->GetTarget();
isMultisampled = target == TextureTarget::Texture2DMultisample ||
target == TextureTarget::Texture2DMultisampleArray;
}
if (isMultisampled) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl/FramebufferImpl", caller,
"Cannot copy from a multisampled read framebuffer."));
return false;
}
return true;
}
} // namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl } // namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl
@@ -20,4 +20,9 @@ namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl {
Bool ValidateColorAttachmentInRange(FramebufferAttachmentType attachment, const char* caller); Bool ValidateColorAttachmentInRange(FramebufferAttachmentType attachment, const char* caller);
Bool ValidateRenderbufferTarget(RenderbufferTarget target); Bool ValidateRenderbufferTarget(RenderbufferTarget target);
Bool ValidateRenderbufferName(Uint index, Bool allowZero = true); Bool ValidateRenderbufferName(Uint index, Bool allowZero = true);
// The read-framebuffer preconditions the CopyTexSubImage family shares (GL 4.6 core 8.6): the
// read framebuffer must be complete, its read buffer must name a real attachment, and it must
// not be multisampled. Incompleteness is INVALID_FRAMEBUFFER_OPERATION, the other two are
// INVALID_OPERATION.
Bool ValidateReadFramebufferForCopy(const char* caller);
} // namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl } // namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl
@@ -25,6 +25,7 @@
#include <MG_Util/Converters/GLToMG/TextureEnumConverter.h> #include <MG_Util/Converters/GLToMG/TextureEnumConverter.h>
#include <MG_Util/Converters/MGToGL/TextureEnumConverter.h> #include <MG_Util/Converters/MGToGL/TextureEnumConverter.h>
#include <MG_Util/Converters/MGToStr/TextureEnumConverter.h> #include <MG_Util/Converters/MGToStr/TextureEnumConverter.h>
#include <MG_Impl/GLImpl/Framebuffer/Validators.h>
#include <MG_Impl/GLImpl/Getter/GL_Getter.h> #include <MG_Impl/GLImpl/Getter/GL_Getter.h>
#include <MG_State/GLState/TextureState/TextureObjectBuffer.h> #include <MG_State/GLState/TextureState/TextureObjectBuffer.h>
@@ -3020,6 +3021,26 @@ namespace MobileGL::MG_Impl::GLImpl {
// TODO: implement // TODO: implement
} }
// What the three CopyTextureSubImage forms check in common (GL 4.6 core 8.6), once the caller
// has rejected an effective target its own form does not accept: the destination region has to
// lie inside the level, and the read framebuffer has to be able to supply pixels at all.
Bool ValidateCopyTextureSubImage(const SharedPtr<MG_State::GLState::ITextureObject>& textureObject, GLint level,
GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height,
GLsizei depth, const char* caller) {
if (!TextureImpl::ValidateTextureLevelNumber(level)) return false;
if (width < 0 || height < 0 || depth < 0) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidValue,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller, "Copy dimensions must be non-negative."));
return false;
}
if (!TextureImpl::ValidateTextureSubImageOffsets(textureObject, xoffset, width, yoffset, height, zoffset,
depth)) {
return false;
}
return FramebufferImpl::ValidateReadFramebufferForCopy(caller);
}
void CopyTexSubImage2D_Backend(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, void CopyTexSubImage2D_Backend(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y,
GLsizei width, GLsizei height) { GLsizei width, GLsizei height) {
MG_Backend::gBackendFunctionsTable.GL.CopyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height); MG_Backend::gBackendFunctionsTable.GL.CopyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
@@ -4706,11 +4727,56 @@ namespace MobileGL::MG_Impl::GLImpl {
"rectangle texture.")); "rectangle texture."));
return; return;
} }
if (!ValidateCopyTextureSubImage(textureObject, level, xoffset, yoffset, 0, width, height, 1, __func__)) {
return;
}
WithTemporarilyBoundNamedTexture(textureObject, [&](GLenum glTarget) { WithTemporarilyBoundNamedTexture(textureObject, [&](GLenum glTarget) {
CopyTexSubImage2D_Backend(glTarget, level, xoffset, yoffset, x, y, width, height); CopyTexSubImage2D_Backend(glTarget, level, xoffset, yoffset, x, y, width, height);
}); });
} }
void CopyTextureSubImage1D(GLuint texture, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width) {
auto textureObject = GetTextureObjectByName(texture, __func__);
if (!textureObject) return;
if (textureObject->GetTarget() != TextureTarget::Texture1D) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
"CopyTextureSubImage1D requires a 1D texture."));
return;
}
if (!ValidateCopyTextureSubImage(textureObject, level, xoffset, 0, 0, width, 1, 1, __func__)) return;
// NOTE: the copy itself is still missing - CopyTexSubImage1D_State is a no-op and no backend
// exposes a 1D blit - so a valid call reaches the destination unchanged. Only the error
// reporting above is complete.
CopyTexSubImage1D_State(GL_TEXTURE_1D, level, xoffset, x, y, width);
}
void CopyTextureSubImage3D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLint x,
GLint y, GLsizei width, GLsizei height) {
auto textureObject = GetTextureObjectByName(texture, __func__);
if (!textureObject) return;
// GL 4.6 core 8.6: the 3D form takes the layered targets, a cube map included - the face
// is selected by zoffset.
const auto target = textureObject->GetTarget();
if (target != TextureTarget::Texture3D && target != TextureTarget::Texture2DArray &&
target != TextureTarget::TextureCubeMap && target != TextureTarget::TextureCubeMapArray) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
"CopyTextureSubImage3D requires a 3D, 2D-array, cube map, or "
"cube map array texture."));
return;
}
if (!ValidateCopyTextureSubImage(textureObject, level, xoffset, yoffset, zoffset, width, height, 1, __func__)) {
return;
}
// NOTE: as with the 1D form, CopyTexSubImage3D_State does not perform the copy yet.
WithTemporarilyBoundNamedTexture(textureObject, [&](GLenum glTarget) {
CopyTexSubImage3D_State(glTarget, level, xoffset, yoffset, zoffset, x, y, width, height);
});
}
void CopyTexSubImage1D(GLenum target, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width) { void CopyTexSubImage1D(GLenum target, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width) {
CopyTexSubImage1D_State(target, level, xoffset, x, y, width); CopyTexSubImage1D_State(target, level, xoffset, x, y, width);
} }
@@ -106,6 +106,9 @@ namespace MobileGL::MG_Impl::GLImpl {
GLsizei width, GLsizei height); GLsizei width, GLsizei height);
void CopyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, void CopyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width,
GLsizei height); GLsizei height);
void CopyTextureSubImage1D(GLuint texture, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width);
void CopyTextureSubImage3D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLint x,
GLint y, GLsizei width, GLsizei height);
void CopyTextureSubImage2D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, void CopyTextureSubImage2D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y,
GLsizei width, GLsizei height); GLsizei width, GLsizei height);
void CopyTexSubImage1D(GLenum target, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width); void CopyTexSubImage1D(GLenum target, GLint level, GLint xoffset, GLint x, GLint y, GLsizei width);
+13
View File
@@ -15,6 +15,7 @@
#include <Config.h> #include <Config.h>
#include <MG_Backend/BackendObjects.h> #include <MG_Backend/BackendObjects.h>
#include <MG_Backend/DirectGLES/Managers.h> #include <MG_Backend/DirectGLES/Managers.h>
#include <MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.h>
#include <MG_Impl/GLImpl/Getter/GL_Getter.h> #include <MG_Impl/GLImpl/Getter/GL_Getter.h>
#include <MG_Impl/GLImpl/RenderState/GL_RenderState.h> #include <MG_Impl/GLImpl/RenderState/GL_RenderState.h>
#include <MG_Impl/GLImpl/Sampler/GL_Sampler.h> #include <MG_Impl/GLImpl/Sampler/GL_Sampler.h>
@@ -296,6 +297,18 @@ TEST_F(TextureTest, CopyTextureSubImage2DUsesNamedObjectAndRestoresBinding) {
MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &boundTexture); MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &boundTexture);
MG_Impl::GLImpl::BindTextureUnit(0, boundTexture); MG_Impl::GLImpl::BindTextureUnit(0, boundTexture);
// The copy is only allowed to reach the backend when the destination region fits the level and
// the read framebuffer can supply pixels, so the call has to be set up as a legal one.
MG_Impl::GLImpl::TextureStorage2D(namedTexture, 3, GL_RGBA8, 16, 16);
GLuint readFramebuffer = 0;
GLuint readTexture = 0;
MG_Impl::GLImpl::CreateFramebuffers(1, &readFramebuffer);
MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &readTexture);
MG_Impl::GLImpl::TextureStorage2D(readTexture, 1, GL_RGBA8, 16, 16);
MG_Impl::GLImpl::NamedFramebufferTexture(readFramebuffer, GL_COLOR_ATTACHMENT0, readTexture, 0);
MG_Impl::GLImpl::BindFramebuffer(GL_READ_FRAMEBUFFER, readFramebuffer);
const auto boundBefore = MG_State::pGLContext->GetTextureUnitObject(0) const auto boundBefore = MG_State::pGLContext->GetTextureUnitObject(0)
.GetBindingSlot(TextureTarget::Texture2D) .GetBindingSlot(TextureTarget::Texture2D)
.GetBoundObject(); .GetBoundObject();