Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions src/graphics/vulkan/pipelines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,34 +136,45 @@ void Pipelines::init(VkDevice device,
_drawImageDescriptorLayout = drawImageDescriptorLayout;
_drawImage = drawImage;

// Fix triangle pipeline configuration
// Triangle pipeline config
GraphicsPipeline::GraphicsPipelineConfig triangleConfig;
triangleConfig.vertexShaderPath = "./shaders/colored_triangle.vert.spv";
triangleConfig.fragmentShaderPath = "./shaders/colored_triangle.frag.spv";
triangleConfig.colorFormat = _drawImage.imageFormat;
triangleConfig.depthFormat = VK_FORMAT_UNDEFINED; // Explicitly undefined since we don't use depth
triangleConfig.depthFormat = VK_FORMAT_UNDEFINED; // No depth testing
triangleConfig.depthTest = false;
triangleConfig.cullMode = VK_CULL_MODE_NONE;
triangleConfig.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST;

// Ensure blending and depth testing setup is consistent
triangleConfig.customPipelineSetup = [](PipelineBuilder& builder) {

Copilot AI May 21, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider documenting the purpose of the customPipelineSetup callback parameter in GraphicsPipelineConfig, so future maintainers understand when and how to use it.

Copilot uses AI. Check for mistakes.
builder.disable_blending();
builder.disable_depthtest();
builder.set_multisampling_none();
builder.set_polygon_mode(VK_POLYGON_MODE_FILL);
};

trianglePipeline = std::make_unique<GraphicsPipeline>(triangleConfig);
trianglePipeline->init(device);

// Fix mesh pipeline configuration
// Mesh pipeline config
GraphicsPipeline::GraphicsPipelineConfig meshConfig;
meshConfig.vertexShaderPath = "./shaders/colored_triangle_mesh.vert.spv";
meshConfig.fragmentShaderPath = "./shaders/tex_image.frag.spv";
meshConfig.colorFormat = _drawImage.imageFormat;

// Must use a valid depth format when depth testing is enabled
// Use engine's depth format instead of VK_FORMAT_UNDEFINED
VkFormat depthFormat = VK_FORMAT_D32_SFLOAT; // Use the standard depth format
meshConfig.depthFormat = depthFormat;
meshConfig.depthFormat = VK_FORMAT_D32_SFLOAT;
meshConfig.depthTest = true;
meshConfig.depthCompareOp = VK_COMPARE_OP_GREATER;
meshConfig.cullMode = VK_CULL_MODE_NONE;
meshConfig.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST;

// Explicitly setup pipeline details via callback
meshConfig.customPipelineSetup = [](PipelineBuilder& builder) {

Copilot AI May 21, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding an inline comment within this lambda to explain the rationale behind using VK_COMPARE_OP_GREATER for depth testing in the mesh pipeline configuration.

Copilot uses AI. Check for mistakes.
builder.enable_depthtest(true, VK_COMPARE_OP_GREATER);
builder.set_multisampling_none();
builder.set_polygon_mode(VK_POLYGON_MODE_FILL);
};

VkPushConstantRange bufferRange{};
bufferRange.offset = 0;
bufferRange.size = sizeof(GPUDrawPushConstants);
Expand All @@ -175,7 +186,7 @@ void Pipelines::init(VkDevice device,
meshPipeline = std::make_unique<GraphicsPipeline>(meshConfig);
meshPipeline->init(device);

// Initialize gradient pipeline
// Compute pipeline
ComputePipeline::ComputePipelineConfig gradientConfig;
gradientConfig.descriptorSetLayout = _drawImageDescriptorLayout;
gradientConfig.shaderPath = "./shaders/gradient.comp.spv";
Expand Down
191 changes: 131 additions & 60 deletions src/graphics/vulkan/vk_pipelines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include <fmt/base.h>
#include <fstream>
#include <spdlog/spdlog.h>

#include "core/Logging.h"
#include "graphics/vulkan/vk_initializers.h"

bool vkutil::load_shader_module(const char* filePath, VkDevice device,
Expand Down Expand Up @@ -60,96 +60,152 @@ bool vkutil::load_shader_module(const char* filePath, VkDevice device,
}

void PipelineBuilder::clear() {
// clear all of the structs we need back to 0 with their correct stype

// Initialize input assembly with proper defaults
_inputAssembly = {
.sType =
VK_STRUCTURE_TYPE_PIPELINE_INPUT_ASSEMBLY_STATE_CREATE_INFO};
.sType = VK_STRUCTURE_TYPE_PIPELINE_INPUT_ASSEMBLY_STATE_CREATE_INFO,
.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST,
.primitiveRestartEnable = VK_FALSE
};

// Initialize rasterizer with proper defaults
_rasterizer = {
.sType =
VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO};

_colorBlendAttachment = {};

.sType = VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO,
.depthClampEnable = VK_FALSE,
.rasterizerDiscardEnable = VK_FALSE,
.polygonMode = VK_POLYGON_MODE_FILL,
.cullMode = VK_CULL_MODE_NONE,
.frontFace = VK_FRONT_FACE_CLOCKWISE,
.depthBiasEnable = VK_FALSE,
.depthBiasConstantFactor = 0.0f,
.depthBiasClamp = 0.0f,
.depthBiasSlopeFactor = 0.0f,
.lineWidth = 1.0f // CRITICAL: Must be 1.0f, not 0.0f
};

// Initialize color blend attachment with defaults
_colorBlendAttachment = {
.blendEnable = VK_FALSE,
.colorWriteMask = VK_COLOR_COMPONENT_R_BIT | VK_COLOR_COMPONENT_G_BIT |
VK_COLOR_COMPONENT_B_BIT | VK_COLOR_COMPONENT_A_BIT
};

// Initialize multisampling with proper defaults
_multisampling = {
.sType = VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_CREATE_INFO};
.sType = VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_CREATE_INFO,
.rasterizationSamples = VK_SAMPLE_COUNT_1_BIT, // CRITICAL: Must be valid sample count
.sampleShadingEnable = VK_FALSE,
.minSampleShading = 1.0f,
.pSampleMask = nullptr,
.alphaToCoverageEnable = VK_FALSE,
.alphaToOneEnable = VK_FALSE
};

_pipelineLayout = {};

// Initialize depth-stencil with proper defaults
_depthStencil = {
.sType =
VK_STRUCTURE_TYPE_PIPELINE_DEPTH_STENCIL_STATE_CREATE_INFO};

_renderInfo = {.sType = VK_STRUCTURE_TYPE_PIPELINE_RENDERING_CREATE_INFO};
.sType = VK_STRUCTURE_TYPE_PIPELINE_DEPTH_STENCIL_STATE_CREATE_INFO,
.depthTestEnable = VK_FALSE,
.depthWriteEnable = VK_FALSE,
.depthCompareOp = VK_COMPARE_OP_LESS,
.depthBoundsTestEnable = VK_FALSE,
.stencilTestEnable = VK_FALSE,
.minDepthBounds = 0.0f,
.maxDepthBounds = 1.0f
};

