From 07f4211dd3ced9bb2ffe91cfa7f3f8db54814f5b Mon Sep 17 00:00:00 2001
From: Philip Rideout <philiprideout@gmail.com>
Date: Mon, 28 Feb 2022 15:13:51 -0800
Subject: [PATCH] Vulkan: cleanup as per code review.

---
 .../backend/src/vulkan/VulkanPipelineCache.h  | 43 ++++++++-----------
 .../backend/src/vulkan/VulkanSwapChain.cpp    |  4 +-
 filament/backend/src/vulkan/VulkanSwapChain.h |  4 +-
 3 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/filament/backend/src/vulkan/VulkanPipelineCache.h b/filament/backend/src/vulkan/VulkanPipelineCache.h
index bd448fa901..d206058027 100644
--- a/filament/backend/src/vulkan/VulkanPipelineCache.h
+++ b/filament/backend/src/vulkan/VulkanPipelineCache.h
@@ -97,20 +97,18 @@ public:
         VkBool32              blendEnable : 1;
         VkBool32              depthWriteEnable : 1;
         VkBool32              alphaToCoverageEnable : 1;
-        VkBlendFactor         srcColorBlendFactor : 5;
+        VkBlendFactor         srcColorBlendFactor : 5; // offset = 1 byte
         VkBlendFactor         dstColorBlendFactor : 5;
         VkBlendFactor         srcAlphaBlendFactor : 5;
         VkBlendFactor         dstAlphaBlendFactor : 5;
         VkColorComponentFlags colorWriteMask : 4;
-        uint8_t               rasterizationSamples;
-        uint8_t               colorTargetCount;
-        uint8_t               padding0 : 2;
-        BlendEquation         colorBlendOp : 3;
-        BlendEquation         alphaBlendOp : 3;
-        SamplerCompareFunc    depthCompareOp : 3;
-        uint8_t               padding1 : 5;
-        float                 depthBiasConstantFactor;
-        float                 depthBiasSlopeFactor;
+        uint8_t               rasterizationSamples;    // offset = 4 bytes
+        uint8_t               colorTargetCount;        // offset = 5 bytes
+        BlendEquation         colorBlendOp : 4;        // offset = 6 bytes
+        BlendEquation         alphaBlendOp : 4;
+        SamplerCompareFunc    depthCompareOp;          // offset = 7 bytes
+        float                 depthBiasConstantFactor; // offset = 8 bytes
+        float                 depthBiasSlopeFactor;    // offset = 12 bytes
     };
 
     static_assert(std::is_trivially_copyable<RasterState>::value,
@@ -244,23 +242,18 @@ private:
     };
 
     // The pipeline key is a POD that represents all currently bound states that form the immutable
-    // VkPipeline object.
-    struct PipelineKey {
-        VkShaderModule shaders[SHADER_MODULE_COUNT]; // 0
-        VkRenderPass renderPass;                     // 16
-        uint16_t topology : 16;                      // 24
-        uint16_t subpassIndex;                       // 26
-        VertexInputAttributeDescription vertexAttributes[VERTEX_ATTRIBUTE_COUNT]; // 28
-        VertexInputBindingDescription vertexBuffers[VERTEX_ATTRIBUTE_COUNT];      // 156
-        RasterState rasterState;                     // 284
-        uint32_t padding;                            // 300
+    // VkPipeline object. The size:offset comments below are expressed in bytes.
+    struct PipelineKey {                                                          // size : offset
+        VkShaderModule shaders[SHADER_MODULE_COUNT];                              //  16  : 0
+        VkRenderPass renderPass;                                                  //  8   : 16
+        uint16_t topology;                                                        //  2   : 24
+        uint16_t subpassIndex;                                                    //  2   : 26
+        VertexInputAttributeDescription vertexAttributes[VERTEX_ATTRIBUTE_COUNT]; //  128 : 28
+        VertexInputBindingDescription vertexBuffers[VERTEX_ATTRIBUTE_COUNT];      //  128 : 156
+        RasterState rasterState;                                                  //  16  : 284
+        uint32_t padding;                                                         //  4   : 300
     };
 
-    static_assert(offsetof(PipelineKey, renderPass)       == 16);
-    static_assert(offsetof(PipelineKey, subpassIndex)     == 26);
-    static_assert(offsetof(PipelineKey, vertexAttributes) == 28);
-    static_assert(offsetof(PipelineKey, vertexBuffers)    == 156);
-    static_assert(offsetof(PipelineKey, rasterState)      == 284);
     static_assert(sizeof(PipelineKey) == 304, "PipelineKey must not have implicit padding.");
 
     using PipelineHashFn = utils::hash::MurmurHashFn<PipelineKey>;
diff --git a/filament/backend/src/vulkan/VulkanSwapChain.cpp b/filament/backend/src/vulkan/VulkanSwapChain.cpp
index f0c5fdb150..b9153f84bf 100644
--- a/filament/backend/src/vulkan/VulkanSwapChain.cpp
+++ b/filament/backend/src/vulkan/VulkanSwapChain.cpp
@@ -347,11 +347,11 @@ bool VulkanSwapChain::hasResized() const {
     return !equivalent(clientSize, surfaceCapabilities.currentExtent);
 }
 
-VulkanTexture& VulkanSwapChain::getColorTexture() const {
+VulkanTexture& VulkanSwapChain::getColorTexture() {
     return *mColor[mCurrentSwapIndex];
 }
 
-VulkanTexture& VulkanSwapChain::getDepthTexture() const {
+VulkanTexture& VulkanSwapChain::getDepthTexture() {
     return *mDepth;
 }
 
diff --git a/filament/backend/src/vulkan/VulkanSwapChain.h b/filament/backend/src/vulkan/VulkanSwapChain.h
index 0d34a9614d..713d638eb2 100644
--- a/filament/backend/src/vulkan/VulkanSwapChain.h
+++ b/filament/backend/src/vulkan/VulkanSwapChain.h
@@ -39,8 +39,8 @@ struct VulkanSwapChain : public HwSwapChain {
     void makePresentable();
     bool hasResized() const;
 
-    VulkanTexture& getColorTexture() const;
-    VulkanTexture& getDepthTexture() const;
+    VulkanTexture& getColorTexture();
+    VulkanTexture& getDepthTexture();
     uint32_t getSwapIndex() const { return mCurrentSwapIndex; }
 
     VkSurfaceKHR surface = {};