Do not apply UNPACK_SKIP_IMAGES for 2D textures on D3D

src/libANGLE/ImageIndex.cpp

@@ -27,6 +27,11 @@ ImageIndex &ImageIndex::operator=(const ImageIndex &other)
     return *this;
 }
 
+bool ImageIndex::is3D() const
+{
+    return type == GL_TEXTURE_3D || type == GL_TEXTURE_2D_ARRAY;
+}
+
 ImageIndex ImageIndex::Make2D(GLint mipIndex)
 {
     return ImageIndex(GL_TEXTURE_2D, mipIndex, ENTIRE_LEVEL);

src/libANGLE/ImageIndex.h

@@ -28,6 +28,7 @@ struct ImageIndex
     ImageIndex &operator=(const ImageIndex &other);
 
     bool hasLayer() const { return layerIndex != ENTIRE_LEVEL; }
+    bool is3D() const;
 
     static ImageIndex Make2D(GLint mipIndex);
     static ImageIndex MakeCube(GLenum target, GLint mipIndex);

src/libANGLE/formatutils.cpp

@@ -767,9 +767,11 @@ GLuint InternalFormat::computeSkipPixels(GLint rowPitch,
                                          GLint depthPitch,
                                          GLint skipImages,
                                          GLint skipRows,
-                                         GLint skipPixels) const
+                                         GLint skipPixels,
+                                         bool applySkipImages) const
 {
-    return skipImages * depthPitch + skipRows * rowPitch + skipPixels * pixelBytes;
+    GLuint skipImagesBytes = applySkipImages ? skipImages * depthPitch : 0;
+    return skipImagesBytes + skipRows * rowPitch + skipPixels * pixelBytes;
 }
 
 gl::ErrorOrResult<GLuint> InternalFormat::computeUnpackSize(

src/libANGLE/formatutils.h

@@ -63,7 +63,8 @@ struct InternalFormat
                              GLint depthPitch,
                              GLint skipImages,
                              GLint skipRows,
-                             GLint skipPixels) const;
+                             GLint skipPixels,
+                             bool applySkipImages) const;
     gl::ErrorOrResult<GLuint> computeUnpackSize(GLenum formatType,
                                                 const gl::Extents &size,
                                                 const gl::PixelUnpackState &unpack) const;

src/libANGLE/renderer/d3d/FramebufferD3D.cpp

@@ -247,7 +247,7 @@ gl::Error FramebufferD3D::readPixels(ContextImpl *context,
         sizedFormatInfo.computeRowPitch(type, area.width, packState.alignment, packState.rowLength),
         outputPitch);
     GLsizei outputSkipBytes = sizedFormatInfo.computeSkipPixels(
-        outputPitch, 0, 0, packState.skipRows, packState.skipPixels);
+        outputPitch, 0, 0, packState.skipRows, packState.skipPixels, false);
 
     return readPixelsImpl(area, format, type, outputPitch, packState,
                           reinterpret_cast<uint8_t *>(pixels) + outputSkipBytes);

src/libANGLE/renderer/d3d/ImageD3D.h

@@ -51,7 +51,11 @@ class ImageD3D : angle::NonCopyable
 
     virtual bool redefine(GLenum target, GLenum internalformat, const gl::Extents &size, bool forceRelease) = 0;
 
-    virtual gl::Error loadData(const gl::Box &area, const gl::PixelUnpackState &unpack, GLenum type, const void *input) = 0;
+    virtual gl::Error loadData(const gl::Box &area,
+                               const gl::PixelUnpackState &unpack,
+                               GLenum type,
+                               const void *input,
+                               bool applySkipImages) = 0;
     virtual gl::Error loadCompressedData(const gl::Box &area, const void *input) = 0;
 
     virtual gl::Error setManagedSurface2D(TextureStorage *storage, int level) { return gl::Error(GL_NO_ERROR); };

src/libANGLE/renderer/d3d/TextureD3D.cpp

