Skip to content

Commit 7eaa47e

Browse files
committed
addressing more comments.
1 parent f1df3d2 commit 7eaa47e

File tree

5 files changed

+51
-30
lines changed

5 files changed

+51
-30
lines changed

en/Building_a_Simple_Engine/Camera_Transformations/05_vulkan_integration.adoc

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,46 @@ struct UniformBufferObject {
4242

4343
Next, we'll create the uniform buffer and its descriptor set:
4444

45+
[NOTE]
46+
====
47+
Uniform buffers should be allocated per frame-in-flight (maxConcurrentFrames), not per swapchain image. This matches how you submit work and synchronize frames, avoids unnecessary allocations, and simplifies your logic.
48+
====
49+
4550
[source,cpp]
4651
----
52+
// Use a fixed number of frames-in-flight, not the number of swapchain images
53+
constexpr uint32_t maxConcurrentFrames = 2; // Adjust to your renderer
54+
55+
struct UniformBufferObject {
56+
glm::mat4 model;
57+
glm::mat4 view;
58+
glm::mat4 proj;
59+
};
60+
61+
// Keep the mapped pointer alongside the buffer for clarity and safety
62+
struct UboBuffer {
63+
vk::raii::Buffer buffer{nullptr};
64+
vk::raii::DeviceMemory memory{nullptr};
65+
void* mapped = nullptr;
66+
};
67+
68+
std::array<UboBuffer, maxConcurrentFrames> uniformBuffers;
69+
4770
void createUniformBuffers() {
4871
vk::DeviceSize bufferSize = sizeof(UniformBufferObject);
4972
50-
uniformBuffers.resize(swapChainImages.size());
51-
uniformBuffersMapped.resize(swapChainImages.size());
52-
53-
for (size_t i = 0; i < swapChainImages.size(); i++) {
73+
for (size_t i = 0; i < maxConcurrentFrames; i++) {
5474
// Create the buffer
5575
vk::BufferCreateInfo bufferInfo{
5676
.size = bufferSize,
5777
.usage = vk::BufferUsageFlagBits::eUniformBuffer,
5878
.sharingMode = vk::SharingMode::eExclusive
5979
};
6080
61-
uniformBuffers[i] = vk::raii::Buffer(device, bufferInfo);
81+
uniformBuffers[i].buffer = vk::raii::Buffer(device, bufferInfo);
6282
6383
// Allocate and bind memory
64-
vk::MemoryRequirements memRequirements = uniformBuffers[i].getMemoryRequirements();
84+
vk::MemoryRequirements memRequirements = uniformBuffers[i].buffer.getMemoryRequirements();
6585
6686
vk::MemoryAllocateInfo allocInfo{
6787
.allocationSize = memRequirements.size,
@@ -71,11 +91,11 @@ void createUniformBuffers() {
7191
)
7292
};
7393
74-
uniformBuffersMemory[i] = vk::raii::DeviceMemory(device, allocInfo);
75-
uniformBuffers[i].bindMemory(*uniformBuffersMemory[i], 0);
94+
uniformBuffers[i].memory = vk::raii::DeviceMemory(device, allocInfo);
95+
uniformBuffers[i].buffer.bindMemory(*uniformBuffers[i].memory, 0);
7696
7797
// Persistently map the buffer memory
78-
uniformBuffersMapped[i] = uniformBuffersMemory[i].mapMemory(0, bufferSize);
98+
uniformBuffers[i].mapped = uniformBuffers[i].memory.mapMemory(0, bufferSize);
7999
}
80100
}
81101
----
@@ -111,19 +131,20 @@ Now we'll create descriptor sets that point to our uniform buffers:
111131
[source,cpp]
112132
----
113133
void createDescriptorSets() {
114-
std::vector<vk::DescriptorSetLayout> layouts(swapChainImages.size(), *descriptorSetLayout);
134+
std::array<vk::DescriptorSetLayout, maxConcurrentFrames> layouts{};
135+
layouts.fill(*descriptorSetLayout);
115136
116137
vk::DescriptorSetAllocateInfo allocInfo{
117138
.descriptorPool = *descriptorPool,
118-
.descriptorSetCount = static_cast<uint32_t>(swapChainImages.size()),
139+
.descriptorSetCount = maxConcurrentFrames,
119140
.pSetLayouts = layouts.data()
120141
};
121142
122143
descriptorSets = device.allocateDescriptorSets(allocInfo);
123144
124-
for (size_t i = 0; i < swapChainImages.size(); i++) {
145+
for (size_t i = 0; i < maxConcurrentFrames; i++) {
125146
vk::DescriptorBufferInfo bufferInfo{
126-
.buffer = *uniformBuffers[i],
147+
.buffer = *uniformBuffers[i].buffer,
127148
.offset = 0,
128149
.range = sizeof(UniformBufferObject)
129150
};
@@ -148,7 +169,7 @@ In our main loop, we'll update the uniform buffer with the latest camera data:
148169

149170
[source,cpp]
150171
----
151-
void updateUniformBuffer(uint32_t currentImage) {
172+
void updateUniformBuffer(uint32_t currentFrame) {
152173
static auto startTime = std::chrono::high_resolution_clock::now();
153174
auto currentTime = std::chrono::high_resolution_clock::now();
154175
float time = std::chrono::duration<float, std::chrono::seconds::period>(currentTime - startTime).count();
@@ -167,8 +188,8 @@ void updateUniformBuffer(uint32_t currentImage) {
167188
// Vulkan's Y coordinate is inverted compared to OpenGL
168189
ubo.proj[1][1] *= -1;
169190
170-
// Copy the data to the uniform buffer
171-
memcpy(uniformBuffersMapped[currentImage], &ubo, sizeof(ubo));
191+
// Copy the data to the uniform buffer for the current frame-in-flight
192+
memcpy(uniformBuffers[currentFrame].mapped, &ubo, sizeof(ubo));
172193
}
173194
----
174195

en/Building_a_Simple_Engine/Lighting_Materials/04_lighting_implementation.adoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ This function creates a new pipeline for our PBR shader, including support for p
691691
[source,cpp]
692692
----
693693
// Update uniform buffer
694-
void Renderer::updateUniformBuffer(uint32_t currentImage, Entity* entity, CameraComponent* camera) {
694+
void Renderer::updateUniformBuffer(uint32_t currentFrame, Entity* entity, CameraComponent* camera) {
695695
// Get the transform component from the entity
696696
auto transform = entity->GetComponent<TransformComponent>();
697697
if (!transform) {
@@ -744,7 +744,7 @@ void Renderer::updateUniformBuffer(uint32_t currentImage, Entity* entity, Camera
744744
745745
// Copy the uniform buffer object to the device memory using vk::raii
746746
// With vk::raii, we can use the mapped memory directly
747-
memcpy(uniformBuffersMapped[currentImage], &ubo, sizeof(ubo));
747+
memcpy(uniformBuffers[currentFrame].mapped, &ubo, sizeof(ubo));
748748
}
749749
----
750750

en/Building_a_Simple_Engine/Loading_Models/05_pbr_rendering.adoc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ void setupLights() {
408408
};
409409
410410
// Update uniform buffer with light data
411-
for (size_t i = 0; i < swapChainImages.size(); i++) {
411+
for (size_t i = 0; i < maxConcurrentFrames; i++) {
412412
UniformBufferObject ubo{};
413413
// ... (set up transformation matrices)
414414
@@ -425,8 +425,8 @@ void setupLights() {
425425
ubo.exposure = 4.5f;
426426
ubo.gamma = 2.2f;
427427
428-
// Copy to uniform buffer
429-
memcpy(uniformBuffersMapped[i], &ubo, sizeof(ubo));
428+
// Copy to uniform buffer (per frame-in-flight)
429+
memcpy(uniformBuffers[i].mapped, &ubo, sizeof(ubo));
430430
}
431431
}
432432
----
@@ -437,7 +437,7 @@ PBR relies on view-dependent effects like the Fresnel effect, so we need to inte
437437

438438
[source,cpp]
439439
----
440-
void updateUniformBuffer(uint32_t currentImage) {
440+
void updateUniformBuffer(uint32_t currentFrame) {
441441
UniformBufferObject ubo{};
442442
443443
// Update transformation matrices
@@ -453,8 +453,8 @@ void updateUniformBuffer(uint32_t currentImage) {
453453
454454
// ... (update other PBR parameters)
455455
456-
// Copy to uniform buffer
457-
memcpy(uniformBuffersMapped[currentImage], &ubo, sizeof(ubo));
456+
// Copy to uniform buffer (per frame-in-flight)
457+
memcpy(uniformBuffers[currentFrame].mapped, &ubo, sizeof(ubo));
458458
}
459459
----
460460

en/Building_a_Simple_Engine/Loading_Models/06_multiple_objects.adoc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,8 @@ void drawFrame() {
234234
ubo.proj = camera.getProjectionMatrix(swapChainExtent.width / (float)swapChainExtent.height);
235235
ubo.proj[1][1] *= -1; // Vulkan's Y coordinate is inverted
236236
237-
// Copy to uniform buffer
238-
memcpy(uniformBuffersMapped[currentFrame], &ubo, sizeof(ubo));
237+
// Copy to uniform buffer (per frame-in-flight)
238+
memcpy(uniformBuffers[currentFrame].mapped, &ubo, sizeof(ubo));
239239
240240
// Render each object instance
241241
for (size_t i = 0; i < objectInstances.size(); i++) {
@@ -535,8 +535,8 @@ void drawFrame() {
535535
ubo.proj = camera.getProjectionMatrix(swapChainExtent.width / (float)swapChainExtent.height);
536536
ubo.proj[1][1] *= -1; // Vulkan's Y coordinate is inverted
537537
538-
// Copy to uniform buffer
539-
memcpy(uniformBuffersMapped[currentFrame], &ubo, sizeof(ubo));
538+
// Copy to uniform buffer (per frame-in-flight)
539+
memcpy(uniformBuffers[currentFrame].mapped, &ubo, sizeof(ubo));
540540
541541
// Bind descriptor set
542542
commandBuffer.bindDescriptorSets(

en/Building_a_Simple_Engine/Loading_Models/07_scene_rendering.adoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ void drawFrame() {
357357
ubo.proj = camera.getProjectionMatrix(swapChainExtent.width / (float)swapChainExtent.height);
358358
ubo.proj[1][1] *= -1; // Vulkan's Y coordinate is inverted
359359
360-
// Copy to uniform buffer
361-
memcpy(uniformBuffersMapped[currentFrame], &ubo, sizeof(ubo));
360+
// Copy to uniform buffer (per frame-in-flight)
361+
memcpy(uniformBuffers[currentFrame].mapped, &ubo, sizeof(ubo));
362362
363363
// Render the scene
364364
renderScene(commandBuffer, model, ubo.view, ubo.proj);

0 commit comments

Comments
 (0)