source/blender/gpu/vulkan/vk_memory_layout.hh

@@ -11,20 +11,6 @@
 
 namespace blender::gpu {
 
-/**
- * Information about alignment/components and memory size for types when using std430 layout.
- */
-struct Std430 {
-  /** Get the memory size in bytes of a single component using by the given type.*/
-  static uint32_t component_mem_size(const shader::Type type);
-  /** Get to alignment of the given type in bytes.*/
-  static uint32_t element_alignment(const shader::Type type, bool is_array);
-  /** Get the number of components that should be allocated for the given type.*/
-  static uint32_t element_components_len(const shader::Type type);
-  /** Get the number of components of the given type when used in an array.*/
-  static uint32_t array_components_len(const shader::Type type);
-};
-
 /**
  * Information about alignment/components and memory size for types when using std140 layout.
  */
@@ -39,6 +25,39 @@ struct Std140 {
   static uint32_t array_components_len(const shader::Type type);
 };
 
+/**
+ * Information about alignment/components and memory size for types when using std430 layout.
+ */
+struct Std430 {
+  /** Get the memory size in bytes of a single component using by the given type.*/
+  static uint32_t component_mem_size(const shader::Type type);
+  /** Get to alignment of the given type in bytes.*/
+  static uint32_t element_alignment(const shader::Type type, bool is_array);
+  /** Get the number of components that should be allocated for the given type.*/
+  static uint32_t element_components_len(const shader::Type type);
+  /** Get the number of components of the given type when used in an array.*/
+  static uint32_t array_components_len(const shader::Type type);
+};
+
+template<typename Layout> static uint32_t element_stride(const shader::Type type)
+{
+  return Layout::element_components_len(type) * Layout::component_mem_size(type);
+}
+
+template<typename Layout> static uint32_t array_stride(const shader::Type type)
+{
+  return Layout::array_components_len(type) * Layout::component_mem_size(type);
+}
+
+/**
+ * Move the r_offset to the next alignment where the given type+array_size can be
+ * reserved.
+ *
+ * 'type': The type that needs to be aligned.
+ * 'array_size': The array_size that needs to be aligned. (0=no array).
+ * 'r_offset': After the call it will point to the byte where the reservation
+ *     can happen.
+ */
 template<typename Layout>
 static void align(const shader::Type &type, const int32_t array_size, uint32_t *r_offset)
 {
@@ -52,16 +71,18 @@ static void align(const shader::Type &type, const int32_t array_size, uint32_t *
   }
 }
 
-template<typename Layout> static uint32_t element_stride(const shader::Type type)
-{
-  return Layout::element_components_len(type) * Layout::component_mem_size(type);
-}
-
-template<typename Layout> static uint32_t array_stride(const shader::Type type)
-{
-  return Layout::array_components_len(type) * Layout::component_mem_size(type);
-}
-
+/**
+ * Reserve space for the given type and array size.
+ *
+ * This function doesn't handle alignment this needs to be done up front by calling
+ * 'align<Layout>' function. Caller is responsible for this.
+ *
+ * 'type': The type that needs to be reserved.
+ * 'array_size': The array_size that needs to be reserved. (0=no array).
+ * 'r_offset': When calling needs to be pointing to the aligned location where to
+ *     reserve space. After the call it will point to the byte just after reserved
+ *     space.
+ */
 template<typename Layout>
 static void reserve(const shader::Type type, int32_t array_size, uint32_t *r_offset)
 {
@@ -70,6 +91,12 @@ static void reserve(const shader::Type type, int32_t array_size, uint32_t *r_off
   *r_offset += size;
 }
 
+/**
+ * Update 'r_offset' to be aligned to the end of the struct.
+ *
+ * Call this function when all attributes have been added to make sure that the struct size is
+ * correct.
+ */
 template<typename Layout> static void align_end_of_struct(uint32_t *r_offset)
 {
   align<Layout>(shader::Type::VEC4, 0, r_offset);

source/blender/gpu/vulkan/vk_push_constants.hh

@@ -70,9 +70,30 @@ struct VKPushConstantsLayout {
   VKDescriptorSet::Location storage_buffer_binding_;
 
  public:
+  /**
+   * Return the desired storage type that can fit the push constants of the given shader create
+   * info, matching the device limits.
+   *
+   * Returns:
+   * - StorageType::NONE: No push constants are needed.
+   * - StorageType::PUSH_CONSTANTS: Regular vulkan push constants can be used.
+   * - StorageType::UNIFORM_BUFFER: The push constants don't fit in the limits of the given device.
+   *   A uniform buffer should be used as a fallback method.
+   */
   static StorageType determine_storage_type(
       const shader::ShaderCreateInfo &info,
       const VkPhysicalDeviceLimits &vk_physical_device_limits);
+
+  /**
+   * Initialize the push constants of the given shader create info with the
+   * binding location.
+   *
+   * interface: Uniform locations of the interface are used as lookup key.
+   * storage_type: The type of storage for push constants to use.
+   * location: When storage_type=StorageType::UNIFORM_BUFFER this contains
+   *    the location in the descriptor set where the uniform buffer can be
+   *    bound.
+   */
   void init(const shader::ShaderCreateInfo &info,
             const VKShaderInterface &interface,
             StorageType storage_type,
@@ -86,21 +107,44 @@ struct VKPushConstantsLayout {
     return storage_type_;
   }
 
+  /**
+   * Get the binding location for the uniform buffer.
+   *
+   * Only valid when storage_type=StorageType::UNIFORM_BUFFER.
+   */
   VKDescriptorSet::Location storage_buffer_binding_get() const
   {
     return storage_buffer_binding_;
   }
 
+  /**
+   * Get the size needed to store the push constants.
+   */
   uint32_t size_in_bytes() const
   {
     return size_in_bytes_;
   }
 
+  /**
+   * Find the push constant layout for the given location.
+   * Location = ShaderInput.location.
+   */
   const PushConstantLayout *find(int32_t location) const;
 };
 
+/**
+ * Container to store push constants in a buffer.
+ *
+ * Can handle buffers with different memory layouts (std140/std430)
+ * Which memory layout is used is based on the storage type.
+ *
+ * VKPushConstantsLayout only describes the buffer, an instance of this
+ * class can handle setting/modifying/duplicating push constants.
+ *
+ * It should also keep track of the submissions in order to reuse the allocated
+ * data.
+ */
 class VKPushConstants : NonCopyable {
-
  private:
   const VKPushConstantsLayout *layout_ = nullptr;
   void *data_ = nullptr;
@@ -124,14 +168,46 @@ class VKPushConstants : NonCopyable {
     return *layout_;
   }
 
+  /**
+   * When storage type = StorageType::UNIFORM_BUFFER use this method to update the uniform
+   * buffer.
+   *
+   * It must be called just before adding a draw/compute command to the command queue.
+   */
   void update_uniform_buffer();
+
+  /**
+   * Get a reference to the uniform buffer.
+   *
+   * Only valid when storage type = StorageType::UNIFORM_BUFFER.
+   */
   VKUniformBuffer &uniform_buffer_get();
 
+  /**
+   * Get the reference to the active data.
+   *
+   * Data can get inactive when push constants are modified, after being added to the command
+   * queue. We still keep track of the old data for reuse and make sure we don't overwrite data
+   * that is still not on the GPU.
+   */
   const void *data() const
   {
     return data_;
   }
 
+  /**
+   * Modify a push constant.
+   *
+   * location: ShaderInput.location of the push constant to update.
+   * comp_len: number of components has the data type that is being updated.
+   * array_size: number of elements when an array to update. (0=no array)
+   * input_data: packed source data to use.
+   *
+   * TODO: this function still needs to convert the input_data layout to that
+   * what the storage type is expected.
+   * TODO: Current implementation has a work around for missing implementation
+   * of builtin uniforms. Builtin uniforms should eventually also be supported.
+   */
   template<typename T>
   void push_constant_set(int32_t location,
                          int32_t comp_len,

source/blender/gpu/vulkan/vk_shader_interface.hh

@@ -32,6 +32,9 @@ class VKShaderInterface : public ShaderInterface {
   VKPushConstantsLayout push_constants_layout_;
 
  public:
+  /**
+   * When the push constants fallback is used, this name will be used in the GLSL source.
+   */
   static constexpr StringRefNull PUSH_CONSTANTS_FALLBACK_NAME = StringRefNull(
       "push_constants_fallback", 23);
   static constexpr size_t PUSH_CONSTANTS_FALLBACK_NAME_LEN = PUSH_CONSTANTS_FALLBACK_NAME.size();
@@ -46,6 +49,7 @@ class VKShaderInterface : public ShaderInterface {
   const VKDescriptorSet::Location descriptor_set_location(
       const shader::ShaderCreateInfo::Resource::BindType &bind_type, int binding) const;
 
+  /** Get the VKPushConstantsLayout of the shader.*/
   const VKPushConstantsLayout &push_constants_layout_get() const
   {
     return push_constants_layout_;