FramebufferGL: add readPixels workarounds

src/libANGLE/Context.cpp

@@ -2569,6 +2569,11 @@ void Context::readPixels(GLint x,
                          GLenum type,
                          GLvoid *pixels)
 {
+    if (width == 0 || height == 0)
+    {
+        return;
+    }
+
     syncStateForReadPixels();
 
     Framebuffer *framebufferObject = mGLState.getReadFramebuffer();
@@ -2607,6 +2612,11 @@ void Context::copyTexSubImage2D(GLenum target,
                                 GLsizei width,
                                 GLsizei height)
 {
+    if (width == 0 || height == 0)
+    {
+        return;
+    }
+
     // Only sync the read FBO
     mGLState.syncDirtyObject(GL_READ_FRAMEBUFFER);
 
@@ -2629,6 +2639,11 @@ void Context::copyTexSubImage3D(GLenum target,
                                 GLsizei width,
                                 GLsizei height)
 {
+    if (width == 0 || height == 0)
+    {
+        return;
+    }
+
     // Only sync the read FBO
     mGLState.syncDirtyObject(GL_READ_FRAMEBUFFER);
 

src/libANGLE/renderer/gl/FramebufferGL.cpp

@@ -25,10 +25,55 @@
 #include "platform/Platform.h"
 
 using namespace gl;
+using angle::CheckedNumeric;
 
 namespace rx
 {
 
+namespace
+{
+gl::ErrorOrResult<bool> ShouldApplyLastRowPaddingWorkaround(const gl::Rectangle &area,
+                                                            const gl::PixelPackState &pack,
+                                                            GLenum format,
+                                                            GLenum type,
+                                                            const void *pixels)
+{
+    if (pack.pixelBuffer.get() == nullptr)
+    {
+        return false;
+    }
+
+    // We are using an pack buffer, compute what the driver thinks is going to be the last
+    // byte written. If it is past the end of the buffer, we will need to use the workaround
+    // otherwise the driver will generate INVALID_OPERATION.
+    CheckedNumeric<size_t> checkedEndByte;
+    CheckedNumeric<size_t> pixelBytes;
+    size_t rowPitch;
+
+    gl::Extents size(area.width, area.height, 1);
+    const gl::InternalFormat &glFormat =
+        gl::GetInternalFormatInfo(gl::GetSizedInternalFormat(format, type));
+    ANGLE_TRY_RESULT(glFormat.computePackEndByte(type, size, pack), checkedEndByte);
+    ANGLE_TRY_RESULT(glFormat.computeRowPitch(type, area.width, pack.alignment, pack.rowLength),
+                     rowPitch);
+    pixelBytes = glFormat.computePixelBytes(type);
+
+    checkedEndByte += reinterpret_cast<intptr_t>(pixels);
+
+    // At this point checkedEndByte is the actual last byte written.
+    // The driver adds an extra row padding (if any), mimic it.
+    ANGLE_TRY_CHECKED_MATH(pixelBytes);
+    if (pixelBytes.ValueOrDie() * size.width < rowPitch)
+    {
+        checkedEndByte += rowPitch - pixelBytes * size.width;
+    }
+
+    ANGLE_TRY_CHECKED_MATH(checkedEndByte);
+
+    return checkedEndByte.ValueOrDie() > static_cast<size_t>(pack.pixelBuffer->getSize());
+}
+}  // anonymous namespace
+
 FramebufferGL::FramebufferGL(const FramebufferState &state,
                              const FunctionsGL *functions,
                              StateManagerGL *stateManager,
@@ -234,14 +279,35 @@ Error FramebufferGL::readPixels(ContextImpl *context,
     const PixelPackState &packState = context->getGLState().getPackState();
     mStateManager->setPixelPackState(packState);
 
-    mStateManager->bindFramebuffer(GL_READ_FRAMEBUFFER, mFramebufferID);
-
     nativegl::ReadPixelsFormat readPixelsFormat =
         nativegl::GetReadPixelsFormat(mFunctions, mWorkarounds, format, type);
-    mFunctions->readPixels(area.x, area.y, area.width, area.height, readPixelsFormat.format,
-                           readPixelsFormat.type, pixels);
+    GLenum readFormat = readPixelsFormat.format;
+    GLenum readType   = readPixelsFormat.type;
 
-    return Error(GL_NO_ERROR);
+    mStateManager->bindFramebuffer(GL_READ_FRAMEBUFFER, mFramebufferID);
+
+    if (mWorkarounds.packOverlappingRowsSeparatelyPackBuffer && packState.pixelBuffer.get() &&
+        packState.rowLength != 0 && packState.rowLength < area.width)
+    {
+        return readPixelsRowByRowWorkaround(area, readFormat, readType, packState, pixels);
+    }
+
+    if (mWorkarounds.packLastRowSeparatelyForPaddingInclusion)
+    {
+        bool apply;
+        ANGLE_TRY_RESULT(
+            ShouldApplyLastRowPaddingWorkaround(area, packState, readFormat, readType, pixels),
+            apply);
+
+        if (apply)
+        {
+            return readPixelsPaddingWorkaround(area, readFormat, readType, packState, pixels);
+        }
+    }
+
+    mFunctions->readPixels(area.x, area.y, area.width, area.height, readFormat, readType, pixels);
+
+    return gl::NoError();
 }
 
 Error FramebufferGL::blit(ContextImpl *context,
@@ -396,4 +462,75 @@ void FramebufferGL::syncClearBufferState(GLenum buffer, GLint drawBuffer)
         }
     }
 }
+gl::Error FramebufferGL::readPixelsRowByRowWorkaround(const gl::Rectangle &area,
+                                                      GLenum format,
+                                                      GLenum type,
+                                                      const gl::PixelPackState &pack,
+                                                      GLvoid *pixels) const
+{
+    intptr_t offset = reinterpret_cast<intptr_t>(pixels);
+
+    const gl::InternalFormat &glFormat =
+        gl::GetInternalFormatInfo(gl::GetSizedInternalFormat(format, type));
+    GLuint rowBytes = 0;
+    ANGLE_TRY_RESULT(glFormat.computeRowPitch(type, area.width, pack.alignment, pack.rowLength),
+                     rowBytes);
+    GLuint skipBytes = 0;
+    ANGLE_TRY_RESULT(
+        glFormat.computeSkipBytes(rowBytes, 0, 0, pack.skipRows, pack.skipPixels, false),
+        skipBytes);
+
+    gl::PixelPackState directPack;
+    directPack.pixelBuffer = pack.pixelBuffer;
+    directPack.alignment   = 1;
+    mStateManager->setPixelPackState(directPack);
+    directPack.pixelBuffer.set(nullptr);
+
+    offset += skipBytes;
+    for (GLint row = 0; row < area.height; ++row)
+    {
+        mFunctions->readPixels(area.x, row + area.y, area.width, 1, format, type,
+                               reinterpret_cast<GLvoid *>(offset));
+        offset += row * rowBytes;
+    }
+
+    return gl::NoError();
+}
+
+gl::Error FramebufferGL::readPixelsPaddingWorkaround(const gl::Rectangle &area,
+                                                     GLenum format,
+                                                     GLenum type,
+                                                     const gl::PixelPackState &pack,
+                                                     GLvoid *pixels) const
+{
+    const gl::InternalFormat &glFormat =
+        gl::GetInternalFormatInfo(gl::GetSizedInternalFormat(format, type));
+    GLuint rowBytes = 0;
+    ANGLE_TRY_RESULT(glFormat.computeRowPitch(type, area.width, pack.alignment, pack.rowLength),
+                     rowBytes);
+    GLuint skipBytes = 0;
+    ANGLE_TRY_RESULT(
+        glFormat.computeSkipBytes(rowBytes, 0, 0, pack.skipRows, pack.skipPixels, false),
+        skipBytes);
+
+    // Get all by the last row
+    if (area.height > 1)
+    {
+        mFunctions->readPixels(area.x, area.y, area.width, area.height - 1, format, type, pixels);
+    }
+
+    // Get the last row manually
+    gl::PixelPackState directPack;
+    directPack.pixelBuffer = pack.pixelBuffer;
+    directPack.alignment   = 1;
+    mStateManager->setPixelPackState(directPack);
+    directPack.pixelBuffer.set(nullptr);
+
+    intptr_t lastRowOffset =
+        reinterpret_cast<intptr_t>(pixels) + skipBytes + (area.height - 1) * rowBytes;
+    mFunctions->readPixels(area.x, area.y + area.height - 1, area.width, 1, format, type,
+                           reinterpret_cast<GLvoid *>(lastRowOffset));
+
+    return gl::NoError();
+}
 }  // namespace rx

src/libANGLE/renderer/gl/FramebufferGL.h

@@ -85,6 +85,18 @@ class FramebufferGL : public FramebufferImpl
     void syncClearState(GLbitfield mask);
     void syncClearBufferState(GLenum buffer, GLint drawBuffer);
 
+    gl::Error readPixelsRowByRowWorkaround(const gl::Rectangle &area,
+                                           GLenum format,
+                                           GLenum type,
+                                           const gl::PixelPackState &pack,
+                                           GLvoid *pixels) const;
+
+    gl::Error readPixelsPaddingWorkaround(const gl::Rectangle &area,
+                                          GLenum format,
+                                          GLenum type,
+                                          const gl::PixelPackState &pack,
+                                          GLvoid *pixels) const;
+
     const FunctionsGL *mFunctions;
     StateManagerGL *mStateManager;
     const WorkaroundsGL &mWorkarounds;

src/libANGLE/renderer/gl/TextureGL.cpp

@@ -120,7 +120,7 @@ gl::ErrorOrResult<bool> ShouldApplyLastRowPaddingWorkaround(const gl::Box &area,
 
     checkedEndByte += reinterpret_cast<intptr_t>(pixels);
 
-    // At the point checkedEndByte is the actual last byte read.
+    // At this point checkedEndByte is the actual last byte read.
     // The driver adds an extra row padding (if any), mimic it.
     ANGLE_TRY_CHECKED_MATH(pixelBytes);
     if (pixelBytes.ValueOrDie() * size.width < rowPitch)
@@ -194,6 +194,12 @@ gl::Error TextureGL::setImage(GLenum target, size_t level, GLenum internalFormat
         // The rows overlap in unpack memory. Upload the texture row by row to work around
         // driver bug.
         reserveTexImageToBeFilled(target, level, internalFormat, size, format, type);
+
+        if (size.width == 0 || size.height == 0 || size.depth == 0)
+        {
+            return gl::NoError();
+        }
+
         gl::Box area(0, 0, 0, size.width, size.height, size.depth);
         return setSubImageRowByRowWorkaround(target, level, area, format, type, unpack, pixels);
     }
@@ -211,8 +217,13 @@ gl::Error TextureGL::setImage(GLenum target, size_t level, GLenum internalFormat
         if (apply)
         {
             reserveTexImageToBeFilled(target, level, internalFormat, size, format, type);
-            return setSubImageAlignmentWorkaround(target, level, area, format, type, unpack,
-                                                  pixels);
+
+            if (size.width == 0 || size.height == 0 || size.depth == 0)
+            {
+                return gl::NoError();
+            }
+
+            return setSubImagePaddingWorkaround(target, level, area, format, type, unpack, pixels);
         }
     }
 
@@ -301,8 +312,7 @@ gl::Error TextureGL::setSubImage(GLenum target, size_t level, const gl::Box &are
         // by uploading the last row (and last level if 3D) separately.
         if (apply)
         {
-            return setSubImageAlignmentWorkaround(target, level, area, format, type, unpack,
-                                                  pixels);
+            return setSubImagePaddingWorkaround(target, level, area, format, type, unpack, pixels);
         }
     }
 
@@ -383,13 +393,13 @@ gl::Error TextureGL::setSubImageRowByRowWorkaround(GLenum target,
     return gl::NoError();
 }
 
-gl::Error TextureGL::setSubImageAlignmentWorkaround(GLenum target,
-                                                    size_t level,
-                                                    const gl::Box &area,
-                                                    GLenum format,
-                                                    GLenum type,
-                                                    const gl::PixelUnpackState &unpack,
-                                                    const uint8_t *pixels)
+gl::Error TextureGL::setSubImagePaddingWorkaround(GLenum target,
+                                                  size_t level,
+                                                  const gl::Box &area,
+                                                  GLenum format,
+                                                  GLenum type,
+                                                  const gl::PixelUnpackState &unpack,
+                                                  const uint8_t *pixels)
 {
     const gl::InternalFormat &glFormat =
         gl::GetInternalFormatInfo(gl::GetSizedInternalFormat(format, type));

src/libANGLE/renderer/gl/TextureGL.h

@@ -120,13 +120,13 @@ class TextureGL : public TextureImpl
                                             const gl::PixelUnpackState &unpack,
                                             const uint8_t *pixels);
 
-    gl::Error setSubImageAlignmentWorkaround(GLenum target,
-                                             size_t level,
-                                             const gl::Box &area,
-                                             GLenum format,
-                                             GLenum type,
-                                             const gl::PixelUnpackState &unpack,
-                                             const uint8_t *pixels);
+    gl::Error setSubImagePaddingWorkaround(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;

src/libANGLE/renderer/gl/WorkaroundsGL.h

@@ -69,6 +69,8 @@ struct WorkaroundsGL
 
     // In the case of unpacking from a pixel unpack buffer, unpack overlapping rows row by row.
     bool unpackOverlappingRowsSeparatelyUnpackBuffer;
+    // In the case of packing to a pixel pack buffer, pack overlapping rows row by row.
+    bool packOverlappingRowsSeparatelyPackBuffer;
 
     // During initialization, assign the current vertex attributes to the spec-mandated defaults.
     bool initializeCurrentVertexAttributes;
@@ -94,6 +96,8 @@ struct WorkaroundsGL
     // The last pixel read will be A, but the driver will think it is B, causing it to generate an
     // error when the pixel buffer is just big enough.
     bool unpackLastRowSeparatelyForPaddingInclusion;
+    // Equivalent workaround when uploading data from a pixel pack buffer.
+    bool packLastRowSeparatelyForPaddingInclusion;
 };
 }
 

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -900,13 +900,16 @@ void GenerateWorkarounds(const FunctionsGL *functions, WorkaroundsGL *workaround
     workarounds->alwaysCallUseProgramAfterLink = true;
 
     workarounds->unpackOverlappingRowsSeparatelyUnpackBuffer = vendor == VENDOR_ID_NVIDIA;
+    workarounds->packOverlappingRowsSeparatelyPackBuffer     = vendor == VENDOR_ID_NVIDIA;
 
     workarounds->initializeCurrentVertexAttributes = vendor == VENDOR_ID_NVIDIA;
 
 #if defined(ANGLE_PLATFORM_APPLE)
     workarounds->unpackLastRowSeparatelyForPaddingInclusion = true;
+    workarounds->packLastRowSeparatelyForPaddingInclusion   = true;
 #else
     workarounds->unpackLastRowSeparatelyForPaddingInclusion = vendor == VENDOR_ID_NVIDIA;
+    workarounds->packLastRowSeparatelyForPaddingInclusion   = vendor == VENDOR_ID_NVIDIA;
 #endif
 }