Fix unpacking overlapping unpack buffer rows on NVIDIA GL

src/libANGLE/renderer/gl/StateManagerGL.cpp

@@ -432,7 +432,7 @@ void StateManagerGL::setPixelUnpackState(GLint alignment,
         mUnpackSkipRows = skipRows;
         mFunctions->pixelStorei(GL_UNPACK_SKIP_ROWS, mUnpackSkipRows);
 
-        // TODO: set dirty bit once one exists
+        mLocalDirtyBits.set(gl::State::DIRTY_BIT_UNPACK_SKIP_ROWS);
     }
 
     if (mUnpackSkipPixels != skipPixels)
@@ -440,7 +440,7 @@ void StateManagerGL::setPixelUnpackState(GLint alignment,
         mUnpackSkipPixels = skipPixels;
         mFunctions->pixelStorei(GL_UNPACK_SKIP_PIXELS, mUnpackSkipPixels);
 
-        // TODO: set dirty bit once one exists
+        mLocalDirtyBits.set(gl::State::DIRTY_BIT_UNPACK_SKIP_PIXELS);
     }
 
     if (mUnpackImageHeight != imageHeight)
@@ -448,7 +448,7 @@ void StateManagerGL::setPixelUnpackState(GLint alignment,
         mUnpackImageHeight = imageHeight;
         mFunctions->pixelStorei(GL_UNPACK_IMAGE_HEIGHT, mUnpackImageHeight);
 
-        // TODO: set dirty bit once one exists
+        mLocalDirtyBits.set(gl::State::DIRTY_BIT_UNPACK_IMAGE_HEIGHT);
     }
 
     if (mUnpackSkipImages != skipImages)
@@ -456,7 +456,7 @@ void StateManagerGL::setPixelUnpackState(GLint alignment,
         mUnpackSkipImages = skipImages;
         mFunctions->pixelStorei(GL_UNPACK_SKIP_IMAGES, mUnpackSkipImages);
 
-        // TODO: set dirty bit once one exists
+        mLocalDirtyBits.set(gl::State::DIRTY_BIT_UNPACK_SKIP_IMAGES);
     }
 
     bindBuffer(GL_PIXEL_UNPACK_BUFFER, unpackBuffer);
@@ -1616,4 +1616,9 @@ void StateManagerGL::setTextureCubemapSeamlessEnabled(bool enabled)
     }
 }
 
+GLuint StateManagerGL::getBoundBuffer(GLenum type)
+{
+    ASSERT(mBuffers.find(type) != mBuffers.end());
+    return mBuffers[type];
+}
 }

src/libANGLE/renderer/gl/StateManagerGL.h

@@ -146,6 +146,8 @@ class StateManagerGL final : angle::NonCopyable
 
     void syncState(const gl::State &state, const gl::State::DirtyBits &glDirtyBits);
 
+    GLuint getBoundBuffer(GLenum type);
+
   private:
     gl::Error setGenericDrawState(const gl::ContextState &data);
 

src/libANGLE/renderer/gl/TextureGL.cpp

