From ac1c5f20ecfb1e1809e6e113e7488cbc4175e2e7 Mon Sep 17 00:00:00 2001 From: Metehan Tuncbilek Date: Tue, 16 Jul 2024 14:59:09 +0300 Subject: [PATCH] review fixes --- examples/main.cpp | 2 - openVulkanoCpp/Base/Utils.cpp | 18 ++++-- openVulkanoCpp/Shader/ShaderCompiler.cpp | 78 ++++++++---------------- openVulkanoCpp/Shader/ShaderCompiler.hpp | 16 +++-- openVulkanoCpp/Shader/test.vert | 8 +++ openVulkanoCpp/Shader/test1.glsl | 3 + 6 files changed, 59 insertions(+), 66 deletions(-) create mode 100644 openVulkanoCpp/Shader/test.vert create mode 100644 openVulkanoCpp/Shader/test1.glsl diff --git a/examples/main.cpp b/examples/main.cpp index c0a103b..18a8431 100644 --- a/examples/main.cpp +++ b/examples/main.cpp @@ -17,8 +17,6 @@ #include #include -#include "Shader/ShaderCompiler.hpp" - using namespace OpenVulkano; int main(int argc, char** argv) diff --git a/openVulkanoCpp/Base/Utils.cpp b/openVulkanoCpp/Base/Utils.cpp index a028fe6..cbdd6ed 100644 --- a/openVulkanoCpp/Base/Utils.cpp +++ b/openVulkanoCpp/Base/Utils.cpp @@ -7,9 +7,9 @@ #include "Utils.hpp" #ifdef _MSC_VER -#include + #include #else -#include + #include #endif #include #include @@ -57,7 +57,7 @@ namespace OpenVulkano #ifdef _MSC_VER return (uint64_t)::GetThreadId(::GetCurrentThread()); #else - return (uint64_t)pthread_self(); + return (uint64_t) pthread_self(); #endif } @@ -69,8 +69,16 @@ namespace OpenVulkano if (emptyOnMissing) return {}; throw std::runtime_error("Failed to open file '" + filePath + "'!"); } - size_t fileSize = static_cast(file.tellg()); - if (nullTerminateString) fileSize++; + const size_t fileSize = static_cast(file.tellg()); + if (nullTerminateString) + { + Array data(fileSize + 1); + file.seekg(0); + file.read(data.Data(), fileSize); + data[fileSize] = '\0'; + file.close(); + return data; + } Array data(fileSize); file.seekg(0); diff --git a/openVulkanoCpp/Shader/ShaderCompiler.cpp b/openVulkanoCpp/Shader/ShaderCompiler.cpp index 2f24c67..548986d 100644 --- a/openVulkanoCpp/Shader/ShaderCompiler.cpp +++ b/openVulkanoCpp/Shader/ShaderCompiler.cpp @@ -1,5 +1,9 @@ #include "ShaderCompiler.hpp" +#include "Base/Logger.hpp" + +#include + namespace OpenVulkano { Unique ShaderCompiler::CompileGLSLToSpirv(const std::string& absPath, const std::string& incPath, @@ -8,11 +12,6 @@ namespace OpenVulkano { Array file = Utils::ReadFile(absPath, false, true); -#if defined(_DEBUG) - printf("Compiling shader: %s\n", absPath.c_str()); - printf("%s\n", file.Data() + 1); -#endif - shaderc::Compiler shaderCompiler; shaderc::CompileOptions options; @@ -25,8 +24,7 @@ namespace OpenVulkano if (preResult.GetCompilationStatus() != shaderc_compilation_status_success) { - throw std::runtime_error("Failed to preprocess shader: " + preResult.GetErrorMessage()); - return nullptr; + throw std::runtime_error("Failed preprocessing shader. Reason: " + preResult.GetErrorMessage()); } shaderc::CompilationResult result = @@ -34,68 +32,42 @@ namespace OpenVulkano if (result.GetCompilationStatus() != shaderc_compilation_status_success) { - throw std::runtime_error("Failed to compile shader: " + result.GetErrorMessage()); - return nullptr; + throw std::runtime_error("Failed compiling shader. Reason: " + result.GetErrorMessage()); } Unique spirv = std::make_unique((void*) result.begin()); return spirv; } - ShaderIncluder::ShaderIncluder(const std::string& path) - { - // TODO : Add the path to your include folder - m_includes.push_back(path); - } + ShaderIncluder::ShaderIncluder(const std::string& path) : m_includePath(path) {} - shaderc_include_result* ShaderIncluder::GetInclude(const char* requested_source, shaderc_include_type type, - const char* requesting_source, uint64_t include_depth) + shaderc_include_result* ShaderIncluder::GetInclude(const char* requestedSource, shaderc_include_type type, + const char* requestingSource, uint64_t includeDepth) { - const char* includePath = GetActualPath(requested_source); - const char* content = RecordInclude(includePath); + IncludeData* includeData = new IncludeData(); + includeData->m_fullPath = ResolveInclude(requestedSource); + includeData->m_content = Utils::ReadFile(includeData->m_fullPath, false, false); // using nullTerminate as true causes crash in here. Needed to use as false. - shaderc_include_result* result = new shaderc_include_result(); - result->content = content; - result->content_length = strlen(content); - result->source_name = includePath; - result->source_name_length = strlen(includePath); - result->user_data = nullptr; + shaderc_include_result* result = &includeData->result; + result->content = includeData->m_content.Data(); + result->content_length = includeData->m_content.Size(); + result->source_name = includeData->m_fullPath.data(); + result->source_name_length = includeData->m_fullPath.size(); + result->user_data = includeData; return result; } - void ShaderIncluder::ReleaseInclude(shaderc_include_result* data) { delete data; } - - std::string ShaderIncluder::ResolveInclude(const std::string requestedSource) + void ShaderIncluder::ReleaseInclude(shaderc_include_result* data) { - // Check if the file exists in the include paths - for (const std::string& includePath: m_includes) - { - std::string path = includePath + requestedSource; - // TODO: Need to check if the folder exists - return path; - } - - return ""; + delete static_cast(data->user_data); } - const char* ShaderIncluder::RecordInclude(const std::string& requestedSource) + std::string ShaderIncluder::ResolveInclude(const std::string& requestedSource) const { - Array data = Utils::ReadFile(requestedSource); - - char* result = new char[data.Size() + 1]; - strcpy_s(result, data.Size(), data.Data()); - - return result; - } - - const char* ShaderIncluder::GetActualPath(const std::string& requestedSource) - { - std::string includePath = ResolveInclude(requestedSource); - - char* result = new char[includePath.size() + 1]; - strcpy_s(result, includePath.size(), includePath.c_str()); - - return result; + // Check if the file exists in the include path + std::string path = m_includePath + requestedSource; + if (std::filesystem::exists(path)) return path; + else throw std::runtime_error("Failed to resolve include '" + requestedSource + "'!"); } } diff --git a/openVulkanoCpp/Shader/ShaderCompiler.hpp b/openVulkanoCpp/Shader/ShaderCompiler.hpp index b4d083d..6eb0df0 100644 --- a/openVulkanoCpp/Shader/ShaderCompiler.hpp +++ b/openVulkanoCpp/Shader/ShaderCompiler.hpp @@ -11,21 +11,25 @@ namespace OpenVulkano { class ShaderIncluder : public shaderc::CompileOptions::IncluderInterface { + struct IncludeData + { + shaderc_include_result result = {}; + std::string m_fullPath; + Array m_content; + }; public: ShaderIncluder(const std::string& path); ~ShaderIncluder() override = default; - shaderc_include_result* GetInclude(const char* requested_source, shaderc_include_type type, - const char* requesting_source, uint64_t include_depth) override; + shaderc_include_result* GetInclude(const char* requestedSource, shaderc_include_type type, + const char* requestingSource, uint64_t includeDepth) override; void ReleaseInclude(shaderc_include_result* data) override; private: - std::string ResolveInclude(const std::string requestedSource); - const char* RecordInclude(const std::string& requestedSource); - const char* GetActualPath(const std::string& requestedSource); + std::string ResolveInclude(const std::string& requestedSource) const; private: - std::vector m_includes; + std::string m_includePath; }; class ShaderCompiler diff --git a/openVulkanoCpp/Shader/test.vert b/openVulkanoCpp/Shader/test.vert new file mode 100644 index 0000000..3d16cda --- /dev/null +++ b/openVulkanoCpp/Shader/test.vert @@ -0,0 +1,8 @@ +#version 460 +#extension GL_KHR_vulkan_glsl : enable + +#include "test1.glsl" + +void main() +{ +}; \ No newline at end of file diff --git a/openVulkanoCpp/Shader/test1.glsl b/openVulkanoCpp/Shader/test1.glsl new file mode 100644 index 0000000..92671bc --- /dev/null +++ b/openVulkanoCpp/Shader/test1.glsl @@ -0,0 +1,3 @@ +void testfunction() +{ +}; \ No newline at end of file