From 81ee4d29192dcd0bdef05d39829d7c37e231b0e2 Mon Sep 17 00:00:00 2001 From: shrekshao Date: Wed, 4 Dec 2019 17:05:11 -0800 Subject: [PATCH] Workaround EXT_texture_norm16 for OpenGL ES drivers Implement a workaround for widespread bugs calling glReadPixels with RGBA/UNSIGNED_SHORT against R16/RG16 color attachments. Read back the data using the GL_IMPLEMENTATION_COLOR_READ_FORMAT, and then rearrange the read back pixels to fit the RGBA layout. Also skip RGB16/RGB16_SNORM texture sample test on Nexus 5X/Nexus 6P due to a another driver bug. Bug: chromium:1000354, angleproject:4214, angleproject:4215, angleproject:4245 Change-Id: Iedea6f4136878cac5ad0dec3757c77b73502e1cd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1952166 Reviewed-by: Kenneth Russell Commit-Queue: Shrek Shao --- include/platform/FeaturesGL.h | 8 + src/libANGLE/formatutils.cpp | 2 + src/libANGLE/renderer/gl/FramebufferGL.cpp | 247 ++++++++++++++++-- src/libANGLE/renderer/gl/FramebufferGL.h | 2 + src/libANGLE/renderer/gl/formatutilsgl.cpp | 13 +- src/libANGLE/renderer/gl/formatutilsgl.h | 1 + src/libANGLE/renderer/gl/renderergl_utils.cpp | 6 + src/tests/gl_tests/TextureTest.cpp | 232 ++++++++++++++-- 8 files changed, 460 insertions(+), 51 deletions(-) diff --git a/include/platform/FeaturesGL.h b/include/platform/FeaturesGL.h index ec70c7971..440c6ebdc 100644 --- a/include/platform/FeaturesGL.h +++ b/include/platform/FeaturesGL.h @@ -386,6 +386,14 @@ struct FeaturesGL : FeatureSetBase "All Mac drivers do not handle struct scopes correctly. This workaround overwrites a struct" "name with a unique prefix.", &members, "http://crbug.com/403957"}; + + // Quite some OpenGL ES drivers don't implement readPixels for RGBA/UNSIGNED_SHORT from + // EXT_texture_norm16 correctly + Feature readPixelsUsingImplementationColorReadFormatForNorm16 = { + "read_pixels_using_implementation_color_read_format", FeatureCategory::OpenGLWorkarounds, + "Quite some OpenGL ES drivers don't implement readPixels for RGBA/UNSIGNED_SHORT from " + "EXT_texture_norm16 correctly", + &members, "http://anglebug.com/4214"}; }; inline FeaturesGL::FeaturesGL() = default; diff --git a/src/libANGLE/formatutils.cpp b/src/libANGLE/formatutils.cpp index f9c5ab863..43fa27c5e 100644 --- a/src/libANGLE/formatutils.cpp +++ b/src/libANGLE/formatutils.cpp @@ -992,8 +992,10 @@ static InternalFormatInfoMap BuildInternalFormatInfoMap() // | Internal format |sized | R | G | B | A |S | Format | Type | Component type | SRGB | Texture supported | Filterable | Texture attachment | Renderbuffer | AddRGBAFormat(&map, GL_RED, false, 8, 0, 0, 0, 0, GL_RED, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureRG>, AlwaysSupported, RequireExt<&Extensions::textureRG>, NeverSupported); AddRGBAFormat(&map, GL_RED, false, 8, 0, 0, 0, 0, GL_RED, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported); + AddRGBAFormat(&map, GL_RED, false, 16, 0, 0, 0, 0, GL_RED, GL_UNSIGNED_SHORT, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16>, AlwaysSupported, RequireExt<&Extensions::textureNorm16>, NeverSupported); AddRGBAFormat(&map, GL_RG, false, 8, 8, 0, 0, 0, GL_RG, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureRG>, AlwaysSupported, RequireExt<&Extensions::textureRG>, NeverSupported); AddRGBAFormat(&map, GL_RG, false, 8, 8, 0, 0, 0, GL_RG, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported); + AddRGBAFormat(&map, GL_RG, false, 16, 16, 0, 0, 0, GL_RG, GL_UNSIGNED_SHORT, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16>, AlwaysSupported, RequireExt<&Extensions::textureNorm16>, NeverSupported); AddRGBAFormat(&map, GL_RGB, false, 8, 8, 8, 0, 0, GL_RGB, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported); AddRGBAFormat(&map, GL_RGB, false, 5, 6, 5, 0, 0, GL_RGB, GL_UNSIGNED_SHORT_5_6_5, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported); AddRGBAFormat(&map, GL_RGB, false, 8, 8, 8, 0, 0, GL_RGB, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported); diff --git a/src/libANGLE/renderer/gl/FramebufferGL.cpp b/src/libANGLE/renderer/gl/FramebufferGL.cpp index 486075b4f..05abad871 100644 --- a/src/libANGLE/renderer/gl/FramebufferGL.cpp +++ b/src/libANGLE/renderer/gl/FramebufferGL.cpp @@ -230,6 +230,150 @@ bool IsEmulatedAlphaChannelTextureAttachment(const FramebufferAttachment *attach return textureGL->hasEmulatedAlphaChannel(attachment->getTextureImageIndex()); } +class ScopedEXTTextureNorm16ReadbackWorkaround +{ + public: + ScopedEXTTextureNorm16ReadbackWorkaround() + : tmpPixels(nullptr), clientPixels(nullptr), enabled(false) + {} + + ~ScopedEXTTextureNorm16ReadbackWorkaround() + { + if (tmpPixels) + { + delete[] tmpPixels; + } + } + + angle::Result Initialize(const gl::Context *context, + const gl::Rectangle &area, + GLenum originalReadFormat, + GLenum format, + GLenum type, + GLuint skipBytes, + GLuint rowBytes, + GLuint pixelBytes, + GLubyte *pixels) + { + // Separate from constructor as there may be checked math result exception that needs to + // early return + ASSERT(tmpPixels == nullptr); + ASSERT(clientPixels == nullptr); + + ContextGL *contextGL = GetImplAs(context); + const angle::FeaturesGL &features = GetFeaturesGL(context); + + enabled = features.readPixelsUsingImplementationColorReadFormatForNorm16.enabled && + type == GL_UNSIGNED_SHORT && originalReadFormat == GL_RGBA && + (format == GL_RED || format == GL_RG); + + clientPixels = pixels; + + if (enabled) + { + CheckedNumeric checkedRowBytes(rowBytes); + CheckedNumeric checkedRows(area.height); + CheckedNumeric checkedSkipBytes(skipBytes); + auto checkedAllocatedBytes = checkedSkipBytes + checkedRowBytes * checkedRows; + if (rowBytes < area.width * pixelBytes) + { + checkedAllocatedBytes += area.width * pixelBytes - rowBytes; + } + ANGLE_CHECK_GL_MATH(contextGL, checkedAllocatedBytes.IsValid()); + tmpPixels = new GLubyte[checkedAllocatedBytes.ValueOrDie()]; + memset(tmpPixels, 0, checkedAllocatedBytes.ValueOrDie()); + } + + return angle::Result::Continue; + } + + GLubyte *Pixels() const { return tmpPixels ? tmpPixels : clientPixels; } + + bool IsEnabled() const { return enabled; } + + private: + // Temporarily allocated pixel readback buffer + GLubyte *tmpPixels; + // Client pixel array pointer passed to readPixels + GLubyte *clientPixels; + + bool enabled; +}; + +// Workaround to rearrange pixels read by RED/RG to RGBA for RGBA/UNSIGNED_SHORT pixel type +// combination +angle::Result RearrangeEXTTextureNorm16Pixels(const gl::Context *context, + const gl::Rectangle &area, + GLenum originalReadFormat, + GLenum format, + GLenum type, + GLuint skipBytes, + GLuint rowBytes, + GLuint pixelBytes, + const gl::PixelPackState &pack, + GLubyte *clientPixels, + GLubyte *tmpPixels) +{ + ASSERT(tmpPixels != nullptr); + ASSERT(originalReadFormat == GL_RGBA); + ASSERT(format == GL_RED_EXT || format == GL_RG_EXT); + ASSERT(type == GL_UNSIGNED_SHORT); + + ContextGL *contextGL = GetImplAs(context); + + const gl::InternalFormat &glFormatOriginal = + gl::GetInternalFormatInfo(originalReadFormat, type); + + GLuint originalReadFormatRowBytes = 0; + ANGLE_CHECK_GL_MATH( + contextGL, glFormatOriginal.computeRowPitch(type, area.width, pack.alignment, + pack.rowLength, &originalReadFormatRowBytes)); + GLuint originalReadFormatSkipBytes = 0; + ANGLE_CHECK_GL_MATH(contextGL, + glFormatOriginal.computeSkipBytes(type, originalReadFormatRowBytes, 0, pack, + false, &originalReadFormatSkipBytes)); + + GLuint originalReadFormatPixelBytes = glFormatOriginal.computePixelBytes(type); + GLuint alphaChannelBytes = glFormatOriginal.alphaBits / 8; + + ASSERT(originalReadFormatPixelBytes > pixelBytes); + ASSERT(originalReadFormatPixelBytes > alphaChannelBytes); + ASSERT(alphaChannelBytes != 0); + ASSERT(glFormatOriginal.alphaBits % 8 == 0); + + // Populating rearrangedPixels values from pixels + GLubyte *srcRowStart = tmpPixels; + GLubyte *dstRowStart = clientPixels; + + srcRowStart += skipBytes; + dstRowStart += originalReadFormatSkipBytes; + + for (GLint y = 0; y < area.height; ++y) + { + GLubyte *src = srcRowStart; + GLubyte *dst = dstRowStart; + for (GLint x = 0; x < area.width; ++x) + { + GLushort *srcPixel = reinterpret_cast(src); + GLushort *dstPixel = reinterpret_cast(dst); + dstPixel[0] = srcPixel[0]; + dstPixel[1] = format == GL_RG ? srcPixel[1] : 0; + // Set other channel of RGBA to 0 (GB when format == GL_RED, B when format == GL_RG) + dstPixel[2] = 0; + // Set alpha channel to 1 + dstPixel[3] = 0xFFFF; + + src += pixelBytes; + dst += originalReadFormatPixelBytes; + } + + srcRowStart += rowBytes; + dstRowStart += originalReadFormatRowBytes; + } + + return angle::Result::Continue; +} + } // namespace FramebufferGL::FramebufferGL(const gl::FramebufferState &data, @@ -483,7 +627,8 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context, const angle::FeaturesGL &features = GetFeaturesGL(context); // Clip read area to framebuffer. - const gl::Extents fbSize = getState().getReadAttachment()->getSize(); + const auto *readAttachment = mState.getReadAttachment(); + const gl::Extents fbSize = readAttachment->getSize(); const gl::Rectangle fbRect(0, 0, fbSize.width, fbSize.height); gl::Rectangle clippedArea; if (!ClipRectangle(area, fbRect, &clippedArea)) @@ -496,10 +641,20 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context, const gl::Buffer *packBuffer = context->getState().getTargetBuffer(gl::BufferBinding::PixelPack); + GLenum attachmentReadFormat = + readAttachment->getFormat().info->getReadPixelsFormat(context->getExtensions()); nativegl::ReadPixelsFormat readPixelsFormat = - nativegl::GetReadPixelsFormat(functions, features, format, type); + nativegl::GetReadPixelsFormat(functions, features, attachmentReadFormat, format, type); GLenum readFormat = readPixelsFormat.format; GLenum readType = readPixelsFormat.type; + if (features.readPixelsUsingImplementationColorReadFormatForNorm16.enabled && + readType == GL_UNSIGNED_SHORT) + { + ANGLE_CHECK(contextGL, readFormat == GL_RED || readFormat == GL_RG || readFormat == GL_RGBA, + "glReadPixels: GL_IMPLEMENTATION_COLOR_READ_FORMAT advertised by the driver is " + "not handled by RGBA16 readPixels workaround.", + GL_INVALID_OPERATION); + } stateManager->bindFramebuffer(GL_READ_FRAMEBUFFER, mFramebufferID); @@ -535,7 +690,8 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context, if (cannotSetDesiredRowLength || useOverlappingRowsWorkaround) { - return readPixelsRowByRow(context, clippedArea, readFormat, readType, packState, outPtr); + return readPixelsRowByRow(context, clippedArea, format, readFormat, readType, packState, + outPtr); } bool useLastRowPaddingWorkaround = false; @@ -546,8 +702,8 @@ angle::Result FramebufferGL::readPixels(const gl::Context *context, readFormat, readType, false, outPtr, &useLastRowPaddingWorkaround)); } - return readPixelsAllAtOnce(context, clippedArea, readFormat, readType, packState, outPtr, - useLastRowPaddingWorkaround); + return readPixelsAllAtOnce(context, clippedArea, format, readFormat, readType, packState, + outPtr, useLastRowPaddingWorkaround); } angle::Result FramebufferGL::blit(const gl::Context *context, @@ -1275,14 +1431,16 @@ bool FramebufferGL::modifyInvalidateAttachmentsForEmulatedDefaultFBO( angle::Result FramebufferGL::readPixelsRowByRow(const gl::Context *context, const gl::Rectangle &area, + GLenum originalReadFormat, GLenum format, GLenum type, const gl::PixelPackState &pack, GLubyte *pixels) const { - ContextGL *contextGL = GetImplAs(context); - const FunctionsGL *functions = GetFunctionsGL(context); - StateManagerGL *stateManager = GetStateManagerGL(context); + ContextGL *contextGL = GetImplAs(context); + const FunctionsGL *functions = GetFunctionsGL(context); + StateManagerGL *stateManager = GetStateManagerGL(context); + GLubyte *originalReadFormatPixels = pixels; const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(format, type); @@ -1293,15 +1451,32 @@ angle::Result FramebufferGL::readPixelsRowByRow(const gl::Context *context, ANGLE_CHECK_GL_MATH(contextGL, glFormat.computeSkipBytes(type, rowBytes, 0, pack, false, &skipBytes)); + ScopedEXTTextureNorm16ReadbackWorkaround workaround; + angle::Result result = + workaround.Initialize(context, area, originalReadFormat, format, type, skipBytes, rowBytes, + glFormat.computePixelBytes(type), pixels); + if (result != angle::Result::Continue) + { + return result; + } + gl::PixelPackState directPack; directPack.alignment = 1; stateManager->setPixelPackState(directPack); - pixels += skipBytes; + GLubyte *readbackPixels = workaround.Pixels(); + readbackPixels += skipBytes; for (GLint y = area.y; y < area.y + area.height; ++y) { - functions->readPixels(area.x, y, area.width, 1, format, type, pixels); - pixels += rowBytes; + functions->readPixels(area.x, y, area.width, 1, format, type, readbackPixels); + readbackPixels += rowBytes; + } + + if (workaround.IsEnabled()) + { + return RearrangeEXTTextureNorm16Pixels( + context, area, originalReadFormat, format, type, skipBytes, rowBytes, + glFormat.computePixelBytes(type), pack, originalReadFormatPixels, workaround.Pixels()); } return angle::Result::Continue; @@ -1309,41 +1484,61 @@ angle::Result FramebufferGL::readPixelsRowByRow(const gl::Context *context, angle::Result FramebufferGL::readPixelsAllAtOnce(const gl::Context *context, const gl::Rectangle &area, + GLenum originalReadFormat, GLenum format, GLenum type, const gl::PixelPackState &pack, GLubyte *pixels, bool readLastRowSeparately) const { - ContextGL *contextGL = GetImplAs(context); - const FunctionsGL *functions = GetFunctionsGL(context); - StateManagerGL *stateManager = GetStateManagerGL(context); + ContextGL *contextGL = GetImplAs(context); + const FunctionsGL *functions = GetFunctionsGL(context); + StateManagerGL *stateManager = GetStateManagerGL(context); + GLubyte *originalReadFormatPixels = pixels; + + const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(format, type); + + GLuint rowBytes = 0; + ANGLE_CHECK_GL_MATH(contextGL, glFormat.computeRowPitch(type, area.width, pack.alignment, + pack.rowLength, &rowBytes)); + GLuint skipBytes = 0; + ANGLE_CHECK_GL_MATH(contextGL, + glFormat.computeSkipBytes(type, rowBytes, 0, pack, false, &skipBytes)); + + ScopedEXTTextureNorm16ReadbackWorkaround workaround; + angle::Result result = + workaround.Initialize(context, area, originalReadFormat, format, type, skipBytes, rowBytes, + glFormat.computePixelBytes(type), pixels); + if (result != angle::Result::Continue) + { + return result; + } GLint height = area.height - readLastRowSeparately; if (height > 0) { stateManager->setPixelPackState(pack); - functions->readPixels(area.x, area.y, area.width, height, format, type, pixels); + functions->readPixels(area.x, area.y, area.width, height, format, type, + workaround.Pixels()); } if (readLastRowSeparately) { - const gl::InternalFormat &glFormat = gl::GetInternalFormatInfo(format, type); - - GLuint rowBytes = 0; - ANGLE_CHECK_GL_MATH(contextGL, glFormat.computeRowPitch(type, area.width, pack.alignment, - pack.rowLength, &rowBytes)); - GLuint skipBytes = 0; - ANGLE_CHECK_GL_MATH(contextGL, - glFormat.computeSkipBytes(type, rowBytes, 0, pack, false, &skipBytes)); - gl::PixelPackState directPack; directPack.alignment = 1; stateManager->setPixelPackState(directPack); - pixels += skipBytes + (area.height - 1) * rowBytes; + GLubyte *readbackPixels = workaround.Pixels(); + readbackPixels += skipBytes + (area.height - 1) * rowBytes; functions->readPixels(area.x, area.y + area.height - 1, area.width, 1, format, type, - pixels); + readbackPixels); + } + + if (workaround.IsEnabled()) + { + return RearrangeEXTTextureNorm16Pixels( + context, area, originalReadFormat, format, type, skipBytes, rowBytes, + glFormat.computePixelBytes(type), pack, originalReadFormatPixels, workaround.Pixels()); } return angle::Result::Continue; diff --git a/src/libANGLE/renderer/gl/FramebufferGL.h b/src/libANGLE/renderer/gl/FramebufferGL.h index fc276360f..6226575e4 100644 --- a/src/libANGLE/renderer/gl/FramebufferGL.h +++ b/src/libANGLE/renderer/gl/FramebufferGL.h @@ -101,6 +101,7 @@ class FramebufferGL : public FramebufferImpl angle::Result readPixelsRowByRow(const gl::Context *context, const gl::Rectangle &area, + GLenum originalReadFormat, GLenum format, GLenum type, const gl::PixelPackState &pack, @@ -108,6 +109,7 @@ class FramebufferGL : public FramebufferImpl angle::Result readPixelsAllAtOnce(const gl::Context *context, const gl::Rectangle &area, + GLenum originalReadFormat, GLenum format, GLenum type, const gl::PixelPackState &pack, diff --git a/src/libANGLE/renderer/gl/formatutilsgl.cpp b/src/libANGLE/renderer/gl/formatutilsgl.cpp index b600470ec..d43522836 100644 --- a/src/libANGLE/renderer/gl/formatutilsgl.cpp +++ b/src/libANGLE/renderer/gl/formatutilsgl.cpp @@ -655,9 +655,17 @@ static GLenum GetNativeReadType(const FunctionsGL *functions, static GLenum GetNativeReadFormat(const FunctionsGL *functions, const angle::FeaturesGL &features, - GLenum format) + GLenum attachmentReadFormat, + GLenum format, + GLenum type) { GLenum result = format; + if (features.readPixelsUsingImplementationColorReadFormatForNorm16.enabled && + type == GL_UNSIGNED_SHORT && format == GL_RGBA && + (attachmentReadFormat == GL_RED || attachmentReadFormat == GL_RG)) + { + result = attachmentReadFormat; + } return result; } @@ -736,11 +744,12 @@ RenderbufferFormat GetRenderbufferFormat(const FunctionsGL *functions, } ReadPixelsFormat GetReadPixelsFormat(const FunctionsGL *functions, const angle::FeaturesGL &features, + GLenum attachmentReadFormat, GLenum format, GLenum type) { ReadPixelsFormat result; - result.format = GetNativeReadFormat(functions, features, format); + result.format = GetNativeReadFormat(functions, features, attachmentReadFormat, format, type); result.type = GetNativeReadType(functions, features, type); return result; } diff --git a/src/libANGLE/renderer/gl/formatutilsgl.h b/src/libANGLE/renderer/gl/formatutilsgl.h index c59ff9bbc..1195290b0 100644 --- a/src/libANGLE/renderer/gl/formatutilsgl.h +++ b/src/libANGLE/renderer/gl/formatutilsgl.h @@ -131,6 +131,7 @@ struct ReadPixelsFormat }; ReadPixelsFormat GetReadPixelsFormat(const FunctionsGL *functions, const angle::FeaturesGL &features, + GLenum readAttachmentFormat, GLenum format, GLenum type); } // namespace nativegl diff --git a/src/libANGLE/renderer/gl/renderergl_utils.cpp b/src/libANGLE/renderer/gl/renderergl_utils.cpp index b646ef719..625bc98f0 100644 --- a/src/libANGLE/renderer/gl/renderergl_utils.cpp +++ b/src/libANGLE/renderer/gl/renderergl_utils.cpp @@ -1623,6 +1623,12 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature // Ported from gpu_driver_bug_list.json (#184) ANGLE_FEATURE_CONDITION(features, preAddTexelFetchOffsets, IsApple() && isIntel); + + // Workaround for the widespread OpenGL ES driver implementaion bug + ANGLE_FEATURE_CONDITION(features, readPixelsUsingImplementationColorReadFormatForNorm16, + functions->standard == STANDARD_GL_ES && + functions->isAtLeastGLES(gl::Version(3, 1)) && + functions->hasGLESExtension("GL_EXT_texture_norm16")); } void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features) diff --git a/src/tests/gl_tests/TextureTest.cpp b/src/tests/gl_tests/TextureTest.cpp index d3f105fd7..68f84622c 100644 --- a/src/tests/gl_tests/TextureTest.cpp +++ b/src/tests/gl_tests/TextureTest.cpp @@ -4124,6 +4124,10 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3 void testNorm16Texture(GLint internalformat, GLenum format, GLenum type) { + // TODO(http://anglebug.com/4089) Fails on Win Intel OpenGL driver + ANGLE_SKIP_TEST_IF(IsIntel() && IsOpenGL()); + ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_norm16")); + GLushort pixelValue = (type == GL_SHORT) ? 0x7FFF : 0x6A35; GLushort imageData[] = {pixelValue, pixelValue, pixelValue, pixelValue}; @@ -4154,15 +4158,101 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3 ASSERT_GL_NO_ERROR(); } - void testNorm16Render(GLint internalformat, GLenum format, GLenum type) + void testReadPixelsRGBAWithRangeAndPixelStoreMode(GLuint x, + GLuint y, + GLuint width, + GLuint height, + GLint packRowLength, + GLint packAlignment, + GLint packSkipPixels, + GLint packSkipRows, + GLenum type, + GLColor16UI color) { + // PACK modes debugging + GLint s = 2; // single component size in bytes, UNSIGNED_SHORT -> 2 in our case + GLint n = 4; // 4 components per pixel, stands for GL_RGBA + + GLuint l = packRowLength == 0 ? width : packRowLength; + const GLint &a = packAlignment; + + // According to + // https://www.khronos.org/registry/OpenGL-Refpages/es3.0/html/glPixelStorei.xhtml + GLint k = (s >= a) ? n * l : a / s * (1 + (s * n * l - 1) / a); + std::size_t componentCount = n * packSkipPixels + k * (packSkipRows + height); + if (static_cast(packRowLength) < width) + { + componentCount += width * n * s - k; + } + + // Populate the pixels array with random dirty value + constexpr GLushort kDirtyValue = 0x1234; + std::vector pixels(componentCount, kDirtyValue); + glReadPixels(x, y, width, height, GL_RGBA, type, pixels.data()); + + EXPECT_GL_NO_ERROR(); + + GLushort *pixelRowStart = pixels.data(); + pixelRowStart += n * packSkipPixels + k * packSkipRows; + + std::vector modifiedPixels(componentCount, false); + + char errorInfo[200]; + + for (GLuint y = 0; y < height; ++y) + { + GLushort *curPixel = pixelRowStart; + for (GLuint x = 0, len = (y == height - 1) ? width : std::min(l, width); x < len; ++x) + { + snprintf(errorInfo, sizeof(errorInfo), + "extent: {%u, %u}, coord: (%u, %u), rowLength: %d, alignment: %d, " + "skipPixels: %d, skipRows: %d\n", + width, height, x, y, packRowLength, packAlignment, packSkipPixels, + packSkipRows); + EXPECT_EQ(color.R, curPixel[0]) << errorInfo; + EXPECT_EQ(color.G, curPixel[1]) << errorInfo; + EXPECT_EQ(color.B, curPixel[2]) << errorInfo; + EXPECT_EQ(color.A, curPixel[3]) << errorInfo; + + std::ptrdiff_t diff = curPixel - pixels.data(); + modifiedPixels[diff + 0] = true; + modifiedPixels[diff + 1] = true; + modifiedPixels[diff + 2] = true; + modifiedPixels[diff + 3] = true; + + curPixel += n; + } + pixelRowStart += k; + } + + for (std::size_t i = 0; i < modifiedPixels.size(); ++i) + { + if (!modifiedPixels[i]) + { + EXPECT_EQ(pixels[i], kDirtyValue); + } + } + } + + void testNorm16RenderAndReadPixels(GLint internalformat, GLenum format, GLenum type) + { + // TODO(http://anglebug.com/4089) Fails on Win Intel OpenGL driver + ANGLE_SKIP_TEST_IF(IsIntel() && IsOpenGL()); + // TODO(http://anglebug.com/4245) Fails on Win AMD OpenGL driver + ANGLE_SKIP_TEST_IF(IsWindows() && IsAMD() && IsDesktopOpenGL()); + ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_norm16")); + GLushort pixelValue = 0x6A35; GLushort imageData[] = {pixelValue, pixelValue, pixelValue, pixelValue}; + GLColor16UI color = SliceFormatColor16UI( + format, GLColor16UI(pixelValue, pixelValue, pixelValue, pixelValue)); + // Size of drawing viewport + constexpr GLint width = 8, height = 8; setUpProgram(); glBindTexture(GL_TEXTURE_2D, mTextures[1]); - glTexImage2D(GL_TEXTURE_2D, 0, internalformat, 1, 1, 0, format, type, nullptr); + glTexImage2D(GL_TEXTURE_2D, 0, internalformat, width, height, 0, format, type, nullptr); glBindFramebuffer(GL_FRAMEBUFFER, mFBO); glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[1], @@ -4170,14 +4260,72 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3 glBindTexture(GL_TEXTURE_2D, mTextures[2]); glTexImage2D(GL_TEXTURE_2D, 0, internalformat, 1, 1, 0, format, type, imageData); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); EXPECT_GL_NO_ERROR(); drawQuad(mProgram, "position", 0.5f); - EXPECT_PIXEL_16UI_COLOR(0, 0, - SliceFormatColor16UI(format, GLColor16UI(pixelValue, pixelValue, - pixelValue, pixelValue))); + // ReadPixels against different width, height, pixel pack mode combinations to test + // workaround of pixels rearrangement + + // {x, y, width, height} + std::vector> areas = { + {0, 0, 1, 1}, {0, 0, 1, 2}, {0, 0, 2, 1}, {0, 0, 2, 2}, + {0, 0, 3, 2}, {0, 0, 3, 3}, {0, 0, 4, 3}, {0, 0, 4, 4}, + + {1, 3, 3, 2}, {1, 3, 3, 3}, {3, 2, 4, 3}, {3, 2, 4, 4}, + + {0, 0, 5, 6}, {2, 1, 5, 6}, {0, 0, 6, 1}, {0, 0, 7, 1}, + {0, 0, 7, 3}, {0, 0, 7, 8}, {1, 0, 7, 8}, {0, 0, 8, 8}, + }; + + // Put default settings at the last + std::vector paramsPackRowLength = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0}; + std::vector paramsPackAlignment = {1, 2, 8, 4}; + std::vector> paramsPackSkipPixelsAndRows = {{1, 0}, {0, 1}, {1, 1}, + {3, 1}, {20, 20}, {0, 0}}; + + // Restore pixel pack modes later + GLint restorePackAlignment; + glGetIntegerv(GL_PACK_ALIGNMENT, &restorePackAlignment); + GLint restorePackRowLength; + glGetIntegerv(GL_PACK_ROW_LENGTH, &restorePackRowLength); + GLint restorePackSkipPixels; + glGetIntegerv(GL_PACK_SKIP_PIXELS, &restorePackSkipPixels); + GLint restorePackSkipRows; + glGetIntegerv(GL_PACK_SKIP_ROWS, &restorePackSkipRows); + + // Variable symbols are based on: + // https://www.khronos.org/registry/OpenGL-Refpages/es3.0/html/glPixelStorei.xhtml + for (const auto &skipped : paramsPackSkipPixelsAndRows) + { + glPixelStorei(GL_PACK_SKIP_PIXELS, skipped[0]); + glPixelStorei(GL_PACK_SKIP_ROWS, skipped[1]); + for (GLint a : paramsPackAlignment) + { + glPixelStorei(GL_PACK_ALIGNMENT, a); + for (GLint l : paramsPackRowLength) + { + glPixelStorei(GL_PACK_ROW_LENGTH, l); + + for (const auto &area : areas) + { + ASSERT(area[0] + area[2] <= width); + ASSERT(area[1] + area[3] <= height); + testReadPixelsRGBAWithRangeAndPixelStoreMode(area[0], area[1], area[2], + area[3], l, a, skipped[0], + skipped[1], type, color); + } + } + } + } + + glPixelStorei(GL_PACK_ALIGNMENT, restorePackAlignment); + glPixelStorei(GL_PACK_ROW_LENGTH, restorePackRowLength); + glPixelStorei(GL_PACK_SKIP_PIXELS, restorePackSkipPixels); + glPixelStorei(GL_PACK_SKIP_ROWS, restorePackSkipRows); glBindRenderbuffer(GL_RENDERBUFFER, mRenderbuffer); glRenderbufferStorage(GL_RENDERBUFFER, internalformat, 1, 1); @@ -4210,27 +4358,65 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3 GLuint mRenderbuffer; }; -// Test texture formats enabled by the GL_EXT_texture_norm16 extension. -TEST_P(Texture2DNorm16TestES3, TextureNorm16Test) +TEST_P(Texture2DNorm16TestES3, TextureNorm16R16TextureTest) { - // TODO(crbug.com/angleproject/4089) Fails on Nexus5X Adreno - // TODO(crbug.com/1024387) Fails on Nexus6P - ANGLE_SKIP_TEST_IF(IsNexus5X() || IsNexus6P()); - // TODO(crbug.com/angleproject/4089) Fails on Win Intel OpenGL driver - ANGLE_SKIP_TEST_IF(IsIntel() && IsOpenGL()); - - ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_texture_norm16")); - testNorm16Texture(GL_R16_EXT, GL_RED, GL_UNSIGNED_SHORT); - testNorm16Texture(GL_RG16_EXT, GL_RG, GL_UNSIGNED_SHORT); - testNorm16Texture(GL_RGB16_EXT, GL_RGB, GL_UNSIGNED_SHORT); - testNorm16Texture(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT); - testNorm16Texture(GL_R16_SNORM_EXT, GL_RED, GL_SHORT); - testNorm16Texture(GL_RG16_SNORM_EXT, GL_RG, GL_SHORT); - testNorm16Texture(GL_RGB16_SNORM_EXT, GL_RGB, GL_SHORT); - testNorm16Texture(GL_RGBA16_SNORM_EXT, GL_RGBA, GL_SHORT); +} - testNorm16Render(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT); +TEST_P(Texture2DNorm16TestES3, TextureNorm16R16SNORMTextureTest) +{ + testNorm16Texture(GL_R16_SNORM_EXT, GL_RED, GL_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RG16TextureTest) +{ + testNorm16Texture(GL_RG16_EXT, GL_RG, GL_UNSIGNED_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RG16SNORMTextureTest) +{ + testNorm16Texture(GL_RG16_SNORM_EXT, GL_RG, GL_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RGB16TextureTest) +{ + // (http://anglebug.com/4215) Driver bug on some Qualcomm Adreno gpu + ANGLE_SKIP_TEST_IF((IsNexus5X() || IsNexus6P()) && IsOpenGLES()); + + testNorm16Texture(GL_RGB16_EXT, GL_RGB, GL_UNSIGNED_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RGB16SNORMTextureTest) +{ + // (http://anglebug.com/4215) Driver bug on some Qualcomm Adreno gpu + ANGLE_SKIP_TEST_IF((IsNexus5X() || IsNexus6P()) && IsOpenGLES()); + + testNorm16Texture(GL_RGB16_SNORM_EXT, GL_RGB, GL_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RGBA16TextureTest) +{ + testNorm16Texture(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RGBA16SNORMTextureTest) +{ + testNorm16Texture(GL_RGBA16_SNORM_EXT, GL_RGBA, GL_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16R16RenderTest) +{ + testNorm16RenderAndReadPixels(GL_R16_EXT, GL_RED, GL_UNSIGNED_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RG16RenderTest) +{ + testNorm16RenderAndReadPixels(GL_RG16_EXT, GL_RG, GL_UNSIGNED_SHORT); +} + +TEST_P(Texture2DNorm16TestES3, TextureNorm16RGBA16RenderTest) +{ + testNorm16RenderAndReadPixels(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT); } class Texture2DRGTest : public Texture2DTest