Make PackPixels take an angle::Format.

scripts/run_code_generation_hashes.json

@@ -6,7 +6,7 @@
   "ANGLE format:src/libANGLE/renderer/angle_format_map.json":
     "ea6dfe3ebbc86e04f0d4b9f568ba22ae",
   "ANGLE format:src/libANGLE/renderer/gen_angle_format_table.py":
-    "68e7a56c323a44170656478f2bcc9ff8",
+    "4b311db89dc5d3ed17c6e0f0e99260aa",
   "ANGLE load functions table:src/libANGLE/renderer/gen_load_functions_table.py":
     "8afc7eecce2a3ba9f0b4beacb1aa7fe2",
   "ANGLE load functions table:src/libANGLE/renderer/load_functions_data.json":

src/libANGLE/renderer/Format.h

@@ -42,6 +42,8 @@ struct Format final : private angle::NonCopyable
 
     constexpr bool hasDepthOrStencilBits() const;
 
+    bool operator==(const Format &other) const { return this->id == other.id; }
+
     FormatID id;
 
     // The closest matching GL internal format for the storage this format uses. Note that this

src/libANGLE/renderer/Format_table_autogen.cpp

@@ -17,7 +17,7 @@
 namespace angle
 {
 
-static constexpr rx::FastCopyFunctionMap::Entry BGRAEntry  = {GL_RGBA, GL_UNSIGNED_BYTE,
+static constexpr rx::FastCopyFunctionMap::Entry BGRAEntry  = {angle::FormatID::R8G8B8A8_UNORM,
                                                              CopyBGRA8ToRGBA8};
 static constexpr rx::FastCopyFunctionMap BGRACopyFunctions = {&BGRAEntry, 1};
 static constexpr rx::FastCopyFunctionMap NoCopyFunctions;

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

@@ -139,30 +139,31 @@ gl::Error Framebuffer11::invalidateBase(const gl::Context *context,
         return gl::NoError();
     }
 
-    bool foundDepth = false;
+    bool foundDepth   = false;
     bool foundStencil = false;
 
     for (size_t i = 0; i < count; ++i)
     {
         switch (attachments[i])
         {
-          // Handle depth and stencil attachments. Defer discarding until later.
-          case GL_DEPTH_STENCIL_ATTACHMENT:
-            foundDepth = true;
-            foundStencil = true;
-            break;
-          case GL_DEPTH_EXT:
-          case GL_DEPTH_ATTACHMENT:
-            foundDepth = true;
-            break;
-          case GL_STENCIL_EXT:
-          case GL_STENCIL_ATTACHMENT:
-            foundStencil = true;
-            break;
-          default:
+            // Handle depth and stencil attachments. Defer discarding until later.
+            case GL_DEPTH_STENCIL_ATTACHMENT:
+                foundDepth   = true;
+                foundStencil = true;
+                break;
+            case GL_DEPTH_EXT:
+            case GL_DEPTH_ATTACHMENT:
+                foundDepth = true;
+                break;
+            case GL_STENCIL_EXT:
+            case GL_STENCIL_ATTACHMENT:
+                foundStencil = true;
+                break;
+            default:
             {
                 // Handle color attachments
-                ASSERT((attachments[i] >= GL_COLOR_ATTACHMENT0 && attachments[i] <= GL_COLOR_ATTACHMENT15) ||
+                ASSERT((attachments[i] >= GL_COLOR_ATTACHMENT0 &&
+                        attachments[i] <= GL_COLOR_ATTACHMENT15) ||
                        (attachments[i] == GL_COLOR));
 
                 size_t colorIndex =
@@ -178,14 +179,15 @@ gl::Error Framebuffer11::invalidateBase(const gl::Context *context,
         }
     }
 
-    bool discardDepth = false;
+    bool discardDepth   = false;
     bool discardStencil = false;
 
     // The D3D11 renderer uses the same view for depth and stencil buffers, so we must be careful.
     if (useEXTBehavior)
     {
         // In the extension, if the app discards only one of the depth and stencil attachments, but
-        // those are backed by the same packed_depth_stencil buffer, then both images become undefined.
+        // those are backed by the same packed_depth_stencil buffer, then both images become
+        // undefined.
         discardDepth = foundDepth;
 
         // Don't bother discarding the stencil buffer if the depth buffer will already do it
@@ -195,8 +197,9 @@ gl::Error Framebuffer11::invalidateBase(const gl::Context *context,
     {
         // In ES 3.0.4, if a specified attachment has base internal format DEPTH_STENCIL but the
         // attachments list does not include DEPTH_STENCIL_ATTACHMENT or both DEPTH_ATTACHMENT and
-        // STENCIL_ATTACHMENT, then only the specified portion of every pixel in the subregion of pixels
-        // of the DEPTH_STENCIL buffer may be invalidated, and the other portion must be preserved.
+        // STENCIL_ATTACHMENT, then only the specified portion of every pixel in the subregion of
+        // pixels of the DEPTH_STENCIL buffer may be invalidated, and the other portion must be
+        // preserved.
         discardDepth = (foundDepth && foundStencil) ||
                        (foundDepth && (mState.getStencilAttachment() == nullptr));
         discardStencil = (foundStencil && (mState.getDepthAttachment() == nullptr));
@@ -257,8 +260,9 @@ gl::Error Framebuffer11::readPixelsImpl(const gl::Context *context,
     gl::Buffer *packBuffer = context->getGLState().getTargetBuffer(gl::BufferBinding::PixelPack);
     if (packBuffer != nullptr)
     {
-        Buffer11 *packBufferStorage = GetImplAs<Buffer11>(packBuffer);
-        PackPixelsParams packParams(area, format, type, static_cast<GLuint>(outputPitch), pack,
+        Buffer11 *packBufferStorage      = GetImplAs<Buffer11>(packBuffer);
+        const angle::Format &angleFormat = GetFormatFromFormatType(format, type);
+        PackPixelsParams packParams(area, angleFormat, static_cast<GLuint>(outputPitch), pack,
                                     packBuffer, reinterpret_cast<ptrdiff_t>(pixels));
 
         return packBufferStorage->packPixels(context, *readAttachment, packParams);
@@ -290,12 +294,12 @@ gl::Error Framebuffer11::blitImpl(const gl::Context *context,
         const auto &colorAttachments = mState.getColorAttachments();
         const auto &drawBufferStates = mState.getDrawBufferStates();
 
-        for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size(); colorAttachment++)
+        for (size_t colorAttachment = 0; colorAttachment < colorAttachments.size();
+             colorAttachment++)
         {
             const gl::FramebufferAttachment &drawBuffer = colorAttachments[colorAttachment];
 
-            if (drawBuffer.isAttached() &&
-                drawBufferStates[colorAttachment] != GL_NONE)
+            if (drawBuffer.isAttached() && drawBufferStates[colorAttachment] != GL_NONE)
             {
                 RenderTargetD3D *drawRenderTarget = nullptr;
                 ANGLE_TRY(drawBuffer.getRenderTarget(context, &drawRenderTarget));
@@ -306,8 +310,8 @@ gl::Error Framebuffer11::blitImpl(const gl::Context *context,
                 if (invertColorSource)
                 {
                     RenderTarget11 *readRenderTarget11 = GetAs<RenderTarget11>(readRenderTarget);
-                    actualSourceArea.y                 = readRenderTarget11->getHeight() - sourceArea.y;
-                    actualSourceArea.height            = -sourceArea.height;
+                    actualSourceArea.y      = readRenderTarget11->getHeight() - sourceArea.y;
+                    actualSourceArea.height = -sourceArea.height;
                 }
 
                 const bool invertColorDest   = UsePresentPathFast(mRenderer, &drawBuffer);
@@ -315,8 +319,8 @@ gl::Error Framebuffer11::blitImpl(const gl::Context *context,
                 if (invertColorDest)
                 {
                     RenderTarget11 *drawRenderTarget11 = GetAs<RenderTarget11>(drawRenderTarget);
-                    actualDestArea.y                   = drawRenderTarget11->getHeight() - destArea.y;
-                    actualDestArea.height              = -destArea.height;
+                    actualDestArea.y      = drawRenderTarget11->getHeight() - destArea.y;
+                    actualDestArea.height = -destArea.height;
                 }
 
                 ANGLE_TRY(mRenderer->blitRenderbufferRect(

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

@@ -3119,10 +3119,11 @@ gl::Error Renderer11::readFromAttachment(const gl::Context *context,
     mDeviceContext->CopySubresourceRegion(stagingHelper.get(), 0, 0, 0, 0, srcTexture->get(),
                                           sourceSubResource, &srcBox);
 
+    const angle::Format &angleFormat = GetFormatFromFormatType(format, type);
     gl::Buffer *packBuffer = context->getGLState().getTargetBuffer(gl::BufferBinding::PixelPack);
     if (!invertTexture)
     {
-        PackPixelsParams packParams(safeArea, format, type, outputPitch, pack, packBuffer, 0);
+        PackPixelsParams packParams(safeArea, angleFormat, outputPitch, pack, packBuffer, 0);
         return packPixels(stagingHelper, packParams, pixelsOut);
     }
 
@@ -3134,7 +3135,7 @@ gl::Error Renderer11::readFromAttachment(const gl::Context *context,
     invertTexturePack.alignment       = pack.alignment;
     invertTexturePack.reverseRowOrder = !pack.reverseRowOrder;
 
-    PackPixelsParams packParams(safeArea, format, type, outputPitch, invertTexturePack, packBuffer,
+    PackPixelsParams packParams(safeArea, angleFormat, outputPitch, invertTexturePack, packBuffer,
                                 0);
     gl::Error error = packPixels(stagingHelper, packParams, pixelsOut);
     ANGLE_TRY(error);

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

@@ -188,6 +188,7 @@ gl::Error Framebuffer9::readPixelsImpl(const gl::Context *context,
     int inputPitch  = lock.Pitch;
 
     const d3d9::D3DFormat &d3dFormatInfo = d3d9::GetD3DFormatInfo(desc.Format);
+
     gl::FormatType formatType(format, type);
 
     PackPixelsParams packParams;
@@ -195,8 +196,7 @@ gl::Error Framebuffer9::readPixelsImpl(const gl::Context *context,
     packParams.area.y      = rect.top;
     packParams.area.width  = rect.right - rect.left;
     packParams.area.height = rect.bottom - rect.top;
-    packParams.format      = format;
-    packParams.type        = type;
+    packParams.destFormat  = &GetFormatFromFormatType(format, type);
     packParams.outputPitch = static_cast<GLuint>(outputPitch);
     packParams.pack        = pack;
 

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

@@ -1004,7 +1004,7 @@ gl::Error Renderer9::setTexture(const gl::Context *context,
                                 gl::Texture *texture)
 {
     int d3dSamplerOffset = (type == gl::ShaderType::Fragment) ? 0 : D3DVERTEXTEXTURESAMPLER0;
-    int d3dSampler       = index + d3dSamplerOffset;
+    int d3dSampler                    = index + d3dSamplerOffset;
     IDirect3DBaseTexture9 *d3dTexture = nullptr;
     bool forceSetTexture              = false;
 

src/libANGLE/renderer/gen_angle_format_table.py

@@ -56,7 +56,7 @@ template_autogen_inl = """// GENERATED FILE - DO NOT EDIT.
 namespace angle
 {{
 
-static constexpr rx::FastCopyFunctionMap::Entry BGRAEntry = {{GL_RGBA, GL_UNSIGNED_BYTE,
+static constexpr rx::FastCopyFunctionMap::Entry BGRAEntry = {{angle::FormatID::R8G8B8A8_UNORM,
                                                              CopyBGRA8ToRGBA8}};
 static constexpr rx::FastCopyFunctionMap BGRACopyFunctions = {{&BGRAEntry, 1}};
 static constexpr rx::FastCopyFunctionMap NoCopyFunctions;

src/libANGLE/renderer/renderer_utils.h

@@ -21,6 +21,7 @@
 namespace angle
 {
 struct Format;
+enum class FormatID;
 }  // namespace angle
 
 namespace gl
@@ -130,8 +131,7 @@ class FastCopyFunctionMap
   public:
     struct Entry
     {
-        GLenum format;
-        GLenum type;
+        angle::FormatID formatID;
         ColorCopyFunction func;
     };
 
@@ -139,8 +139,8 @@ class FastCopyFunctionMap
 
     constexpr FastCopyFunctionMap(const Entry *data, size_t size) : mSize(size), mData(data) {}
 
-    bool has(const gl::FormatType &formatType) const;
-    ColorCopyFunction get(const gl::FormatType &formatType) const;
+    bool has(angle::FormatID formatID) const;
+    ColorCopyFunction get(angle::FormatID formatID) const;
 
   private:
     size_t mSize;
@@ -151,16 +151,14 @@ struct PackPixelsParams
 {
     PackPixelsParams();
     PackPixelsParams(const gl::Rectangle &area,
-                     GLenum format,
-                     GLenum type,
+                     const angle::Format &destFormat,
                      GLuint outputPitch,
                      const gl::PixelPackState &pack,
                      gl::Buffer *packBufferIn,
                      ptrdiff_t offset);
 
     gl::Rectangle area;
-    GLenum format;
-    GLenum type;
+    const angle::Format *destFormat;
     GLuint outputPitch;
     gl::Buffer *packBuffer;
     gl::PixelPackState pack;
@@ -173,10 +171,6 @@ void PackPixels(const PackPixelsParams &params,
                 const uint8_t *source,
                 uint8_t *destination);
 
-ColorWriteFunction GetColorWriteFunction(const gl::FormatType &formatType);
-ColorCopyFunction GetFastCopyFunction(const FastCopyFunctionMap &fastCopyFunctions,
-                                      const gl::FormatType &formatType);
-
 using InitializeTextureDataFunction = void (*)(size_t width,
                                                size_t height,
                                                size_t depth,
@@ -274,6 +268,7 @@ void GetMatrixUniform(GLenum type, GLfloat *dataOut, const GLfloat *source, bool
 template <typename NonFloatT>
 void GetMatrixUniform(GLenum type, NonFloatT *dataOut, const NonFloatT *source, bool transpose);
 
+const angle::Format &GetFormatFromFormatType(GLenum format, GLenum type);
 }  // namespace rx
 
 #endif  // LIBANGLE_RENDERER_RENDERER_UTILS_H_

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -344,13 +344,10 @@ gl::Error FramebufferVk::readPixels(const gl::Context *context,
     outputSkipBytes += (clippedArea.x - area.x) * sizedFormatInfo.pixelBytes +
                        (clippedArea.y - area.y) * outputPitch;
 
-    PackPixelsParams params;
-    params.area        = flippedArea;
-    params.format      = format;
-    params.type        = type;
-    params.outputPitch = outputPitch;
-    params.packBuffer  = glState.getTargetBuffer(gl::BufferBinding::PixelPack);
-    params.pack        = packState;
+    const angle::Format &angleFormat = GetFormatFromFormatType(format, type);
+
+    PackPixelsParams params(flippedArea, angleFormat, outputPitch, packState,
+                            glState.getTargetBuffer(gl::BufferBinding::PixelPack), 0);
 
     ANGLE_TRY(readPixelsImpl(contextVk, flippedArea, params, VK_IMAGE_ASPECT_COLOR_BIT,
                              getColorReadRenderTarget(),
@@ -408,8 +405,6 @@ angle::Result FramebufferVk::blitWithReadback(ContextVk *contextVk,
     vk::ImageHelper *imageForRead = readRenderTarget->getImageForRead(
         this, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, commandBuffer);
     const angle::Format &drawFormat = drawRenderTarget->getImageFormat().angleFormat();
-    const gl::InternalFormat &sizedInternalDrawFormat =
-        gl::GetSizedInternalFormatInfo(drawFormat.glInternalFormat);
 
     GLuint outputPitch = 0;
 
@@ -429,8 +424,7 @@ angle::Result FramebufferVk::blitWithReadback(ContextVk *contextVk,
     PackPixelsParams packPixelsParams;
     packPixelsParams.pack.alignment       = 1;
     packPixelsParams.pack.reverseRowOrder = true;
-    packPixelsParams.type                 = sizedInternalDrawFormat.type;
-    packPixelsParams.format               = sizedInternalDrawFormat.format;
+    packPixelsParams.destFormat           = &drawFormat;
     packPixelsParams.area.height          = copyArea.height;
     packPixelsParams.area.width           = copyArea.width;
     packPixelsParams.area.x               = copyArea.x;

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -215,13 +215,10 @@ angle::Result PixelBuffer::stageSubresourceUpdateFromFramebuffer(
         pixelPackState.reverseRowOrder = !pixelPackState.reverseRowOrder;
     }
 
-    PackPixelsParams params;
-    params.area        = clippedRectangle;
-    params.format      = formatInfo.format;
-    params.type        = formatInfo.type;
-    params.outputPitch = static_cast<GLuint>(outputRowPitch);
-    params.packBuffer  = nullptr;
-    params.pack        = pixelPackState;
+    const angle::Format &copyFormat =
+        GetFormatFromFormatType(formatInfo.internalFormat, formatInfo.type);
+    PackPixelsParams params(clippedRectangle, copyFormat, static_cast<GLuint>(outputRowPitch),
+                            pixelPackState, nullptr, 0);
 
     // 2- copy the source image region to the pixel buffer using a cpu readback
     if (loadFunction.requiresConversion)

src/libANGLE/renderer/vulkan/vk_format_table_autogen.cpp

@@ -593,10 +593,10 @@ void Format::initialize(VkPhysicalDevice physicalDevice, const angle::Format &an
 
         case angle::FormatID::ETC2_R8G8B8A1_UNORM_BLOCK:
             internalFormat             = GL_COMPRESSED_RGB8_PUNCHTHROUGH_ALPHA1_ETC2;
-            textureFormatID            = angle::FormatID::ETC2_R8G8B8A1_UNORM_BLOCK;
+            textureFormatID              = angle::FormatID::ETC2_R8G8B8A1_UNORM_BLOCK;
             vkTextureFormat            = VK_FORMAT_ETC2_R8G8B8A1_UNORM_BLOCK;
             textureInitializerFunction = Initialize4ComponentData<GLubyte, 0x00, 0x00, 0x00, 0xFF>;
-            bufferFormatID             = angle::FormatID::ETC2_R8G8B8A1_UNORM_BLOCK;
+            bufferFormatID               = angle::FormatID::ETC2_R8G8B8A1_UNORM_BLOCK;
             vkBufferFormat             = VK_FORMAT_ETC2_R8G8B8A1_UNORM_BLOCK;
             vertexLoadFunction         = CopyNativeVertexData<GLubyte, 4, 4, 0>;
             vertexLoadRequiresConversion = false;