@@ -137,6 +137,30 @@ TextureGL::~TextureGL()
 gl::Error TextureGL::setImage(GLenum target, size_t level, GLenum internalFormat, const gl::Extents &size, GLenum format, GLenum type,
                               const gl::PixelUnpackState &unpack, const uint8_t *pixels)
 {
+    if (mWorkarounds.unpackOverlappingRowsSeparatelyUnpackBuffer && unpack.pixelBuffer.get() &&
+        unpack.rowLength != 0 && unpack.rowLength < size.width)
+    {
+        // The rows overlap in unpack memory. Upload the texture row by row to work around
+        // driver bug.
+        reserveTexImageToBeFilled(target, level, internalFormat, size, format, type);
+        gl::Box area(0, 0, 0, size.width, size.height, size.depth);
+        ANGLE_TRY(setSubImageRowByRowWorkaround(target, level, area, format, type, unpack, pixels));
+    }
+    else
+    {
+        setImageHelper(target, level, internalFormat, size, format, type, pixels);
+    }
+    return gl::NoError();
+}
+
+void TextureGL::setImageHelper(GLenum target,
+                               size_t level,
+                               GLenum internalFormat,
+                               const gl::Extents &size,
+                               GLenum format,
+                               GLenum type,
+                               const uint8_t *pixels)
+{
     UNUSED_ASSERTION_VARIABLE(&CompatibleTextureTarget); // Reference this function to avoid warnings.
     ASSERT(CompatibleTextureTarget(mState.mTarget, target));
 
@@ -144,6 +168,7 @@ gl::Error TextureGL::setImage(GLenum target, size_t level, GLenum internalFormat
         nativegl::GetTexImageFormat(mFunctions, mWorkarounds, internalFormat, format, type);
 
     mStateManager->bindTexture(mState.mTarget, mTextureID);
+
     if (UseTexImage2D(mState.mTarget))
     {
         ASSERT(size.depth == 1);
@@ -163,8 +188,20 @@ gl::Error TextureGL::setImage(GLenum target, size_t level, GLenum internalFormat
     }
 
     mLevelInfo[level] = GetLevelInfo(internalFormat, texImageFormat.internalFormat);
+}
 
-    return gl::Error(GL_NO_ERROR);
+void TextureGL::reserveTexImageToBeFilled(GLenum target,
+                                          size_t level,
+                                          GLenum internalFormat,
+                                          const gl::Extents &size,
+                                          GLenum format,
+                                          GLenum type)
+{
+    GLuint unpackBuffer = mStateManager->getBoundBuffer(GL_PIXEL_UNPACK_BUFFER);
+    mStateManager->bindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
+    gl::PixelUnpackState unpack;
+    setImageHelper(target, level, internalFormat, size, format, type, nullptr);
+    mStateManager->bindBuffer(GL_PIXEL_UNPACK_BUFFER, unpackBuffer);
 }
 
 gl::Error TextureGL::setSubImage(GLenum target, size_t level, const gl::Box &area, GLenum format, GLenum type,
@@ -176,7 +213,12 @@ gl::Error TextureGL::setSubImage(GLenum target, size_t level, const gl::Box &are
         nativegl::GetTexSubImageFormat(mFunctions, mWorkarounds, format, type);
 
     mStateManager->bindTexture(mState.mTarget, mTextureID);
-    if (UseTexImage2D(mState.mTarget))
+    if (mWorkarounds.unpackOverlappingRowsSeparatelyUnpackBuffer && unpack.pixelBuffer.get() &&
+        unpack.rowLength != 0 && unpack.rowLength < area.width)
+    {
+        ANGLE_TRY(setSubImageRowByRowWorkaround(target, level, area, format, type, unpack, pixels));
+    }
+    else if (UseTexImage2D(mState.mTarget))
     {
         ASSERT(area.z == 0 && area.depth == 1);
         mFunctions->texSubImage2D(target, static_cast<GLint>(level), area.x, area.y, area.width,
@@ -200,6 +242,69 @@ gl::Error TextureGL::setSubImage(GLenum target, size_t level, const gl::Box &are
     return gl::Error(GL_NO_ERROR);
 }
 
+gl::Error TextureGL::setSubImageRowByRowWorkaround(GLenum target,
+                                                   size_t level,
+                                                   const gl::Box &area,
+                                                   GLenum format,
+                                                   GLenum type,
+                                                   const gl::PixelUnpackState &unpack,
+                                                   const uint8_t *pixels)
+{
+    gl::PixelUnpackState unpackToUse;
+    unpackToUse.pixelBuffer = unpack.pixelBuffer;
+    mStateManager->setPixelUnpackState(unpackToUse);
+    unpackToUse.pixelBuffer.set(nullptr);
+    GLenum sizedFormat =
+        gl::GetSizedInternalFormat(mState.getImageDesc(mState.mTarget, level).internalFormat, type);
+    const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(sizedFormat);
+    GLuint rowBytes                      = 0;
+    ANGLE_TRY_RESULT(
+        formatInfo.computeRowPitch(GL_NONE, area.width, unpack.alignment, unpack.rowLength),
+        rowBytes);
+    GLuint imageBytes = 0;
+    ANGLE_TRY_RESULT(
+        formatInfo.computeDepthPitch(GL_NONE, area.width, area.height, unpack.alignment,
+                                     unpack.rowLength, unpack.imageHeight),
+        imageBytes);
+    bool useTexImage3D = UseTexImage3D(mState.mTarget);
+    GLuint skipBytes   = 0;
+    ANGLE_TRY_RESULT(formatInfo.computeSkipBytes(rowBytes, imageBytes, unpack.skipImages,
+                                                 unpack.skipRows, unpack.skipPixels, useTexImage3D),
+                     skipBytes);
+
+    const uint8_t *pixelsWithSkip = pixels + skipBytes;
+    if (useTexImage3D)
+    {
+        for (GLint image = 0; image < area.depth; ++image)
+        {
+            GLint imageByteOffset = image * imageBytes;
+            for (GLint row = 0; row < area.height; ++row)
+            {
+                GLint byteOffset         = imageByteOffset + row * rowBytes;
+                const GLubyte *rowPixels = pixelsWithSkip + byteOffset;
+                mFunctions->texSubImage3D(target, static_cast<GLint>(level), area.x, row + area.y,
+                                          image + area.z, area.width, 1, 1, format, type,
+                                          rowPixels);
+            }
+        }
+    }
+    else if (UseTexImage2D(mState.mTarget))
+    {
+        for (GLint row = 0; row < area.height; ++row)
+        {
+            GLint byteOffset         = row * rowBytes;
+            const GLubyte *rowPixels = pixelsWithSkip + byteOffset;
+            mFunctions->texSubImage2D(target, static_cast<GLint>(level), area.x, row + area.y,
+                                      area.width, 1, format, type, rowPixels);
+        }
+    }
+    else
+    {
+        UNREACHABLE();
+    }
+    return gl::NoError();
+}
+
 gl::Error TextureGL::setCompressedImage(GLenum target, size_t level, GLenum internalFormat, const gl::Extents &size,
                                         const gl::PixelUnpackState &unpack, size_t imageSize, const uint8_t *pixels)
 {

src/libANGLE/renderer/gl/TextureGL.h

@@ -99,6 +99,27 @@ class TextureGL : public TextureImpl
     void setBaseLevel(GLuint) override {}
 
   private:
+    void setImageHelper(GLenum target,
+                        size_t level,
+                        GLenum internalFormat,
+                        const gl::Extents &size,
+                        GLenum format,
+                        GLenum type,
+                        const uint8_t *pixels);
+    void reserveTexImageToBeFilled(GLenum target,
+                                   size_t level,
+                                   GLenum internalFormat,
+                                   const gl::Extents &size,
+                                   GLenum format,
+                                   GLenum type);
+    gl::Error setSubImageRowByRowWorkaround(GLenum target,
+                                            size_t level,
+                                            const gl::Box &area,
+                                            GLenum format,
+                                            GLenum type,
+                                            const gl::PixelUnpackState &unpack,
+                                            const uint8_t *pixels);
+
     const FunctionsGL *mFunctions;
     const WorkaroundsGL &mWorkarounds;
     StateManagerGL *mStateManager;

src/libANGLE/renderer/gl/WorkaroundsGL.h

@@ -20,7 +20,8 @@ struct WorkaroundsGL
           doesSRGBClearsOnLinearFramebufferAttachments(false),
           doWhileGLSLCausesGPUHang(false),
           finishDoesNotCauseQueriesToBeAvailable(false),
-          alwaysCallUseProgramAfterLink(false)
+          alwaysCallUseProgramAfterLink(false),
+          unpackOverlappingRowsSeparatelyUnpackBuffer(false)
     {
     }
 
@@ -63,6 +64,9 @@ struct WorkaroundsGL
     // workaround in Chromium (http://crbug.com/110263). It has been shown that this workaround is
     // not necessary for MacOSX 10.9 and higher (http://crrev.com/39eb535b).
     bool alwaysCallUseProgramAfterLink;
+
+    // In the case of unpacking from a pixel unpack buffer, unpack overlapping rows row by row.
+    bool unpackOverlappingRowsSeparatelyUnpackBuffer;
 };
 }
 

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -697,6 +697,8 @@ void GenerateWorkarounds(const FunctionsGL *functions, WorkaroundsGL *workaround
 
     // TODO(cwallez): Disable this workaround for MacOSX versions 10.9 or later.
     workarounds->alwaysCallUseProgramAfterLink = true;
+
+    workarounds->unpackOverlappingRowsSeparatelyUnpackBuffer = vendor == VENDOR_ID_NVIDIA;
 }
 
 }

src/tests/gl_tests/TextureTest.cpp

@@ -3388,6 +3388,67 @@ TEST_P(Texture2DTestES3, UnpackSkipPixelsOutOfBounds)
     EXPECT_GL_ERROR(GL_INVALID_OPERATION);
 }
 
+// Test that unpacking rows that overlap in a pixel unpack buffer works as expected.
+TEST_P(Texture2DTestES3, UnpackOverlappingRowsFromUnpackBuffer)
+{
+    if (IsD3D11())
+    {
+        std::cout << "Test skipped on D3D." << std::endl;
+        return;
+    }
+    if (IsOSX() && IsAMD())
+    {
+        // Incorrect rendering results seen on OSX AMD.
+        std::cout << "Test skipped on OSX AMD." << std::endl;
+        return;
+    }
+
+    const GLuint width            = 8u;
+    const GLuint height           = 8u;
+    const GLuint unpackRowLength  = 5u;
+    const GLuint unpackSkipPixels = 1u;
+
+    setWindowWidth(width);
+    setWindowHeight(height);
+
+    glBindTexture(GL_TEXTURE_2D, mTexture2D);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    ASSERT_GL_NO_ERROR();
+
+    GLBuffer buf;
+    glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buf.get());
+    std::vector<GLColor> pixelsGreen((height - 1u) * unpackRowLength + width + unpackSkipPixels,
+                                     GLColor::green);
+
+    for (GLuint skippedPixel = 0u; skippedPixel < unpackSkipPixels; ++skippedPixel)
+    {
+        pixelsGreen[skippedPixel] = GLColor(255, 0, 0, 255);
+    }
+
+    glBufferData(GL_PIXEL_UNPACK_BUFFER, pixelsGreen.size() * 4u, pixelsGreen.data(),
+                 GL_DYNAMIC_COPY);
+    ASSERT_GL_NO_ERROR();
+
+    glPixelStorei(GL_UNPACK_ROW_LENGTH, unpackRowLength);
+    glPixelStorei(GL_UNPACK_SKIP_PIXELS, unpackSkipPixels);
+    ASSERT_GL_NO_ERROR();
+
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, 0);
+    ASSERT_GL_NO_ERROR();
+
+    glUseProgram(mProgram);
+    drawQuad(mProgram, "position", 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    GLuint windowPixelCount = getWindowWidth() * getWindowHeight();
+    std::vector<GLColor> actual(windowPixelCount, GLColor::black);
+    glReadPixels(0, 0, getWindowWidth(), getWindowHeight(), GL_RGBA, GL_UNSIGNED_BYTE,
+                 actual.data());
+    std::vector<GLColor> expected(windowPixelCount, GLColor::green);
+    EXPECT_EQ(expected, actual);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 // TODO(oetuaho): Enable all below tests on OpenGL. Requires a fix for ANGLE bug 1278.
 ANGLE_INSTANTIATE_TEST(Texture2DTest,