Address CR comments

Note: CR comments regarding `ShaderSet` and `PipelineStages` will be addressed at a later date with a common class for associative enum arrays.

app/src/main/cpp/skyline/gpu/descriptor_allocator.cpp

@@ -6,7 +6,7 @@
 #include "descriptor_allocator.h"
 
 namespace skyline::gpu {
-    DescriptorAllocator::DescriptorPool::DescriptorPool(const vk::raii::Device &device, const vk::DescriptorPoolCreateInfo &createInfo) : vk::raii::DescriptorPool(device, createInfo), setCount(createInfo.maxSets) {}
+    DescriptorAllocator::DescriptorPool::DescriptorPool(const vk::raii::Device &device, const vk::DescriptorPoolCreateInfo &createInfo) : vk::raii::DescriptorPool(device, createInfo), freeSetCount(createInfo.maxSets) {}
 
     void DescriptorAllocator::AllocateDescriptorPool() {
         namespace maxwell3d = soc::gm20b::engine::maxwell3d::type; // We use Maxwell3D as reference for base descriptor counts
@@ -31,13 +31,13 @@ namespace skyline::gpu {
     }
 
     DescriptorAllocator::ActiveDescriptorSet::ActiveDescriptorSet(std::shared_ptr<DescriptorPool> pPool, vk::DescriptorSet set) : pool(std::move(pPool)), DescriptorSet(set) {
-        pool->setCount--;
+        pool->freeSetCount--;
     }
 
     DescriptorAllocator::ActiveDescriptorSet::~ActiveDescriptorSet() {
         std::scoped_lock lock(*pool);
         pool->getDevice().freeDescriptorSets(**pool, 1, this, *pool->getDispatcher());
-        pool->setCount++;
+        pool->freeSetCount++;
     }
 
     DescriptorAllocator::DescriptorAllocator(GPU &gpu) : gpu(gpu) {
@@ -61,9 +61,9 @@ namespace skyline::gpu {
             if (result == vk::Result::eSuccess) {
                 return ActiveDescriptorSet(pool, set);
             } else if (result == vk::Result::eErrorOutOfPoolMemory) {
-                if (pool->setCount == 0)
+                if (pool->freeSetCount == 0)
                     // The amount of maximum descriptor sets is insufficient
-                    descriptorSetCount += BaseDescriptorSetCount;
+                    descriptorSetCount += DescriptorSetCountIncrement;
                 else
                     // The amount of maximum descriptors is insufficient
                     descriptorMultiplier++;

app/src/main/cpp/skyline/gpu/descriptor_allocator.h

@@ -14,15 +14,15 @@ namespace skyline::gpu {
         GPU &gpu;
         std::mutex mutex; //!< Synchronizes the creation and replacement of the pool object
 
-        static constexpr u32 BaseDescriptorSetCount{64}; //!< An arbitrary amount of descriptor sets that we allocate in multiples of
+        static constexpr u32 DescriptorSetCountIncrement{64}; //!< The amount of descriptor sets that we allocate in increments of
-        u32 descriptorSetCount{BaseDescriptorSetCount}; //!< The maximum amount of descriptor sets in the pool
+        u32 descriptorSetCount{DescriptorSetCountIncrement}; //!< The maximum amount of descriptor sets in the pool
         u32 descriptorMultiplier{1}; //!< A multiplier for the maximum amount of descriptors in the pool
 
         /**
          * @brief A lockable VkDescriptorPool for maintaining external synchronization requirements
          */
         struct DescriptorPool : public std::mutex, public vk::raii::DescriptorPool {
-            u64 setCount{}; //!< The amount of sets free to allocate from this pool
+            u64 freeSetCount{}; //!< The amount of sets free to allocate from this pool
 
             DescriptorPool(vk::raii::Device const &device, vk::DescriptorPoolCreateInfo const &createInfo);
         };
@@ -30,14 +30,14 @@ namespace skyline::gpu {
         std::shared_ptr<DescriptorPool> pool; //!< The current pool used by any allocations in the class, replaced when an error is ran into
 
         /**
-         * @brief (Re-)Allocates the descriptor pool with the current multiplier applied to the base counts
+         * @brief (Re-)Allocates the descriptor pool with the current multiplier applied to the descriptor counts and the current descriptor set count
          * @note `DescriptorAllocator::mutex` **must** be locked prior to calling this
          */
         void AllocateDescriptorPool();
 
       public:
         /**
-         * @brief A RAII-bound descriptor set that automatically frees of resources into the pool on destruction while respecting external synchronization requirements
+         * @brief A RAII-bound descriptor set that automatically frees resources into the pool on destruction while respecting external synchronization requirements
          */
         struct ActiveDescriptorSet : public vk::DescriptorSet {
           private:
@@ -56,7 +56,7 @@ namespace skyline::gpu {
         DescriptorAllocator(GPU &gpu);
 
         /**
-         * @note It is UB to allocate a set with a descriptor type that isn't in the pool
+         * @note It is UB to allocate a set with a descriptor type that isn't in the pool as defined in AllocateDescriptorPool
          */
         ActiveDescriptorSet AllocateSet(vk::DescriptorSetLayout layout);
     };

app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h

@@ -592,8 +592,8 @@ namespace skyline::gpu::interconnect {
             }
         };
 
-        struct Shaders : public std::array<Shader, maxwell3d::ShaderStageCount> {
+        struct ShaderSet : public std::array<Shader, maxwell3d::ShaderStageCount> {
-            Shaders() : array({
+            ShaderSet() : array({
                                   Shader{ShaderCompiler::Stage::VertexA},
                                   Shader{ShaderCompiler::Stage::VertexB},
                                   Shader{ShaderCompiler::Stage::TessellationControl},
@@ -646,7 +646,7 @@ namespace skyline::gpu::interconnect {
         };
 
         IOVA shaderBaseIova{}; //!< The base IOVA that shaders are located at an offset from
-        Shaders shaders;
+        ShaderSet shaders;
         PipelineStages pipelineStages;
 
         std::array<vk::PipelineShaderStageCreateInfo, maxwell3d::PipelineStageCount> shaderStagesInfo{}; //!< Storage backing for the pipeline shader stage information for all shaders aside from 'VertexA' which uses the same stage as 'VertexB'
@@ -688,7 +688,7 @@ namespace skyline::gpu::interconnect {
                 auto setStageRecompile{[this](maxwell3d::PipelineStage stage) {
                     pipelineStages[stage].needsRecompile = true;
                 }};
-                ((void) setStageRecompile(stages), ...);
+                (setStageRecompile(stages), ...);
             }
         }
 
@@ -704,24 +704,26 @@ namespace skyline::gpu::interconnect {
                     if (shader.invalidated) {
                         // If a shader is invalidated, we need to reparse the program (given that it has changed)
 
-                        bool shouldParseShader{true};
+                        bool shouldParseShader{[&]() {
-                        if (!shader.data.empty() && shader.shouldCheckSame) {
+                            if (!shader.data.empty() && shader.shouldCheckSame) {
-                            // A fast path to check if the shader is the same as before to avoid reparsing the shader
+                                // A fast path to check if the shader is the same as before to avoid reparsing the shader
-                            auto newIovaRanges{channelCtx.asCtx->gmmu.TranslateRange(shaderBaseIova + shader.offset, shader.data.size())};
+                                auto newIovaRanges{channelCtx.asCtx->gmmu.TranslateRange(shaderBaseIova + shader.offset, shader.data.size())};
-                            auto originalShader{shader.data.data()};
+                                auto originalShader{shader.data.data()};
 
-                            shouldParseShader = false;
+                                for (auto &range : newIovaRanges) {
-                            for (auto &range : newIovaRanges) {
+                                    if (range.data() && std::memcmp(range.data(), originalShader, range.size()) == 0) {
-                                if (range.data() && std::memcmp(range.data(), originalShader, range.size()) == 0) {
+                                        originalShader += range.size();
-                                    originalShader += range.size();
+                                    } else {
-                                } else {
+                                        return true;
-                                    shouldParseShader = true;
+                                    }
-                                    break;
                                 }
-                            }
 
-                            shader.shouldCheckSame = true;
+                                return false;
-                        }
+                            } else {
+                                shader.shouldCheckSame = true; // We want to reset the value and check for it being same the next time
+                                return true;
+                            }
+                        }()};
 
                         if (shouldParseShader) {
                             // A pass to check if the shader has a BRA infloop opcode ending (On most commercial games)

app/src/main/cpp/skyline/gpu/shader_manager.cpp

@@ -117,11 +117,11 @@ namespace skyline::gpu {
         }
 
         [[nodiscard]] u32 SharedMemorySize() const final {
-            return 0; // Shared memory size is only relevant for compute shaders
+            return 0; // Only relevant for compute shaders
         }
 
         [[nodiscard]] std::array<u32, 3> WorkgroupSize() const final {
-            return {0, 0, 0}; // Workgroup size is only relevant for compute shaders
+            return {0, 0, 0}; // Only relevant for compute shaders
         }
     };
 
@@ -156,11 +156,11 @@ namespace skyline::gpu {
         }
 
         [[nodiscard]] u32 SharedMemorySize() const final {
-            return 0;
+            return 0; // Only relevant for compute shaders
         }
 
         [[nodiscard]] std::array<u32, 3> WorkgroupSize() const final {
-            return {0, 0, 0};
+            return {0, 0, 0}; // Only relevant for compute shaders
         }
     };