swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-07 19:58:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (Framebuffer): the four glFramebufferTexture error conditions the DSA sibling already implemented

Browse Source
This commit is contained in:
Swung0x48
2026-08-27 02:11:20 -04:00
parent d5286e69b6
commit 6979926a6f
2 changed files with 133 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp
+45 -3
View File
@@ -492,16 +492,29 @@ namespace MobileGL::MG_Impl::GLImpl {
const FramebufferAttachmentType attachmentType = MG_Util::ConvertGLEnumToFramebufferAttachmentType(attachment);
const FramebufferTarget framebufferTarget = MG_Util::ConvertGLEnumToFramebufferTarget(target);
if (!FramebufferImpl::ValidateFramebufferAttachmentType(attachmentType)) return;
// GL 4.6 core 9.2.8: COLOR_ATTACHMENTm with m >= MAX_COLOR_ATTACHMENTS is an
// INVALID_OPERATION. The DSA sibling (NamedFramebufferTexture_State) has always asked
// this; the bound-target family did not, so an attachment one past the limit was
// accepted and then silently ignored.
if (!FramebufferImpl::ValidateColorAttachmentInRange(attachmentType, functionName)) return;
if (!FramebufferImpl::ValidateFramebufferTarget(framebufferTarget)) return;
if (!TextureImpl::ValidateTextureName(texture, true)) return;
auto& bindingSlot = MG_State::pGLContext->GetFramebufferBindingSlot(framebufferTarget);
auto& framebufferObject = bindingSlot.GetBoundObject();
if (!framebufferObject) {
// GL 4.6 core 9.2.8: an INVALID_OPERATION error is generated if ZERO is bound to
// target. MobileGL keeps a real FramebufferObject for framebuffer 0, so the null test
// this replaced could never fire - the object is always there. Framebuffer 0 has to be
// recognised by identity instead, the same comparison DrawBuffers_State makes.
const auto& defaultFramebufferInfo = FramebufferImpl::pDefaultFramebufferInfo;
if (!framebufferObject ||
(defaultFramebufferInfo && framebufferObject == defaultFramebufferInfo->defaultFBO)) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", functionName,
"Framebuffer target is bound to no framebuffer object."));
MakeUnique<GenericErrorInfo>(
"MG_Impl/GLImpl", functionName,
"No framebuffer object is bound to the target; the default framebuffer's attachments "
"cannot be named."));
return;
}
@@ -510,6 +523,18 @@ namespace MobileGL::MG_Impl::GLImpl {
return;
}
// GL 4.6 core 9.2.8: a negative level is INVALID_VALUE, and so is a level a texture
// with immutable storage does not have. Asked here rather than in each entry point so
// the whole glFramebufferTexture* family answers the same way, which is what the DSA
// sibling already did for the negative half.
if (level < 0) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidValue,
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", functionName,
"Texture level must be non-negative."));
return;
}
auto& textureObject = MG_State::pGLContext->GetTextureObject(texture);
if (!textureObject) {
MG_State::pGLContext->RecordError(
@@ -519,6 +544,17 @@ namespace MobileGL::MG_Impl::GLImpl {
return;
}
if (textureObject->IsImmutable() &&
level >= static_cast<GLint>(textureObject->GetImmutableLevels())) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidValue,
MakeUnique<GenericErrorInfo>(
"MG_Impl/GLImpl", functionName,
std::format("Texture level {} is beyond the {} level(s) immutable texture {} has.", level,
textureObject->GetImmutableLevels(), texture)));
return;
}
const auto expectedTextureTarget = MG_Util::ConvertTextureUploadTargetToTextureTarget(textureUploadTarget);
if (expectedTextureTarget == TextureTarget::Unknown ||
textureObject->GetTarget() != expectedTextureTarget) {
@@ -1241,6 +1277,12 @@ namespace MobileGL::MG_Impl::GLImpl {
return;
}
// The name's validity is an INVALID_VALUE condition (GL 4.6 core 9.2.8), and it has to be
// asked BEFORE the object is resolved: reporting the miss as the INVALID_OPERATION below
// pre-empted the shared helper's ValidateTextureName and answered the wrong error code for
// every texture name that was never generated.
if (!TextureImpl::ValidateTextureName(texture, true)) return;
auto& textureObject = MG_State::pGLContext->GetTextureObject(texture);
if (!textureObject) {
MG_State::pGLContext->RecordError(
MobileGL/MG_Test/Framebuffer/FramebufferTest.cpp
+88
View File
@@ -1252,3 +1252,91 @@ TEST_F(FramebufferTest, ApplicationAlphaMaskOffIsStillHonouredOnANativeDrawBuffe
EXPECT_EQ(g_driverIndexedColorMasks[2].a, GL_TRUE) << "a native buffer keeps its alpha writes";
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
// --- glFramebufferTexture error conditions (GL 4.6 core 9.2.8) ---------------------------------
//
// Four of them were missing from the bound-target path while its DSA sibling
// (glNamedFramebufferTexture) implemented all four, which is what KHR-GL4x.geometry_shader.
// layered_fbo.fb_texture_* fails on. Two of them - the attachment-range check and the
// default-framebuffer rejection - newly REFUSE calls that used to succeed, so they are pinned
// here rather than left to the conformance suite.
TEST_F(FramebufferTest, FramebufferTextureRejectsTheDefaultFramebuffer) {
GLuint texture = 0;
MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &texture);
MG_Impl::GLImpl::TextureStorage2D(texture, 1, GL_RGBA8, 64, 32);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
// MobileGL models framebuffer 0 as a real FramebufferObject, so the null test that used to
// stand in for this could never fire and the attach silently "succeeded".
MG_Impl::GLImpl::BindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
MG_Impl::GLImpl::FramebufferTexture(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, texture, 0);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
DrainPendingGlErrors();
}
TEST_F(FramebufferTest, FramebufferTextureRejectsAColourAttachmentPastTheLimit) {
GLuint framebuffer = 0;
GLuint texture = 0;
MG_Impl::GLImpl::CreateFramebuffers(1, &framebuffer);
MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &texture);
MG_Impl::GLImpl::TextureStorage2D(texture, 1, GL_RGBA8, 64, 32);
MG_Impl::GLImpl::BindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffer);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
// The same limit ValidateColorAttachmentInRange reads, so the test cannot disagree with the
// implementation about where the boundary is.
const GLint limit = MG_Backend::pActiveBackendObject
? static_cast<GLint>(
MG_Backend::pActiveBackendObject->GetDynamicParameters().MaxColorAttachments)
: static_cast<GLint>(MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS);
ASSERT_GT(limit, 0);
ASSERT_LT(limit, 32) << "the test needs a colour attachment enum past the limit to exist";
MG_Impl::GLImpl::FramebufferTexture(GL_DRAW_FRAMEBUFFER,
static_cast<GLenum>(GL_COLOR_ATTACHMENT0 + limit), texture, 0);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
DrainPendingGlErrors();
// The last legal one still attaches, so the boundary is off-by-none.
MG_Impl::GLImpl::FramebufferTexture(GL_DRAW_FRAMEBUFFER,
static_cast<GLenum>(GL_COLOR_ATTACHMENT0 + limit - 1), texture, 0);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
}
TEST_F(FramebufferTest, FramebufferTextureReportsInvalidValueForANameThatWasNeverGenerated) {
GLuint framebuffer = 0;
MG_Impl::GLImpl::CreateFramebuffers(1, &framebuffer);
MG_Impl::GLImpl::BindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffer);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
// INVALID_VALUE, not INVALID_OPERATION: the entry point used to resolve the texture object
// first and report the miss with the wrong code, pre-empting ValidateTextureName.
const GLuint neverGenerated = std::numeric_limits<GLuint>::max();
ASSERT_FALSE(MG_State::pGLContext->ValidateTextureName(neverGenerated));
MG_Impl::GLImpl::FramebufferTexture(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, neverGenerated, 0);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_VALUE);
DrainPendingGlErrors();
}
TEST_F(FramebufferTest, FramebufferTextureRejectsALevelTheTextureDoesNotHave) {
GLuint framebuffer = 0;
GLuint texture = 0;
MG_Impl::GLImpl::CreateFramebuffers(1, &framebuffer);
MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &texture);
// Two levels of immutable storage: level 1 is legal, level 2 is not.
MG_Impl::GLImpl::TextureStorage2D(texture, 2, GL_RGBA8, 64, 32);
MG_Impl::GLImpl::BindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffer);
ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
MG_Impl::GLImpl::FramebufferTexture(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, texture, 1);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "the last level the texture has is legal";
MG_Impl::GLImpl::FramebufferTexture(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, texture, 2);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_VALUE);
DrainPendingGlErrors();
MG_Impl::GLImpl::FramebufferTexture(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, texture, -1);
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_VALUE);
DrainPendingGlErrors();
}