// Initialize rendering info with proper defaults
_renderInfo = {
.sType = VK_STRUCTURE_TYPE_PIPELINE_RENDERING_CREATE_INFO,
.depthAttachmentFormat = VK_FORMAT_UNDEFINED
};

_shaderStages.clear();
}

VkPipeline PipelineBuilder::build_pipeline(VkDevice device) const {
// Make local copies of struct members that we need to modify
VkPipelineRenderingCreateInfo renderInfo = _renderInfo;
VkPipelineRasterizationStateCreateInfo rasterizer = _rasterizer;
VkPipelineMultisampleStateCreateInfo multisampling = _multisampling;
VkPipelineInputAssemblyStateCreateInfo inputAssembly = _inputAssembly;

// Fix any uninitialized values
if (renderInfo.depthAttachmentFormat == 0) {
renderInfo.depthAttachmentFormat = VK_FORMAT_UNDEFINED;
}
Comment on lines +134 to +136

Copilot AI May 21, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check for == 0 is redundant since VK_FORMAT_UNDEFINED is defined as 0. This branch will never change the value. Consider removing this check or updating it to catch truly invalid formats.

Suggested change
if (renderInfo.depthAttachmentFormat == 0) {
renderInfo.depthAttachmentFormat = VK_FORMAT_UNDEFINED;
}
// Removed redundant check and assignment for depthAttachmentFormat.

Copilot uses AI. Check for mistakes.

// Ensure rasterizer line width is valid
if (rasterizer.lineWidth <= 0.0f) {
rasterizer.lineWidth = 1.0f;
}

// Ensure multisampling uses a valid sample count
if (multisampling.rasterizationSamples == 0) {
multisampling.rasterizationSamples = VK_SAMPLE_COUNT_1_BIT;
}

// Avoid POINT_LIST topology without PointSize in shader
if (inputAssembly.topology == VK_PRIMITIVE_TOPOLOGY_POINT_LIST) {
inputAssembly.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST;
}

// make viewport state from our stored viewport and scissor.
// at the moment we wont support multiple viewports or scissors
VkPipelineViewportStateCreateInfo viewportState = {};
viewportState.sType = VK_STRUCTURE_TYPE_PIPELINE_VIEWPORT_STATE_CREATE_INFO;
viewportState.pNext = nullptr;

viewportState.viewportCount = 1;
viewportState.scissorCount = 1;

// setup dummy color blending. We arent using transparent objects yet
// the blending is just "no blend", but we do write to the color attachment

// Use dynamic viewport and scissor
VkDynamicState dynamicStates[] = { VK_DYNAMIC_STATE_VIEWPORT, VK_DYNAMIC_STATE_SCISSOR };
VkPipelineDynamicStateCreateInfo dynamicState = {};
dynamicState.sType = VK_STRUCTURE_TYPE_PIPELINE_DYNAMIC_STATE_CREATE_INFO;
dynamicState.pDynamicStates = dynamicStates;
dynamicState.dynamicStateCount = 2;

// setup color blending
VkPipelineColorBlendStateCreateInfo colorBlending = {};
colorBlending.sType =
VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO;
colorBlending.sType = VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO;
colorBlending.pNext = nullptr;

colorBlending.logicOpEnable = VK_FALSE;
colorBlending.logicOp = VK_LOGIC_OP_COPY;
colorBlending.attachmentCount = 1;
colorBlending.pAttachments = &_colorBlendAttachment;

// completely clear VertexInputStateCreateInfo, as we have no need for it
constexpr VkPipelineVertexInputStateCreateInfo _vertexInputInfo = {
.sType = VK_STRUCTURE_TYPE_PIPELINE_VERTEX_INPUT_STATE_CREATE_INFO};
// completely clear VertexInputStateCreateInfo
const VkPipelineVertexInputStateCreateInfo vertexInputInfo = {
.sType = VK_STRUCTURE_TYPE_PIPELINE_VERTEX_INPUT_STATE_CREATE_INFO
};

// build the actual pipeline
// we now use all of the info structs we have been writing into into this
// one to create the pipeline
VkGraphicsPipelineCreateInfo pipelineInfo = {
.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO};
// connect the renderInfo to the pNext extension mechanism
pipelineInfo.pNext = &_renderInfo;

pipelineInfo.stageCount = (uint32_t)_shaderStages.size();
VkGraphicsPipelineCreateInfo pipelineInfo = {};
pipelineInfo.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO;
pipelineInfo.pNext = &renderInfo; // Use our local copy
pipelineInfo.stageCount = static_cast<uint32_t>(_shaderStages.size());
pipelineInfo.pStages = _shaderStages.data();
pipelineInfo.pVertexInputState = &_vertexInputInfo;
pipelineInfo.pInputAssemblyState = &_inputAssembly;
pipelineInfo.pVertexInputState = &vertexInputInfo;
pipelineInfo.pInputAssemblyState = &inputAssembly; // Use our local copy
pipelineInfo.pViewportState = &viewportState;
pipelineInfo.pRasterizationState = &_rasterizer;
pipelineInfo.pMultisampleState = &_multisampling;
pipelineInfo.pColorBlendState = &colorBlending;
pipelineInfo.pRasterizationState = &rasterizer; // Use our local copy
pipelineInfo.pMultisampleState = &multisampling; // Use our local copy
pipelineInfo.pDepthStencilState = &_depthStencil;
pipelineInfo.pColorBlendState = &colorBlending;
pipelineInfo.pDynamicState = &dynamicState;
pipelineInfo.layout = _pipelineLayout;
pipelineInfo.renderPass = VK_NULL_HANDLE; // We use dynamic rendering
pipelineInfo.subpass = 0;
pipelineInfo.basePipelineHandle = VK_NULL_HANDLE;

constexpr VkDynamicState state[] = {VK_DYNAMIC_STATE_VIEWPORT,
VK_DYNAMIC_STATE_SCISSOR};

VkPipelineDynamicStateCreateInfo dynamicInfo = {
.sType = VK_STRUCTURE_TYPE_PIPELINE_DYNAMIC_STATE_CREATE_INFO};
dynamicInfo.pDynamicStates = &state[0];
dynamicInfo.dynamicStateCount = 2;

pipelineInfo.pDynamicState = &dynamicInfo;

// its easy to error out on create graphics pipeline, so we handle it a bit
// better than the common VK_CHECK case
VkPipeline newPipeline;
if (vkCreateGraphicsPipelines(device, VK_NULL_HANDLE, 1, &pipelineInfo,
nullptr, &newPipeline) != VK_SUCCESS) {
fmt::println("failed to create pipeline");
return VK_NULL_HANDLE; // failed to create graphics pipeline
} else {
return newPipeline;

LOGE("Failed to create graphics pipeline in build_pipeline.");
return VK_NULL_HANDLE;
}

return newPipeline;
}

