diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 9e24e1bd..8e720833 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -5581,6 +5581,16 @@ namespace MobileGL::MG_Backend::DirectGLES { TextureImpl::SyncTextureObjectToBackend(srcTexture); const SharedPtr dstBackendTexture = TextureImpl::SyncTextureObjectToBackend(dstTexture); + // The DirectVulkan half of this entry point died exactly here, on a texture whose sync + // produced nothing - and it died in a release build, where the MOBILEGL_ASSERT that was + // supposed to catch it expands to nothing. The four GetBackendTextureId() calls below + // are the same dereference. The frontend validator is what keeps this unreachable and + // what reports the error the application is owed; declining is only how a future gap up + // there stops being a crash. See the level guard in VulkanRenderer::CopyImageSubData. + if (!srcBackendTexture || !dstBackendTexture) { + MGLOG_E_ONCE("%s: source or destination texture failed to sync; declining the copy", __func__); + return; + } const Bool srcIsDepth = MG_Util::IsDepthFormatInternalFormat(srcTexture->GetFormat()); const Bool dstIsDepth = MG_Util::IsDepthFormatInternalFormat(dstTexture->GetFormat()); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index bdb32b72..2d03a4e1 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -8529,12 +8529,31 @@ void main() { auto* srcResource = m_textureManager->SyncTextureAndGetDescriptor(*srcTexture); auto* dstResource = m_textureManager->SyncTextureAndGetDescriptor(*dstTexture); - MOBILEGL_ASSERT(srcResource != nullptr && dstResource != nullptr, - "CopyImageSubData failed to sync source or destination texture."); - MOBILEGL_ASSERT(srcLevel >= 0 && dstLevel >= 0 && - static_cast(srcLevel) < srcResource->mipLevels && - static_cast(dstLevel) < dstResource->mipLevels, - "CopyImageSubData mip level is out of range."); + // Real checks, not MOBILEGL_ASSERT: the assertions this replaces compile to nothing in + // a release build, which is where both observed failures happened - a null resource + // dereferenced right below (lavapipe) and a mip level the VkImage does not have handed + // to vkCmdCopyImage (Adreno, SIGSEGV inside the driver). Neither is caught downstream: + // an out-of-range subresource is a promise the driver takes at face value. + // + // _ONCE, because the severity is right but the repetition is not: MGLOG_E is the level + // the project logs failures at and it IS live at the default MOBILEGL_LOG_ACTIVE_LEVEL, + // so an application that reissues the same rejected copy every frame would otherwise + // print at ERROR every frame. Once per site says the same thing and says it in a log + // somebody can still read. + // + // The frontend validator (ValidateTextureLevelExists) is what produces the + // GL_INVALID_VALUE the application is actually owed. This guard exists so the next gap + // up there declines a copy instead of taking the process down. + if (srcResource == nullptr || dstResource == nullptr) { + MGLOG_E_ONCE("%s: source or destination texture failed to sync; declining the copy", __func__); + return; + } + if (srcLevel < 0 || dstLevel < 0 || static_cast(srcLevel) >= srcResource->mipLevels || + static_cast(dstLevel) >= dstResource->mipLevels) { + MGLOG_E_ONCE("%s: mip level out of range (src %d of %u, dst %d of %u); declining the copy", __func__, + srcLevel, srcResource->mipLevels, dstLevel, dstResource->mipLevels); + return; + } const VkImageAspectFlags copyAspectMask = srcResource->aspect & dstResource->aspect & (VK_IMAGE_ASPECT_COLOR_BIT | VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT); @@ -8548,12 +8567,23 @@ void main() { const Uint32 srcMipHeight = std::max(1u, srcResource->extent.height >> srcMipLevel); const Uint32 dstMipWidth = std::max(1u, dstResource->extent.width >> dstMipLevel); const Uint32 dstMipHeight = std::max(1u, dstResource->extent.height >> dstMipLevel); - MOBILEGL_ASSERT(srcX >= 0 && srcY >= 0 && dstX >= 0 && dstY >= 0 && - static_cast(srcX + srcWidth) <= srcMipWidth && - static_cast(srcY + srcHeight) <= srcMipHeight && - static_cast(dstX + srcWidth) <= dstMipWidth && - static_cast(dstY + srcHeight) <= dstMipHeight, - "CopyImageSubData region is outside source or destination bounds."); + // Promoted for the same reason as the level range above, and it is the same bug class: + // a VkImageCopy whose region runs past the image is an out-of-bounds promise to the + // driver, and the frontend does not check the region at all (there is a CTS sibling, + // copy_image.exceeding_boundaries, that asks for exactly this input). Nothing legal is + // lost by declining - a copy that reads or writes outside the image was never going to + // produce a correct result, it was going to produce whatever the driver did next. + if (srcX < 0 || srcY < 0 || dstX < 0 || dstY < 0 || + static_cast(srcX + srcWidth) > srcMipWidth || + static_cast(srcY + srcHeight) > srcMipHeight || + static_cast(dstX + srcWidth) > dstMipWidth || + static_cast(dstY + srcHeight) > dstMipHeight) { + MGLOG_E_ONCE("%s: region outside image bounds (src %dx%d+%d+%d of %ux%u, dst +%d+%d of %ux%u); " + "declining the copy", + __func__, srcWidth, srcHeight, srcX, srcY, srcMipWidth, srcMipHeight, dstX, dstY, + dstMipWidth, dstMipHeight); + return; + } const Bool clearReady = MaterializePendingClearForTexture(frame.commandBuffer, *srcTexture); MOBILEGL_ASSERT(clearReady, "%s: failed to materialize pending clear for source textureId=%d", diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index ba2c2f6f..138fc924 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -3404,6 +3404,17 @@ namespace MobileGL::MG_Impl::GLImpl { !TextureImpl::ValidateTextureLevelNumber(dstLevel)) { return false; } + // ValidateTextureLevelNumber only bounds the index by GL_MAX_TEXTURE_SIZE; it cannot + // see that this particular texture stops at level 0. Both backends turn into an + // image subresource with no further checking (DirectVulkan builds a VkImageCopy from it, + // DirectGLES forwards it to the ES copy), so a level the texture never had reached the + // driver as an out-of-range mip index - on Adreno that is a SIGSEGV inside + // vkCmdCopyImage, which is what KHR-GL43.copy_image.non_existent_mipmap used to do to + // the whole glcts process. The answer the spec asks for is GL_INVALID_VALUE. + if (!TextureImpl::ValidateTextureLevelExists(srcTexture, srcLevel, __func__) || + !TextureImpl::ValidateTextureLevelExists(dstTexture, dstLevel, __func__)) { + return false; + } if (srcWidth < 0 || srcHeight < 0 || srcDepth < 0) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, diff --git a/MobileGL/MG_Impl/GLImpl/Texture/Validators.cpp b/MobileGL/MG_Impl/GLImpl/Texture/Validators.cpp index 3e75cf10..a08ca473 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/Validators.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/Validators.cpp @@ -353,6 +353,63 @@ namespace MobileGL::MG_Impl::GLImpl::TextureImpl { return true; } + Bool ValidateTextureLevelExists(const SharedPtr& textureObject, Int level, + const char* caller) { + // A null object is somebody else's error to report - ValidateTextureObject runs + // first at every call site and has already recorded it. + if (!textureObject) return false; + + const auto* mipmapTexture = MG_State::GLState::AsMipmapTexture(textureObject.get()); + if (mipmapTexture == nullptr) { + // The only non-mipmap storage class is a buffer texture, and GL_TEXTURE_BUFFER is + // not a target glCopyImageSubData accepts at all (it is in the CTS's invalid-target + // set). Declining here is not the error code the spec asks for - that would be + // INVALID_ENUM from a target check this validator is not - but it does keep a + // texture with no image levels whatsoever from reaching a backend that would + // dereference a backend texture it never created. + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", caller, + "Texture has no mipmap levels to address.")); + return false; + } + + // What this number is, exactly, because two other things are almost it and neither is + // safe to assume: it is the number of level SLOTS the shadow has allocated - holes + // included, since MipmapStorage::AllocateLevel grows to level+1 and never fills the gap. + // For a cube map MipmapUploadTargetArray reports face +X's chain rather than the union. + // + // The guarantee that matters is one-sided: this count is always >= the level count the + // backends derive (VkTextureManager::GetUploadMipLevelCount stops at the first level + // with a non-positive extent, so it can only be shorter). That is the safe direction - + // no copy to a level the texture genuinely has is ever rejected here. It is NOT an + // exact match, so the backends keep their own range guard for the band in between: a + // chain with a hole (level 0 and 2 defined, 1 not) is accepted by this predicate and + // declined by the backend, which is a silent no-op rather than a copy. That band is a + // backend storage limitation, not a validation one - rejecting it here with + // INVALID_VALUE would be refusing a copy the spec permits. + const Uint levelCount = mipmapTexture->GetMipmapLevelCount(); + + if (levelCount == 0) { + // No image has ever been defined on this texture, so the fault is the texture, + // not the number: GL 4.6 core 18.3.2 asks for INVALID_OPERATION when an object a + // copy names is an incomplete texture. + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", caller, + "Texture has no image defined at any level.")); + return false; + } + if (level < 0 || static_cast(level) >= levelCount) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", caller, + "Texture level does not exist in this texture.")); + return false; + } + return true; + } + Bool ValidateTextureObject(const SharedPtr& textureObject) { if (!textureObject) { MG_State::pGLContext->RecordError( diff --git a/MobileGL/MG_Impl/GLImpl/Texture/Validators.h b/MobileGL/MG_Impl/GLImpl/Texture/Validators.h index 7ebd14ff..20b74553 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/Validators.h +++ b/MobileGL/MG_Impl/GLImpl/Texture/Validators.h @@ -30,6 +30,16 @@ namespace MobileGL::MG_Impl::GLImpl::TextureImpl { TextureInternalFormat internalFormat, TexturePixelDataType type); Bool ValidateTextureLevelWithUploadTarget(TextureUploadTarget target, Int level); + // "Is a level this texture actually has?", which ValidateTextureLevelNumber above + // does NOT answer - that one only bounds the index by GL_MAX_TEXTURE_SIZE and knows nothing + // about the object. Entry points that resolve a level straight into a backend image + // subresource need this one: a level the texture never had is GL_INVALID_VALUE (GL 4.6 core + // 18.3.2), and passing it through instead reaches the driver as an out-of-range subresource. + // Note the error split is per-entry-point, so this is not universally reusable: + // glClearTexImage owes INVALID_OPERATION for the same out-of-range level and spells its own + // copy of this predicate in GL_Texture.cpp (GetClearTextureObject). + Bool ValidateTextureLevelExists(const SharedPtr& textureObject, Int level, + const char* caller); Bool ValidateTextureObject(const SharedPtr& textureObject); // Rejects the per-target default texture objects (name 0) with GL_INVALID_OPERATION for entry // points that require a GenTextures-created texture, e.g. TexStorage* ("An INVALID_OPERATION diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index ceb469b7..920db51e 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -78,6 +78,7 @@ add_executable(MobileGLIntegrationTest Scenarios/VertexAttribBindingScenario.cpp Scenarios/XfbCaptureBufferReuseScenario.cpp Scenarios/VertexArrayEnableDisableScenario.cpp + Scenarios/CopyImageLevelRangeScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/CopyImageLevelRangeScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/CopyImageLevelRangeScenario.cpp new file mode 100644 index 00000000..2cc4ea79 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/CopyImageLevelRangeScenario.cpp @@ -0,0 +1,209 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/CopyImageLevelRangeScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// KHR-GL43.copy_image.non_existent_mipmap, and what it cost. +// +// The CTS case is a pure negative test: two 16x16 textures that have level 0 and +// nothing else, and a glCopyImageSubData naming level 1. The answer is +// GL_INVALID_VALUE (GL 4.6 core 18.3.2 / ARB_copy_image: "srcLevel/dstLevel is not +// a valid level"). MobileGL's frontend only checked the level against +// GL_MAX_TEXTURE_SIZE, so level 1 sailed through into the backends, DirectVulkan +// resolved it into a VkImageCopy subresource on a VkImage that was created with +// exactly one mip level, and the Adreno driver dereferenced the level it was +// promised - SIGSEGV inside vkCmdCopyImage, taking the whole glcts process down +// mid-run. A negative case must never do that. +// +// So the level-1-on-a-one-level-texture rejection is the regression proper, and the +// rest of this file is what keeps the fix honest. A validator that answered +// GL_INVALID_VALUE to every level would satisfy the regression tests alone, so the +// scenarios below pin the BOUNDARY rather than the symptom: +// +// * a texture that really does have two levels must accept a copy at level 1, +// * the same texture must still reject level 2, +// * and a plain level-0 copy must move pixels, which is checked by reading the +// destination back rather than by trusting glGetError. +// +// Both backends are covered because the fix is in the shared frontend: DirectGLES +// forwards to the ES glCopyImageSubData (whose own error lands in the ES context, +// not in MobileGL's, so it never reached the application either) and DirectVulkan +// records the copy itself. + +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr GLsizei kSize = 16; + + struct Rgba8 { + GLubyte r, g, b, a; + bool operator==(const Rgba8& other) const { + return r == other.r && g == other.g && b == other.b && a == other.a; + } + }; + + std::vector SolidImage(GLsizei width, GLsizei height, Rgba8 color) { + return std::vector(static_cast(width) * static_cast(height), color); + } + + class CopyImageLevelRangeScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + DrainErrors(); + } + + void TearDown() override { + if (!Ready()) return; + DeleteTextures(); + if (m_fbo != 0) { + glBindFramebuffer(GL_FRAMEBUFFER, 0); + glDeleteFramebuffers(1, &m_fbo); + m_fbo = 0; + } + DrainErrors(); + ScenarioTest::TearDown(); + } + + static void DrainErrors() { + for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) { + } + } + + void DeleteTextures() { + if (m_src != 0) glDeleteTextures(1, &m_src); + if (m_dst != 0) glDeleteTextures(1, &m_dst); + m_src = 0; + m_dst = 0; + } + + // One 16x16 RGBA8 texture with `levelCount` levels defined through + // glTexImage2D - the same way the CTS case builds its textures, and + // deliberately NOT glTexStorage2D: an immutable allocation would define the + // whole chain up front and could not express "level 1 does not exist". + GLuint MakeTexture(int levelCount, Rgba8 baseColor) { + GLuint texture = 0; + glGenTextures(1, &texture); + glBindTexture(GL_TEXTURE_2D, texture); + for (int level = 0; level < levelCount; ++level) { + const GLsizei extent = kSize >> level; + const std::vector pixels = SolidImage(extent, extent, baseColor); + glTexImage2D(GL_TEXTURE_2D, level, GL_RGBA8, extent, extent, 0, GL_RGBA, GL_UNSIGNED_BYTE, + pixels.data()); + } + // What Utils::makeTextureComplete does in the CTS case: the texture is + // complete for the levels it actually has, not for a chain it does not. + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, 0); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, levelCount - 1); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glBindTexture(GL_TEXTURE_2D, 0); + return texture; + } + + void MakePair(int levelCount) { + DeleteTextures(); + m_src = MakeTexture(levelCount, Rgba8{11, 22, 33, 255}); + m_dst = MakeTexture(levelCount, Rgba8{200, 100, 50, 255}); + ASSERT_EQ(glGetError(), GL_NO_ERROR) << "texture setup with " << levelCount << " level(s)"; + } + + // The call under test, at whatever levels the caller wants, over a 1x1 + // region so the region check can never be what rejects it. + GLenum CopyAt(GLint srcLevel, GLint dstLevel, GLsizei extent = 1) { + DrainErrors(); + glCopyImageSubData(m_src, GL_TEXTURE_2D, srcLevel, 0, 0, 0, m_dst, GL_TEXTURE_2D, dstLevel, 0, 0, 0, + extent, extent, 1); + const GLenum error = glGetError(); + // A second pending error would mean the entry point queued more than one, + // and the extra would be handed out at an unrelated call site later. + EXPECT_EQ(glGetError(), GL_NO_ERROR) << "the copy recorded more than one error"; + return error; + } + + Rgba8 ReadBackDestinationLevel0() { + if (m_fbo == 0) glGenFramebuffers(1, &m_fbo); + glBindFramebuffer(GL_FRAMEBUFFER, m_fbo); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, m_dst, 0); + const GLenum status = glCheckFramebufferStatus(GL_FRAMEBUFFER); + if (status != GL_FRAMEBUFFER_COMPLETE) { + ADD_FAILURE() << "readback framebuffer incomplete: " << status; + glBindFramebuffer(GL_FRAMEBUFFER, 0); + return Rgba8{0, 0, 0, 0}; + } + Rgba8 texel{0, 0, 0, 0}; + glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &texel); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + return texel; + } + + GLuint m_src = 0; + GLuint m_dst = 0; + GLuint m_fbo = 0; + }; + + // The regression. Level 1 of a texture that has only level 0 is not a level, and + // saying so is the whole job: before the fix this reached DirectVulkan, which + // handed mipLevel=1 to vkCmdCopyImage on a one-level VkImage and died inside the + // Adreno driver. + TEST_F(CopyImageLevelRangeScenario, LevelOneOfASingleLevelTextureIsRejected) { + if (!Ready()) GTEST_SKIP(); + MakePair(1); + + EXPECT_EQ(CopyAt(1, 0), static_cast(GL_INVALID_VALUE)) << "source level 1"; + EXPECT_EQ(CopyAt(0, 1), static_cast(GL_INVALID_VALUE)) << "destination level 1"; + EXPECT_EQ(CopyAt(1, 1), static_cast(GL_INVALID_VALUE)) << "both levels 1"; + } + + // The negative control that makes the test above falsifiable: the same level + // index, on textures that genuinely have it, must be accepted. A validator that + // rejected every non-zero level would pass the regression test and fail here. + TEST_F(CopyImageLevelRangeScenario, LevelOneOfATwoLevelTextureIsAccepted) { + if (!Ready()) GTEST_SKIP(); + MakePair(2); + + EXPECT_EQ(CopyAt(1, 1), static_cast(GL_NO_ERROR)); + } + + // And the boundary from the other side: two levels means 0 and 1, not 2. + TEST_F(CopyImageLevelRangeScenario, LevelTwoOfATwoLevelTextureIsRejected) { + if (!Ready()) GTEST_SKIP(); + MakePair(2); + + EXPECT_EQ(CopyAt(2, 0), static_cast(GL_INVALID_VALUE)) << "source level 2"; + EXPECT_EQ(CopyAt(0, 2), static_cast(GL_INVALID_VALUE)) << "destination level 2"; + } + + // Errors alone cannot tell an accepted copy from a silently dropped one, so the + // ordinary case is checked by reading the destination back: the copy has to move + // the source's texel, not merely decline to complain. + TEST_F(CopyImageLevelRangeScenario, AValidLevelZeroCopyStillMovesPixels) { + if (!Ready()) GTEST_SKIP(); + MakePair(1); + + ASSERT_EQ(ReadBackDestinationLevel0(), (Rgba8{200, 100, 50, 255})) << "destination before the copy"; + EXPECT_EQ(CopyAt(0, 0, kSize), static_cast(GL_NO_ERROR)); + EXPECT_EQ(ReadBackDestinationLevel0(), (Rgba8{11, 22, 33, 255})) << "destination after the copy"; + } + + } // namespace +} // namespace MGITest diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 934d760b..4c8a86fe 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -1651,6 +1651,100 @@ TEST_F(TextureTest, AnUncompressedRespecificationClearsTheCompressedTag) { EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); } +namespace { + // A 16x16 RGBA8 texture with exactly `levelCount` levels, defined the way + // KHR-GL43.copy_image.non_existent_mipmap defines its textures - glTexImage2D per + // level, NOT glTexStorage2D, because an immutable allocation defines the whole chain + // up front and so cannot express "level 1 does not exist". + GLuint MakeCopyImageTexture(int levelCount) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + for (int level = 0; level < levelCount; ++level) { + const GLsizei extent = 16 >> level; + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, level, GL_RGBA8, extent, extent, 0, GL_RGBA, GL_UNSIGNED_BYTE, + nullptr); + } + return texture; + } +} // namespace + +// KHR-GL43.copy_image.non_existent_mipmap. Level 1 of a texture that has only level 0 is +// not a level: GL 4.6 core 18.3.2 asks for GL_INVALID_VALUE. Until this check existed the +// level travelled all the way into the backends, and DirectVulkan built a VkImageCopy +// naming mip 1 of a VkImage created with one mip - which Adreno answered with a SIGSEGV +// inside vkCmdCopyImage, killing the glcts process in the middle of a negative test. +TEST_F(TextureTest, CopyImageSubDataRejectsALevelTheTextureDoesNotHave) { + const GLuint src = MakeCopyImageTexture(1); + const GLuint dst = MakeCopyImageTexture(1); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + MG_Impl::GLImpl::CopyImageSubData(src, GL_TEXTURE_2D, 1, 0, 0, 0, dst, GL_TEXTURE_2D, 0, 0, 0, 0, 1, 1, 1); + ExpectSingleGlError(GL_INVALID_VALUE); + + MG_Impl::GLImpl::CopyImageSubData(src, GL_TEXTURE_2D, 0, 0, 0, 0, dst, GL_TEXTURE_2D, 1, 0, 0, 0, 1, 1, 1); + ExpectSingleGlError(GL_INVALID_VALUE); + + MG_Impl::GLImpl::CopyImageSubData(src, GL_TEXTURE_2D, 1, 0, 0, 0, dst, GL_TEXTURE_2D, 1, 0, 0, 0, 1, 1, 1); + ExpectSingleGlError(GL_INVALID_VALUE); +} + +// The negative control, and the reason the pair below asks for a zero-sized copy: a +// validator that answered GL_INVALID_VALUE to every non-zero level would satisfy the test +// above. The two calls here are IDENTICAL except for how many levels the textures have, +// and a zero extent makes the validator decline the copy without an error just after the +// level check - so the level count is the only thing either assertion can be reading, and +// no backend (there is none in this binary) is ever reached. +TEST_F(TextureTest, CopyImageSubDataAcceptsALevelTheTextureDoesHave) { + const GLuint oneLevelSrc = MakeCopyImageTexture(1); + const GLuint oneLevelDst = MakeCopyImageTexture(1); + const GLuint twoLevelSrc = MakeCopyImageTexture(2); + const GLuint twoLevelDst = MakeCopyImageTexture(2); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + MG_Impl::GLImpl::CopyImageSubData(oneLevelSrc, GL_TEXTURE_2D, 1, 0, 0, 0, oneLevelDst, GL_TEXTURE_2D, 1, 0, 0, 0, + 0, 0, 0); + ExpectSingleGlError(GL_INVALID_VALUE); + + MG_Impl::GLImpl::CopyImageSubData(twoLevelSrc, GL_TEXTURE_2D, 1, 0, 0, 0, twoLevelDst, GL_TEXTURE_2D, 1, 0, 0, 0, + 0, 0, 0); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "level 1 of a two-level texture is a level"; + + // And the boundary from the other side: two levels means 0 and 1, not 2. + MG_Impl::GLImpl::CopyImageSubData(twoLevelSrc, GL_TEXTURE_2D, 2, 0, 0, 0, twoLevelDst, GL_TEXTURE_2D, 0, 0, 0, 0, + 0, 0, 0); + ExpectSingleGlError(GL_INVALID_VALUE); +} + +// A texture that has never been given an image is a different fault from a level out of +// range, and the spec spells it differently: an incomplete object named by a copy is +// GL_INVALID_OPERATION. Worth pinning because the natural implementation of the check +// above - level >= levelCount - reports INVALID_VALUE for level 0 of a texture whose level +// count is zero, which is the wrong answer to the wrong question. +// +// BOTH textures are imageless on purpose, and that is the whole point rather than symmetry +// for its own sake. With one imageless and one RGBA8 texture the format comparison further +// down already rejected the call, so the case proved nothing about this check. With both +// imageless the formats are Unknown == Unknown, they MATCH, and every validator downstream +// waves the call through - which is how the second crash in this entry point was found: the +// call reached DirectVulkan, SyncTextureAndGetDescriptor returned nothing for a texture with +// no image, and the release build (where the guarding MOBILEGL_ASSERT expands to nothing) +// dereferenced it. Reproduced deterministically on lavapipe by +// KHR-GL43.copy_image.functional_src_target_texture_2d_array_..._dst_format_rgb9_e5. +TEST_F(TextureTest, CopyImageSubDataRejectsTwoTexturesWithNoImageAtAll) { + GLuint firstEmpty = 0; + GLuint secondEmpty = 0; + MG_Impl::GLImpl::GenTextures(1, &firstEmpty); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, firstEmpty); + MG_Impl::GLImpl::GenTextures(1, &secondEmpty); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, secondEmpty); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + MG_Impl::GLImpl::CopyImageSubData(firstEmpty, GL_TEXTURE_2D, 0, 0, 0, 0, secondEmpty, GL_TEXTURE_2D, 0, 0, 0, 0, + 1, 1, 1); + ExpectSingleGlError(GL_INVALID_OPERATION); +} + // GL_DEPTH_STENCIL_TEXTURE_MODE used to be a pure frontend shadow: stored, answered by // glGetTexParameter, and never shown to a backend. Sampling therefore always read the depth // aspect however the mode was set, which is the whole of