From 48077c15e8f5aa6c840e4e9310c88ad1879ada53 Mon Sep 17 00:00:00 2001
From: Artur Wasmut <awasmut@uni-koblenz.de>
Date: Mon, 14 Jun 2021 17:34:32 +0200
Subject: [PATCH] Remove offset calculation in shader reflection. Proper
 offsets are now calculated on vertex binding creation.

---
 include/vkcv/VertexLayout.hpp       |  5 +++--
 projects/cmd_sync_test/src/main.cpp |  9 ---------
 projects/first_mesh/src/main.cpp    | 23 +++--------------------
 src/vkcv/ShaderProgram.cpp          | 10 +---------
 src/vkcv/VertexLayout.cpp           | 13 +++++++++----
 5 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/include/vkcv/VertexLayout.hpp b/include/vkcv/VertexLayout.hpp
index 67687f64..9f609b48 100644
--- a/include/vkcv/VertexLayout.hpp
+++ b/include/vkcv/VertexLayout.hpp
@@ -19,14 +19,15 @@ namespace vkcv{
 	uint32_t getFormatSize(VertexAttachmentFormat format);
 
     struct VertexAttachment{
+        friend struct VertexBinding;
         /**
          * Describes an individual vertex input attribute/attachment.
          * @param inputLocation its location in the vertex shader.
          * @param name the name referred to in the shader.
          * @param format the format (and therefore, the size) this attachment is in.
-         * @param offset the attachment's byte offset within a vertex.
+         * The offset is calculated when a collection of attachments forms a binding, hence the friend declaration.
          */
-        VertexAttachment(uint32_t inputLocation, const std::string &name, VertexAttachmentFormat format, uint32_t offset) noexcept;
+        VertexAttachment(uint32_t inputLocation, const std::string &name, VertexAttachmentFormat format) noexcept;
         VertexAttachment() = delete;
 
         uint32_t                inputLocation;
diff --git a/projects/cmd_sync_test/src/main.cpp b/projects/cmd_sync_test/src/main.cpp
index a8d7b8cb..426cc3a5 100644
--- a/projects/cmd_sync_test/src/main.cpp
+++ b/projects/cmd_sync_test/src/main.cpp
@@ -101,15 +101,6 @@ int main(int argc, const char** argv) {
     firstMeshProgram.addShader(vkcv::ShaderStage::VERTEX, std::filesystem::path("resources/shaders/vert.spv"));
     firstMeshProgram.addShader(vkcv::ShaderStage::FRAGMENT, std::filesystem::path("resources/shaders/frag.spv"));
 
-    /**
-     * TODO:
-     *  Since the framework's vertex layout specification is now separate
-     *  from that defined in the asset loader module, there needs to be a smarter way to translate the asset loader's
-     *  specific layout into "our" uniform vertex layout spec.
-     *
-     *  This is just a quick hack.
-     */
-
     const std::vector<vkcv::VertexAttachment> vertexAttachments = firstMeshProgram.getVertexAttachments();
     
     std::vector<vkcv::VertexBinding> bindings;
diff --git a/projects/first_mesh/src/main.cpp b/projects/first_mesh/src/main.cpp
index 804c316b..8f461512 100644
--- a/projects/first_mesh/src/main.cpp
+++ b/projects/first_mesh/src/main.cpp
@@ -91,17 +91,8 @@ int main(int argc, const char** argv) {
 	std::sort(attributes.begin(), attributes.end(), [](const vkcv::asset::VertexAttribute& x, const vkcv::asset::VertexAttribute& y) {
 		return static_cast<uint32_t>(x.type) < static_cast<uint32_t>(y.type);
 	});
-	
-	/**
-	 * TODO:
-	 *  Since the framework's vertex layout specification is now separate
-	 *  from that defined in the asset loader module, there needs to be a smarter way to translate the asset loader's
-	 *  specific layout into "our" uniform vertex layout spec.
-	 *
-	 */
 
     const std::vector<vkcv::VertexAttachment> vertexAttachments = firstMeshProgram.getVertexAttachments();
-	
 	std::vector<vkcv::VertexBinding> bindings;
 	for (size_t i = 0; i < vertexAttachments.size(); i++) {
 		bindings.push_back(vkcv::VertexBinding(i, { vertexAttachments[i] }));
@@ -138,18 +129,10 @@ int main(int argc, const char** argv) {
 		vkcv::SamplerAddressMode::REPEAT
 	);
 
-    /*
-	const std::vector<vkcv::VertexBufferBinding> vertexBufferBindings = {
-		vkcv::VertexBufferBinding( static_cast<vk::DeviceSize>(mesh.vertexGroups[0].vertexBuffer.attributes[0].offset), vertexBuffer.getVulkanHandle() ),
-		vkcv::VertexBufferBinding( static_cast<vk::DeviceSize>(mesh.vertexGroups[0].vertexBuffer.attributes[1].offset), vertexBuffer.getVulkanHandle() ),
-		vkcv::VertexBufferBinding( static_cast<vk::DeviceSize>(mesh.vertexGroups[0].vertexBuffer.attributes[2].offset), vertexBuffer.getVulkanHandle() )
-	};
-    */
-	
 	const std::vector<vkcv::VertexBufferBinding> vertexBufferBindings = {
-			vkcv::VertexBufferBinding(attributes[0].offset, vertexBuffer.getVulkanHandle()),
-			vkcv::VertexBufferBinding(attributes[1].offset, vertexBuffer.getVulkanHandle()),
-			vkcv::VertexBufferBinding(attributes[2].offset, vertexBuffer.getVulkanHandle()) };
+			vkcv::VertexBufferBinding(static_cast<vk::DeviceSize>(attributes[0].offset), vertexBuffer.getVulkanHandle()),
+			vkcv::VertexBufferBinding(static_cast<vk::DeviceSize>(attributes[1].offset), vertexBuffer.getVulkanHandle()),
+			vkcv::VertexBufferBinding(static_cast<vk::DeviceSize>(attributes[2].offset), vertexBuffer.getVulkanHandle()) };
 
 	vkcv::DescriptorWrites setWrites;
 	setWrites.sampledImageWrites    = { vkcv::SampledImageDescriptorWrite(0, texture.getHandle()) };
diff --git a/src/vkcv/ShaderProgram.cpp b/src/vkcv/ShaderProgram.cpp
index 96a3c4a9..48f1e8df 100644
--- a/src/vkcv/ShaderProgram.cpp
+++ b/src/vkcv/ShaderProgram.cpp
@@ -117,8 +117,6 @@ namespace vkcv {
         //reflect vertex input
 		if (shaderStage == ShaderStage::VERTEX)
 		{
-			uint32_t attachment_offset = 0;
-
 			// spirv-cross API (hopefully) returns the stage_inputs in order
 			for (uint32_t i = 0; i < resources.stage_inputs.size(); i++)
 			{
@@ -133,13 +131,7 @@ namespace vkcv {
 				// vertex input format (implies its size)
 				const VertexAttachmentFormat attachment_format = convertFormat(base_type.basetype, base_type.vecsize);
 
-                m_VertexAttachments.emplace_back(attachment_loc,
-                                                 attachment_name,
-                                                 attachment_format,
-                                                 attachment_offset);
-
-                // calculate offset for the next vertex attachment input in line
-                attachment_offset += base_type.vecsize * base_type.width / 8;
+                m_VertexAttachments.emplace_back(attachment_loc, attachment_name, attachment_format);
             }
 		}
 
diff --git a/src/vkcv/VertexLayout.cpp b/src/vkcv/VertexLayout.cpp
index 23d01840..e6651cfd 100644
--- a/src/vkcv/VertexLayout.cpp
+++ b/src/vkcv/VertexLayout.cpp
@@ -30,11 +30,11 @@ namespace vkcv {
         }
     }
 
-    VertexAttachment::VertexAttachment(uint32_t inputLocation, const std::string &name, VertexAttachmentFormat format, uint32_t offset) noexcept:
+    VertexAttachment::VertexAttachment(uint32_t inputLocation, const std::string &name, VertexAttachmentFormat format) noexcept:
             inputLocation{inputLocation},
             name{name},
             format{format},
-            offset{offset}
+            offset{0}
     {}
 
 
@@ -43,8 +43,13 @@ namespace vkcv {
     stride{0},
     vertexAttachments{attachments}
     {
-        for (const auto &attachment : attachments)
-            stride += getFormatSize(attachment.format);
+        uint32_t offset = 0;
+        for (auto &attachment : vertexAttachments)
+        {
+            offset += getFormatSize(attachment.format);
+            attachment.offset = offset;
+            stride += offset;
+        }
     }
 
     VertexLayout::VertexLayout() noexcept :
-- 
GitLab