void PipelineBuilder::set_shaders(VkShaderModule vertexShader,
Expand All @@ -165,15 +221,19 @@ void PipelineBuilder::set_shaders(VkShaderModule vertexShader,

void PipelineBuilder::set_input_topology(VkPrimitiveTopology topology) {
_inputAssembly.topology = topology;
// we are not going to use primitive restart on the entire tutorial so leave
// it on false

// Avoid POINT_LIST topology which requires special shader setup
if (topology == VK_PRIMITIVE_TOPOLOGY_POINT_LIST) {
// Fall back to triangle list if point list is requested
_inputAssembly.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST;
}

_inputAssembly.primitiveRestartEnable = VK_FALSE;
}

void PipelineBuilder::set_polygon_mode(VkPolygonMode mode) {
_rasterizer.polygonMode = mode;
// Always set lineWidth to 1.0f to ensure compatibility
_rasterizer.lineWidth = 1.0f;
_rasterizer.lineWidth = 1.0f; // Always set to 1.0
}

void PipelineBuilder::set_cull_mode(VkCullModeFlags cullMode,
Expand All @@ -183,8 +243,9 @@ void PipelineBuilder::set_cull_mode(VkCullModeFlags cullMode,
}

void PipelineBuilder::set_multisampling_none() {
_multisampling.sType = VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_CREATE_INFO;
_multisampling.pNext = nullptr;
_multisampling.sampleShadingEnable = VK_FALSE;
// Ensure this is explicitly set to avoid zero value issues
_multisampling.rasterizationSamples = VK_SAMPLE_COUNT_1_BIT;
_multisampling.minSampleShading = 1.0f;
_multisampling.pSampleMask = nullptr;
Expand All @@ -209,6 +270,16 @@ void PipelineBuilder::set_color_attachment_format(VkFormat format) {
}

void PipelineBuilder::set_depth_format(VkFormat format) {
// Ensure format is valid or undefined
if (format != VK_FORMAT_D16_UNORM &&
format != VK_FORMAT_D32_SFLOAT &&
format != VK_FORMAT_D16_UNORM_S8_UINT &&
format != VK_FORMAT_D24_UNORM_S8_UINT &&
format != VK_FORMAT_D32_SFLOAT_S8_UINT &&
format != VK_FORMAT_UNDEFINED) {
format = VK_FORMAT_UNDEFINED;
}

_renderInfo.depthAttachmentFormat = format;
}

Expand Down