diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 25aed3dd..663df754 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -6656,46 +6656,72 @@ namespace MobileGL::MG_Impl::GLImpl { pixels, __func__); } + // GL 4.6 core 8.11.4: the region checks glGetTextureSubImage and glGetCompressedTextureSubImage + // share, ahead of anything about the destination. On success `levelExtent` is the level's + // (width, height, depth) in the sub-image's own coordinates - a 1D array's layers are its + // height, a 2D array's or cube map array's layer(-face)s its depth, and a cube map's six faces + // its depth too. + static Bool ValidateTextureSubImageRegion(const SharedPtr& textureObject, + GLint level, GLint xoffset, GLint yoffset, GLint zoffset, + GLsizei width, GLsizei height, GLsizei depth, IntVec3& levelExtent, + const char* caller) { + const auto recordError = [caller](ErrorCode code, const char* message) { + MG_State::pGLContext->RecordError(code, MakeUnique("MG_Impl/GLImpl", caller, message)); + return false; + }; + const auto target = textureObject->GetTarget(); + if (textureObject->GetStorageType() != TextureStorageType::Mipmap || + target == TextureTarget::Texture2DMultisample || target == TextureTarget::Texture2DMultisampleArray) { + return recordError(ErrorCode::InvalidOperation, + "The effective target is a buffer or multisample texture."); + } + if (level < 0 || xoffset < 0 || yoffset < 0 || zoffset < 0 || width < 0 || height < 0 || depth < 0) { + return recordError(ErrorCode::InvalidValue, "Texture sub-image range is invalid."); + } + auto* textureMipmapObject = static_cast(textureObject.get()); + if (static_cast(level) >= textureMipmapObject->GetMipmapLevelCount()) { + return recordError(ErrorCode::InvalidValue, "Texture level is out of range."); + } + if (target == TextureTarget::Texture1D && (yoffset != 0 || height != 1)) { + return recordError(ErrorCode::InvalidValue, "A 1D texture needs yoffset 0 and height 1."); + } + if ((target == TextureTarget::Texture1D || target == TextureTarget::Texture1DArray || + target == TextureTarget::Texture2D || target == TextureTarget::TextureRectangle) && + (zoffset != 0 || depth != 1)) { + return recordError(ErrorCode::InvalidValue, "This texture target needs zoffset 0 and depth 1."); + } + + levelExtent = textureMipmapObject->GetMipmapTexelSize(GetPrimaryUploadTarget(textureObject), + static_cast(level)); + // A cube map's level is stored per face, so its own extent reads z = 1; the six faces are + // what zoffset/depth address. + if (target == TextureTarget::TextureCubeMap) levelExtent.z() = 6; + if (static_cast(xoffset) + width > levelExtent.x() || + static_cast(yoffset) + height > levelExtent.y() || + static_cast(zoffset) + depth > levelExtent.z()) { + return recordError(ErrorCode::InvalidValue, "Texture sub-image region exceeds the texture level."); + } + return true; + } + void GetTextureSubImage(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, GLsizei bufSize, void* pixels) { auto textureObject = GetTextureObjectByName(texture, __func__); if (!textureObject) return; - if (level < 0 || xoffset < 0 || yoffset < 0 || zoffset < 0 || width < 0 || height < 0 || depth < 0) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidValue, - MakeUnique("MG_Impl/GLImpl", __func__, "Texture sub-image range is invalid.")); - return; - } - if (textureObject->GetStorageType() != TextureStorageType::Mipmap) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", __func__, "Texture storage is not mipmap-backed.")); + IntVec3 levelExtent; + if (!ValidateTextureSubImageRegion(textureObject, level, xoffset, yoffset, zoffset, width, height, depth, + levelExtent, __func__)) { return; } - const auto uploadTarget = GetPrimaryUploadTarget(textureObject); - auto* textureMipmapObject = static_cast(textureObject.get()); - if (static_cast(level) >= textureMipmapObject->GetMipmapLevelCount()) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidValue, - MakeUnique("MG_Impl/GLImpl", __func__, "Texture level is out of range.")); - return; - } - - const auto texelSize = textureMipmapObject->GetMipmapTexelSize(uploadTarget, static_cast(level)); - // On a cube map, z is the FACE axis. A cube map's level is stored per face, so its level - // size reads z = 1 whichever face named it - but GL 4.6 core 8.11.4 addresses the six faces - // of a cube map through zoffset/depth, exactly the six layers a face token names for - // glGetTexImage. Without this arm the z range was measured against that 1 and only zoffset 0 - // (the +X face) was expressible; the other five were rejected as a partial read. - // - // Only ONE face at a time. depth > 1 would have to concatenate faces into the destination, - // which is the same unimplemented multi-image packing the check below still refuses. - const Bool isSingleCubeFaceRead = textureObject->GetTarget() == TextureTarget::TextureCubeMap && - depth == 1 && zoffset < 6; - const Bool isFullLevelRead = xoffset == 0 && yoffset == 0 && width == texelSize.x() && - height == texelSize.y() && - (isSingleCubeFaceRead || (zoffset == 0 && depth == texelSize.z())); + // A cube map answers one face (zoffset names it, the by-name spelling of glGetTexImage's + // face token) or all six; any other run of faces would have to concatenate faces into the + // destination, which is the same unimplemented multi-image packing the check below refuses. + const Bool isCubeMap = textureObject->GetTarget() == TextureTarget::TextureCubeMap; + const Bool isSingleCubeFaceRead = isCubeMap && depth == 1; + const Bool isFullLevelRead = xoffset == 0 && yoffset == 0 && width == levelExtent.x() && + height == levelExtent.y() && + (isSingleCubeFaceRead || (zoffset == 0 && depth == levelExtent.z())); if (!isFullLevelRead) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, @@ -6704,10 +6730,13 @@ namespace MobileGL::MG_Impl::GLImpl { return; } - const TextureUploadTarget readUploadTarget = - isSingleCubeFaceRead ? static_cast( - static_cast(TextureUploadTarget::CubeMapPositiveX) + zoffset) - : uploadTarget; + TextureUploadTarget readUploadTarget = GetPrimaryUploadTarget(textureObject); + if (isSingleCubeFaceRead) { + readUploadTarget = static_cast( + static_cast(TextureUploadTarget::CubeMapPositiveX) + zoffset); + } else if (isCubeMap) { + readUploadTarget = TextureUploadTarget::Unknown; // all six faces, as glGetTextureImage reads them + } if (!ValidateTextureImageQuery(textureObject, level, MG_Util::ConvertGLEnumToTextureInputFormat(format), MG_Util::ConvertGLEnumToTexturePixelDataType(type), bufSize, pixels, __func__, isSingleCubeFaceRead ? 1u : 0u)) { diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 9816c420..abc4eaba 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -2191,11 +2191,50 @@ TEST_F(TextureTest, GetTextureSubImageSelectsTheCubeFaceZOffsetNames) { << "zoffset " << face << " answered with face " << (static_cast(output[0]) - 10); } - // Past the last face. Still a partial read of a level with no sixth-and-beyond image. + // Past the last face: zoffset + depth exceeds the six faces, a range error (GL 4.6 core 8.11.4). Uint8 output[4] = {}; MG_Impl::GLImpl::GetTextureSubImage(texture, 0, 0, 0, 6, 1, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, sizeof(output), output); - EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_VALUE); +} + +// GL 4.6 core 8.11.4's region errors come before anything about whether the read can be served +// (KHR-GL45.get_texture_sub_image.errors_test): they used to be reported as the INVALID_OPERATION +// of an unimplemented partial read. +TEST_F(TextureTest, GetTextureSubImageReportsRegionErrorsAsInvalidValue) { + GLuint textures[4] = {}; + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &textures[0]); + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_1D, 1, &textures[1]); + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_1D_ARRAY, 1, &textures[2]); + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D_MULTISAMPLE, 1, &textures[3]); + MG_Impl::GLImpl::TextureStorage2D(textures[0], 1, GL_RGBA8, 2, 2); + MG_Impl::GLImpl::TextureStorage1D(textures[1], 1, GL_RGBA8, 2); + MG_Impl::GLImpl::TextureStorage2D(textures[2], 1, GL_RGBA8, 2, 2); + MG_Impl::GLImpl::TextureStorage2DMultisample(textures[3], 1, GL_RGBA8, 2, 2, GL_TRUE); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + Uint8 output[64] = {}; + const auto read = [&](GLuint texture, GLint x, GLint y, GLint z, GLsizei w, GLsizei h, GLsizei d) { + MG_Impl::GLImpl::GetTextureSubImage(texture, 0, x, y, z, w, h, d, GL_RGBA, GL_UNSIGNED_BYTE, + sizeof(output), output); + }; + read(textures[0], -1, 0, 0, 2, 2, 1); + ExpectSingleGlError(GL_INVALID_VALUE); + read(textures[0], 2, 2, 0, 4, 4, 1); // past the level's width and height + ExpectSingleGlError(GL_INVALID_VALUE); + read(textures[1], 0, 1, 0, 2, 2, 1); // 1D: yoffset must be 0 and height 1 + ExpectSingleGlError(GL_INVALID_VALUE); + for (const GLuint texture : {textures[0], textures[1], textures[2]}) { + read(texture, 0, 0, 1, 2, texture == textures[1] ? 1 : 2, 2); // zoffset must be 0 and depth 1 + ExpectSingleGlError(GL_INVALID_VALUE); + } + read(textures[3], 0, 0, 0, 2, 2, 1); // a multisample texture has no image to return + ExpectSingleGlError(GL_INVALID_OPERATION); + MG_Impl::GLImpl::GetTextureSubImage(textures[0], 0, 0, 0, 0, 2, 2, 1, GL_RGBA, GL_UNSIGNED_BYTE, 1, output); + ExpectSingleGlError(GL_INVALID_OPERATION); // bufSize too small + + MG_Impl::GLImpl::DeleteTextures(4, textures); + DrainPendingGlErrors(); } TEST_F(TextureTest, TextureParameteriAndBindTextureUnitAreDirectStateAccess) {