@@ -210,7 +210,7 @@ gl::Error TextureD3D::setImageImpl(const gl::ImageIndex &index,
         else
         {
             gl::Box fullImageArea(0, 0, 0, image->getWidth(), image->getHeight(), image->getDepth());
-            error = image->loadData(fullImageArea, unpack, type, pixelData);
+            error = image->loadData(fullImageArea, unpack, type, pixelData, index.is3D());
         }
 
         if (error.isError())
@@ -245,7 +245,7 @@ gl::Error TextureD3D::subImage(const gl::ImageIndex &index, const gl::Box &area,
             return mTexStorage->setData(index, image, &area, type, unpack, pixelData);
         }
 
-        error = image->loadData(area, unpack, type, pixelData);
+        error = image->loadData(area, unpack, type, pixelData, index.is3D());
         if (error.isError())
         {
             return error;

src/libANGLE/renderer/d3d/d3d11/Image11.cpp

@@ -245,7 +245,11 @@ DXGI_FORMAT Image11::getDXGIFormat() const
 
 // Store the pixel rectangle designated by xoffset,yoffset,width,height with pixels stored as format/type at input
 // into the target pixel rectangle.
-gl::Error Image11::loadData(const gl::Box &area, const gl::PixelUnpackState &unpack, GLenum type, const void *input)
+gl::Error Image11::loadData(const gl::Box &area,
+                            const gl::PixelUnpackState &unpack,
+                            GLenum type,
+                            const void *input,
+                            bool applySkipImages)
 {
     const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(mInternalFormat);
     GLsizei inputRowPitch                = 0;
@@ -256,8 +260,9 @@ gl::Error Image11::loadData(const gl::Box &area, const gl::PixelUnpackState &unp
     ANGLE_TRY_RESULT(formatInfo.computeDepthPitch(type, area.width, area.height, unpack.alignment,
                                                   unpack.rowLength, unpack.imageHeight),
                      inputDepthPitch);
-    GLsizei inputSkipBytes = formatInfo.computeSkipPixels(
-        inputRowPitch, inputDepthPitch, unpack.skipImages, unpack.skipRows, unpack.skipPixels);
+    GLsizei inputSkipBytes =
+        formatInfo.computeSkipPixels(inputRowPitch, inputDepthPitch, unpack.skipImages,
+                                     unpack.skipRows, unpack.skipPixels, applySkipImages);
 
     const d3d11::DXGIFormatSize &dxgiFormatInfo = d3d11::GetDXGIFormatSizeInfo(mDXGIFormat);
     GLuint outputPixelSize = dxgiFormatInfo.pixelBytes;

src/libANGLE/renderer/d3d/d3d11/Image11.h

@@ -45,8 +45,12 @@ class Image11 : public ImageD3D
 
     DXGI_FORMAT getDXGIFormat() const;
 
-    virtual gl::Error loadData(const gl::Box &area, const gl::PixelUnpackState &unpack, GLenum type, const void *input);
-    virtual gl::Error loadCompressedData(const gl::Box &area, const void *input);
+    gl::Error loadData(const gl::Box &area,
+                       const gl::PixelUnpackState &unpack,
+                       GLenum type,
+                       const void *input,
+                       bool applySkipImages) override;
+    gl::Error loadCompressedData(const gl::Box &area, const void *input) override;
 
     gl::Error copyFromTexStorage(const gl::ImageIndex &imageIndex, TextureStorage *source) override;
     gl::Error copyFromFramebuffer(const gl::Offset &destOffset,

src/libANGLE/renderer/d3d/d3d11/TextureStorage11.cpp

@@ -655,8 +655,9 @@ gl::Error TextureStorage11::setData(const gl::ImageIndex &index,
     ANGLE_TRY_RESULT(internalFormatInfo.computeDepthPitch(type, width, height, unpack.alignment,
                                                           unpack.rowLength, unpack.imageHeight),
                      srcDepthPitch);
-    const GLsizei srcSkipBytes = internalFormatInfo.computeSkipPixels(
-        srcRowPitch, srcDepthPitch, unpack.skipImages, unpack.skipRows, unpack.skipPixels);
+    const GLsizei srcSkipBytes =
+        internalFormatInfo.computeSkipPixels(srcRowPitch, srcDepthPitch, unpack.skipImages,
+                                             unpack.skipRows, unpack.skipPixels, index.is3D());
 
     const d3d11::TextureFormat &d3d11Format = d3d11::GetTextureFormatInfo(
         image->getInternalFormat(), mRenderer->getRenderer11DeviceCaps());

src/libANGLE/renderer/d3d/d3d9/Image9.cpp

@@ -473,7 +473,11 @@ gl::Error Image9::copyToSurface(IDirect3DSurface9 *destSurface, const gl::Box &a
 
 // Store the pixel rectangle designated by xoffset,yoffset,width,height with pixels stored as format/type at input
 // into the target pixel rectangle.
-gl::Error Image9::loadData(const gl::Box &area, const gl::PixelUnpackState &unpack, GLenum type, const void *input)
+gl::Error Image9::loadData(const gl::Box &area,
+                           const gl::PixelUnpackState &unpack,
+                           GLenum type,
+                           const void *input,
+                           bool applySkipImages)
 {
     // 3D textures are not supported by the D3D9 backend.
     ASSERT(area.z == 0 && area.depth == 1);
@@ -483,8 +487,9 @@ gl::Error Image9::loadData(const gl::Box &area, const gl::PixelUnpackState &unpa
     ANGLE_TRY_RESULT(
         formatInfo.computeRowPitch(type, area.width, unpack.alignment, unpack.rowLength),
         inputRowPitch);
-    GLsizei inputSkipBytes = formatInfo.computeSkipPixels(inputRowPitch, 0, unpack.skipImages,
-                                                          unpack.skipRows, unpack.skipPixels);
+    ASSERT(!applySkipImages);
+    GLsizei inputSkipBytes = formatInfo.computeSkipPixels(
+        inputRowPitch, 0, unpack.skipImages, unpack.skipRows, unpack.skipPixels, false);
 
     const d3d9::TextureFormat &d3dFormatInfo = d3d9::GetTextureFormatInfo(mInternalFormat);
     ASSERT(d3dFormatInfo.loadFunction != NULL);

src/libANGLE/renderer/d3d/d3d9/Image9.h

@@ -42,8 +42,12 @@ class Image9 : public ImageD3D
     virtual gl::Error setManagedSurfaceCube(TextureStorage *storage, int face, int level);
     virtual gl::Error copyToStorage(TextureStorage *storage, const gl::ImageIndex &index, const gl::Box &region);
 
-    virtual gl::Error loadData(const gl::Box &area, const gl::PixelUnpackState &unpack, GLenum type, const void *input);
-    virtual gl::Error loadCompressedData(const gl::Box &area, const void *input);
+    gl::Error loadData(const gl::Box &area,
+                       const gl::PixelUnpackState &unpack,
+                       GLenum type,
+                       const void *input,
+                       bool applySkipImages) override;
+    gl::Error loadCompressedData(const gl::Box &area, const void *input) override;
 
     gl::Error copyFromTexStorage(const gl::ImageIndex &imageIndex, TextureStorage *source) override;
     gl::Error copyFromFramebuffer(const gl::Offset &destOffset,

src/tests/gl_tests/TextureTest.cpp

@@ -3318,6 +3318,42 @@ TEST_P(Texture2DNorm16TestES3, TextureNorm16Test)
     testNorm16Render(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT);
 }
 
+// Test that UNPACK_SKIP_IMAGES doesn't have an effect on 2D texture uploads.
+// GLES 3.0.4 section 3.8.3.
+TEST_P(Texture2DTestES3, UnpackSkipImages2D)
+{
+    if (IsIntel() && isOpenGL())
+    {
+        std::cout << "Test skipped on Intel OpenGL." << std::endl;
+        return;
+    }
+
+    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();
+
+    // SKIP_IMAGES should not have an effect on uploading 2D textures
+    glPixelStorei(GL_UNPACK_SKIP_IMAGES, 1000);
+    ASSERT_GL_NO_ERROR();
+
+    std::vector<GLColor> pixelsGreen(128u * 128u, GLColor::green);
+
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 128, 128, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 pixelsGreen.data());
+    ASSERT_GL_NO_ERROR();
+
+    glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 128, 128, GL_RGBA, GL_UNSIGNED_BYTE,
+                    pixelsGreen.data());
+    ASSERT_GL_NO_ERROR();
+
+    glUseProgram(mProgram);
+    drawQuad(mProgram, "position", 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 // 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,