mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-08 12:18:30 +09:00
[Fix] (MG_Backend/DirectGLES, MG_Impl/GLImpl): ReadPixels - fall back to wide-format conversion when the ES driver rejects a legacy native read combo (Adreno errors on e.g. GL_RED/GL_UNSIGNED_INT and leaves the buffer untouched), and enforce packed-type/format pairing at the state layer via shared ValidateClientFormatTypePairing (GL_RED + GL_UNSIGNED_SHORT_5_6_5 now raises GL_INVALID_OPERATION)
This commit is contained in:
@@ -3760,7 +3760,30 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
}
|
||||
|
||||
MGLOG_D("ReadPixels: glReadPixels()");
|
||||
DrainESErrors();
|
||||
g_GLESFuncs.glReadPixels(x, y, width, height, format, type, pixels);
|
||||
const GLenum nativeReadError = g_GLESFuncs.glGetError();
|
||||
if (nativeReadError != GL_NO_ERROR) {
|
||||
// ES drivers only guarantee GL_RGBA/GL_UNSIGNED_BYTE, GL_RGBA_INTEGER/(U)INT, float RGBA and one
|
||||
// implementation-defined pair; legacy combos like GL_RED/GL_UNSIGNED_INT are rejected by e.g.
|
||||
// Adreno with a GL error and an untouched destination (GL CTS packed_pixels r8_format_red). The
|
||||
// failed read wrote nothing (client memory and PBO alike), so re-service the request through the
|
||||
// wide-format conversion path before any PBO writeback can capture stale contents. The conversion
|
||||
// helper saves/restores the ES pixel-pack binding and handles the state-layer PBO itself.
|
||||
DrainESErrors();
|
||||
MGLOG_D("ReadPixels: native read of %s/%s failed (%s), retrying via client-format conversion",
|
||||
MG_Util::ConvertGLEnumToString(format).c_str(), MG_Util::ConvertGLEnumToString(type).c_str(),
|
||||
MG_Util::ConvertGLEnumToString(nativeReadError).c_str());
|
||||
if (ReadPixelsViaFormatConversion(x, y, width, height, format, type, pixels)) {
|
||||
MGLOG_D("ReadPixels: finished via client-format conversion after native failure");
|
||||
return;
|
||||
}
|
||||
MGLOG_E("ReadPixels: native read of %s/%s failed (%s) and no conversion path covers it, "
|
||||
"skipping readback",
|
||||
MG_Util::ConvertGLEnumToString(format).c_str(), MG_Util::ConvertGLEnumToString(type).c_str(),
|
||||
MG_Util::ConvertGLEnumToString(nativeReadError).c_str());
|
||||
return;
|
||||
}
|
||||
if (usePBO) {
|
||||
// pull back to client memory if PBO is used
|
||||
MGLOG_D("ReadPixels: PBO used, mapping buffer to client memory");
|
||||
|
||||
@@ -1681,6 +1681,13 @@ namespace MobileGL::MG_Impl::GLImpl {
|
||||
}
|
||||
}
|
||||
|
||||
// Packed-type/format pairing (GL CTS packed_pixels: e.g. GL_RED with GL_UNSIGNED_SHORT_5_6_5 must
|
||||
// raise an error instead of reaching the backend). Shared with the TexImage/GetTexImage validators;
|
||||
// runs after the depth-stencil branch above so DEPTH_STENCIL with a wrong type keeps GL_INVALID_ENUM.
|
||||
if (!TextureImpl::ValidateClientFormatTypePairing(textureInputFormat, texturePixelDataType)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check PBO state
|
||||
const auto& pixelPackBufferObject =
|
||||
MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject();
|
||||
|
||||
@@ -235,17 +235,15 @@ namespace MobileGL::MG_Impl::GLImpl::TextureImpl {
|
||||
format == TextureInputFormat::StencilIndex;
|
||||
}
|
||||
|
||||
// Mirrors the desktop-GL validity matrix used by GL CTS packed_pixels (glcPackedPixelsTests
|
||||
// isFormatValid, INPUT_TEXIMAGE): packed-type/format pairing, depth-vs-color mismatch, and
|
||||
// integer-ness matching all raise GL_INVALID_OPERATION instead of reaching the upload path.
|
||||
Bool ValidateTextureInternalFormatCompatibleWithInput(TextureInputFormat format,
|
||||
TextureInternalFormat internalFormat,
|
||||
TexturePixelDataType type) {
|
||||
// Client-memory format<->type pairing rules shared by pixel uploads (TexImage*) and readbacks
|
||||
// (ReadPixels, GetTexImage). Mirrors the desktop-GL validity matrix used by GL CTS packed_pixels
|
||||
// (glcPackedPixelsTests isFormatValid): packed types constrain the formats they may pair with, and
|
||||
// integer formats reject floating-point types; violations raise GL_INVALID_OPERATION.
|
||||
Bool ValidateClientFormatTypePairing(TextureInputFormat format, TexturePixelDataType type) {
|
||||
const auto recordInvalidOperation = [](const char* message) {
|
||||
MG_State::pGLContext->RecordError(
|
||||
ErrorCode::InvalidOperation,
|
||||
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "ValidateTextureInternalFormatCompatibleWithInput",
|
||||
message));
|
||||
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "ValidateClientFormatTypePairing", message));
|
||||
return false;
|
||||
};
|
||||
|
||||
@@ -288,6 +286,27 @@ namespace MobileGL::MG_Impl::GLImpl::TextureImpl {
|
||||
return recordInvalidOperation("Integer format cannot be used with a floating-point type");
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
// Mirrors the desktop-GL validity matrix used by GL CTS packed_pixels (glcPackedPixelsTests
|
||||
// isFormatValid, INPUT_TEXIMAGE): packed-type/format pairing, depth-vs-color mismatch, and
|
||||
// integer-ness matching all raise GL_INVALID_OPERATION instead of reaching the upload path.
|
||||
Bool ValidateTextureInternalFormatCompatibleWithInput(TextureInputFormat format,
|
||||
TextureInternalFormat internalFormat,
|
||||
TexturePixelDataType type) {
|
||||
const auto recordInvalidOperation = [](const char* message) {
|
||||
MG_State::pGLContext->RecordError(
|
||||
ErrorCode::InvalidOperation,
|
||||
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "ValidateTextureInternalFormatCompatibleWithInput",
|
||||
message));
|
||||
return false;
|
||||
};
|
||||
|
||||
if (!ValidateClientFormatTypePairing(format, type)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// TexImage in core 3.3 has no stencil-only upload path (that arrived with GL 4.4).
|
||||
if (format == TextureInputFormat::StencilIndex) {
|
||||
return recordInvalidOperation("STENCIL_INDEX is not a valid texture upload format");
|
||||
|
||||
@@ -23,6 +23,7 @@ namespace MobileGL::MG_Impl::GLImpl::TextureImpl {
|
||||
Bool ValidateTextureSizeRange(Int width, Int height, Int depth);
|
||||
Bool ValidateTextureInternalFormat(TextureInternalFormat format);
|
||||
Bool ValidateTextureBorderNumber(Int border);
|
||||
Bool ValidateClientFormatTypePairing(TextureInputFormat format, TexturePixelDataType type);
|
||||
Bool ValidateTextureInternalFormatCompatibleWithInput(TextureInputFormat format,
|
||||
TextureInternalFormat internalFormat,
|
||||
TexturePixelDataType type);
|
||||
|
||||
@@ -221,6 +221,40 @@ TEST_F(FramebufferTest, ReadPixelsAllowsPersistentMappedPixelPackBuffer) {
|
||||
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
|
||||
}
|
||||
|
||||
TEST_F(FramebufferTest, ReadPixelsRejectsMismatchedPackedTypeFormatPairs) {
|
||||
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, 4, 4);
|
||||
MG_Impl::GLImpl::NamedFramebufferTexture(framebuffer, GL_COLOR_ATTACHMENT0, texture, 0);
|
||||
MG_Impl::GLImpl::BindFramebuffer(GL_READ_FRAMEBUFFER, framebuffer);
|
||||
|
||||
MG_Backend::gBackendFunctionsTable.GL.ReadPixels = RecordReadPixels;
|
||||
Uint8 pixelStorage[4 * 4 * 4] = {};
|
||||
|
||||
// Packed RGB type with a non-RGB format must never reach the backend (GL CTS packed_pixels
|
||||
// reads GL_RED with GL_UNSIGNED_SHORT_5_6_5 and expects an error).
|
||||
MG_Impl::GLImpl::ReadPixels(0, 0, 4, 4, GL_RED, GL_UNSIGNED_SHORT_5_6_5, pixelStorage);
|
||||
EXPECT_EQ(g_readPixelsCallCount, 0);
|
||||
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
|
||||
|
||||
// Packed RGBA type with a non-RGBA/BGRA format is rejected as well.
|
||||
MG_Impl::GLImpl::ReadPixels(0, 0, 4, 4, GL_RGB, GL_UNSIGNED_INT_8_8_8_8, pixelStorage);
|
||||
EXPECT_EQ(g_readPixelsCallCount, 0);
|
||||
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
|
||||
|
||||
// Packed depth-stencil type requires the DEPTH_STENCIL format.
|
||||
MG_Impl::GLImpl::ReadPixels(0, 0, 4, 4, GL_RGBA, GL_UNSIGNED_INT_24_8, pixelStorage);
|
||||
EXPECT_EQ(g_readPixelsCallCount, 0);
|
||||
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
|
||||
|
||||
// A plain RGBA/UNSIGNED_BYTE readback keeps working.
|
||||
MG_Impl::GLImpl::ReadPixels(0, 0, 4, 4, GL_RGBA, GL_UNSIGNED_BYTE, pixelStorage);
|
||||
EXPECT_EQ(g_readPixelsCallCount, 1);
|
||||
EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
|
||||
}
|
||||
|
||||
TEST_F(FramebufferTest, NamedRenderbufferStorageAndFramebufferAttachDoNotChangeBindings) {
|
||||
GLuint framebuffer = 0;
|
||||
GLuint renderbuffer = 0;
|
||||
|
||||
Reference in New Issue
Block a user