diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index a279a4ec..c5d3227b 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2657,25 +2657,51 @@ namespace MobileGL::MG_Backend::DirectGLES { const auto backendSamples = static_cast(ClampSamplesToBackendSupport( GetFormatCapabilityTargetIndex(targetInternal), textureMipmapObject->GetFormat(), glFormat, static_cast(stateTextureObject->GetSamples()))); - switch (targetInternal) { - case TextureTarget::Texture2DMultisample: - g_GLESFuncs.glTexStorage2DMultisample( - target, backendSamples, glInternalFormat, - static_cast(baseSize.x()), static_cast(baseSize.y()), - stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE); - break; - case TextureTarget::Texture2DMultisampleArray: - g_GLESFuncs.glTexStorage3DMultisample( - target, backendSamples, glInternalFormat, - static_cast(baseSize.x()), static_cast(baseSize.y()), - static_cast(baseSize.z()), - stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE); - break; - default: - MOBILEGL_ASSERT(false, "Unexpected multisample target: %d", static_cast(targetInternal)); - break; + // ES 3.1 8.19 requires width/height (and depth, for the array target) >= 1, + // so a degenerate size has nothing to allocate and must not reach the + // driver. The frontend deallocates such an image rather than defining it + // (GL 4.6 core 8.8), so this is belt and braces for any path that still + // syncs one. + const Bool hasAllocatableSize = + baseSize.x() >= 1 && baseSize.y() >= 1 && + (targetInternal != TextureTarget::Texture2DMultisampleArray || baseSize.z() >= 1); + if (!hasAllocatableSize) { + MGLOG_D("Skipping multisample storage for texture %u: degenerate size (%d, %d, %d)", + m_backendTextureId, baseSize.x(), baseSize.y(), baseSize.z()); + } else { + switch (targetInternal) { + case TextureTarget::Texture2DMultisample: + g_GLESFuncs.glTexStorage2DMultisample( + target, backendSamples, glInternalFormat, + static_cast(baseSize.x()), static_cast(baseSize.y()), + stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE); + break; + case TextureTarget::Texture2DMultisampleArray: + g_GLESFuncs.glTexStorage3DMultisample( + target, backendSamples, glInternalFormat, + static_cast(baseSize.x()), static_cast(baseSize.y()), + static_cast(baseSize.z()), + stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE); + break; + default: + MOBILEGL_ASSERT(false, "Unexpected multisample target: %d", + static_cast(targetInternal)); + break; + } + m_backendStorageImmutable = true; } - m_backendStorageImmutable = true; + // The one storage branch that cleared the ES error queue without ever + // draining it again, so anything this call raised was left for an + // unrelated later query to trip over. Paired with its two siblings now. + DebugImpl::ErrorLopper::Loop([file = __FILE__, line = __LINE__, func = __func__, target, + glInternalFormat, backendSamples](GLenum err) { + MGLOG_D("%s(%s:%d) ES error: %s. glTexStorage*Multisample: target=%s, internalformat=%s, " + "samples=%d", + func, file, line, MG_Util::ConvertGLEnumToString(err).c_str(), + MG_Util::ConvertGLEnumToString(target).c_str(), + MG_Util::ConvertGLEnumToString(glInternalFormat).c_str(), + static_cast(backendSamples)); + }); for (const auto& uploadTarget : uploadTargets) { for (SizeT level = 0; level < mipmapCount; ++level) { textureMipmapObject->MarkStorageDirty(uploadTarget, level, false); diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 5e960ad9..c35b527c 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -590,6 +590,20 @@ namespace MobileGL::MG_Impl::GLImpl { "AllocateMultisampleTextureStorage requires mipmap-backed storage"); auto* textureMipmapObject = static_cast(textureObject.get()); + // GL 4.6 core 8.8: a zero-sized image DEALLOCATES the image rather than defining an + // empty one. Only the multisample pair cares, and it cares a great deal: the CTS's + // per-case state reset clears both DEFAULT multisample textures this way on every + // texture unit, and a "defined" 0x0 default texture stops being skipped by + // IsUndefinedDefaultTexture - it then joins the per-draw sync and bind passes on + // every unit the reset touched, and reaches an ES glTexStorage*Multisample(..., 0, 0) + // that ES 3.1 8.19 makes INVALID_VALUE on every driver there is. A proxy target holds + // no image at all, only the query result, so it keeps recording what was asked for. + if ((width <= 0 || height <= 0 || depth <= 0) && + !TextureImpl::IsProxyTextureTarget(textureUploadTarget)) { + textureObject->SetInternalFormat(TextureInternalFormat::Unknown); + textureMipmapObject->TruncateMipmapLevels(textureUploadTarget, 0); + return; + } textureObject->SetInternalFormat(textureInternalFormat); textureObject->SetSamples(samples); textureObject->SetFixedSampleLocations(fixedsamplelocations == GL_TRUE); @@ -4717,6 +4731,22 @@ namespace MobileGL::MG_Impl::GLImpl { TextureStorage3D(textureObject->GetExternalIndex(), levels, internalformat, width, height, depth); } + // Unlike glTexImage*Multisample, where a zero-sized image is a legal deallocation (see + // AllocateMultisampleTextureStorage), the immutable forms take a strictly positive size: GL + // 4.6 core 8.19 makes width, height or depth < 1 INVALID_VALUE. Without this the shared + // _State helper would deallocate the image and TexStorageMultisample_State would then freeze + // the now-imageless texture as immutable. + static Bool ValidateTexStorageMultisampleSize(GLsizei width, GLsizei height, GLsizei depth, const char* caller) { + if (width >= 1 && height >= 1 && depth >= 1) { + return true; + } + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", caller, + "Immutable multisample storage requires width, height and depth >= 1.")); + return false; + } + // The multisample storage forms allocate exactly what the glTexImage*Multisample ones do, and // then freeze it: TEXTURE_IMMUTABLE_FORMAT becomes TRUE and a second call is INVALID_OPERATION // (GL 4.6 core 8.19). Only the allocation was shared before, so a multisample texture stayed @@ -4737,6 +4767,7 @@ namespace MobileGL::MG_Impl::GLImpl { const TextureTarget textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target); auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit()); if (!ValidateTextureMutable(activeUnit.GetBindingSlot(textureTarget).GetBoundObject(), __func__)) return; + if (!ValidateTexStorageMultisampleSize(width, height, 1, __func__)) return; TexStorageMultisample_State( target, TexImage2DMultisample_State(target, samples, internalformat, width, height, fixedsamplelocations), __func__); @@ -4747,6 +4778,7 @@ namespace MobileGL::MG_Impl::GLImpl { const TextureTarget textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target); auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit()); if (!ValidateTextureMutable(activeUnit.GetBindingSlot(textureTarget).GetBoundObject(), __func__)) return; + if (!ValidateTexStorageMultisampleSize(width, height, depth, __func__)) return; TexStorageMultisample_State(target, TexImage3DMultisample_State(target, samples, internalformat, width, height, depth, fixedsamplelocations), diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 7d98521c..19279b0d 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -2975,6 +2975,58 @@ TEST_F(TextureTest, CtsStyleStateResetOnDefaultTexturesLeavesNoError) { EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "GL_TEXTURE_2D_MULTISAMPLE_ARRAY reset failed"; } +// Clean is not enough: per GL 4.6 core 8.8 that zero-sized reset has to DEALLOCATE the image, +// not define an empty one. gluStateReset runs it on both default multisample textures on every +// texture unit of a 3.2+ context, and a default texture left 'defined' afterwards stops being +// skipped by IsUndefinedDefaultTexture - it then joins the per-draw sync and bind passes on +// every unit the reset touched and reaches an ES glTexStorage*Multisample(..., 0, 0), which ES +// 3.1 8.19 rejects on every driver. +TEST_F(TextureTest, ZeroSizedMultisampleTexImageDeallocatesTheImage) { + MG_Impl::GLImpl::ActiveTexture(GL_TEXTURE0); + + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0); + const auto& defaultMultisample = MG_State::pGLContext->GetTextureUnitObject(0) + .GetBindingSlot(TextureTarget::Texture2DMultisample) + .GetBoundObject(); + MG_Impl::GLImpl::TexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 1, GL_RGBA8, 4, 4, GL_TRUE); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + ASSERT_FALSE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisample.get())); + + MG_Impl::GLImpl::TexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 1, GL_RGBA8, 0, 0, GL_TRUE); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + EXPECT_TRUE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisample.get())); + + // The array target's reset also passes zero LAYERS, which deallocates just the same. + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 0); + const auto& defaultMultisampleArray = MG_State::pGLContext->GetTextureUnitObject(0) + .GetBindingSlot(TextureTarget::Texture2DMultisampleArray) + .GetBoundObject(); + MG_Impl::GLImpl::TexImage3DMultisample(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 1, GL_RGBA8, 4, 4, 2, GL_TRUE); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + ASSERT_FALSE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisampleArray.get())); + + MG_Impl::GLImpl::TexImage3DMultisample(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 1, GL_RGBA8, 4, 4, 0, GL_TRUE); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + EXPECT_TRUE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisampleArray.get())); + + // The immutable forms do NOT share that leniency: GL 4.6 core 8.19 makes a size below 1 + // INVALID_VALUE, and freezing an imageless texture as immutable would be unrecoverable. + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE, texture); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + MG_Impl::GLImpl::TexStorage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 1, GL_RGBA8, 0, 0, GL_TRUE); + ExpectSingleGlError(GL_INVALID_VALUE); + EXPECT_FALSE(MG_State::pGLContext->GetTextureUnitObject(0) + .GetBindingSlot(TextureTarget::Texture2DMultisample) + .GetBoundObject() + ->IsImmutable()); + + MG_Impl::GLImpl::DeleteTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + // ---- GL CTS packed_pixels / texture_swizzle readback root-cause regressions -------------------- TEST_F(TextureTest, NormalizeLegacySizedFormatsMapToCanonicalShadowLayouts) {