From 3b60cee5765b5e571319c483e210e252eca27dd3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 23 Aug 2019 11:40:12 -0400 Subject: [PATCH 1/4] hecl/Compilers: Make name strings constexpr Same behavior (as of C++17), without the need to duplicate the variable name. --- hecl/include/hecl/Compilers.hpp | 22 +++++++++++----------- hecl/lib/Compilers.cpp | 17 ----------------- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/hecl/include/hecl/Compilers.hpp b/hecl/include/hecl/Compilers.hpp index 6c214559e..7c714eb75 100644 --- a/hecl/include/hecl/Compilers.hpp +++ b/hecl/include/hecl/Compilers.hpp @@ -12,35 +12,35 @@ using PlatformEnum = boo::IGraphicsDataFactory::Platform; struct Null {}; struct OpenGL { static constexpr PlatformEnum Enum = PlatformEnum::OpenGL; - static const char* Name; + static constexpr char Name[] = "OpenGL"; #if BOO_HAS_GL using Context = boo::GLDataFactory::Context; #endif }; struct D3D11 { static constexpr PlatformEnum Enum = PlatformEnum::D3D11; - static const char* Name; + static constexpr char Name[] = "D3D11"; #if _WIN32 using Context = boo::D3D11DataFactory::Context; #endif }; struct Metal { static constexpr PlatformEnum Enum = PlatformEnum::Metal; - static const char* Name; + static constexpr char Name[] = "Metal"; #if BOO_HAS_METAL using Context = boo::MetalDataFactory::Context; #endif }; struct Vulkan { static constexpr PlatformEnum Enum = PlatformEnum::Vulkan; - static const char* Name; + static constexpr char Name[] = "Vulkan"; #if BOO_HAS_VULKAN using Context = boo::VulkanDataFactory::Context; #endif }; struct NX { static constexpr PlatformEnum Enum = PlatformEnum::NX; - static const char* Name; + static constexpr char Name[] = "NX"; #if BOO_HAS_NX using Context = boo::NXDataFactory::Context; #endif @@ -51,27 +51,27 @@ namespace PipelineStage { using StageEnum = boo::PipelineStage; struct Null { static constexpr StageEnum Enum = StageEnum::Null; - static const char* Name; + static constexpr char Name[] = "Null"; }; struct Vertex { static constexpr StageEnum Enum = StageEnum::Vertex; - static const char* Name; + static constexpr char Name[] = "Vertex"; }; struct Fragment { static constexpr StageEnum Enum = StageEnum::Fragment; - static const char* Name; + static constexpr char Name[] = "Fragment"; }; struct Geometry { static constexpr StageEnum Enum = StageEnum::Geometry; - static const char* Name; + static constexpr char Name[] = "Geometry"; }; struct Control { static constexpr StageEnum Enum = StageEnum::Control; - static const char* Name; + static constexpr char Name[] = "Control"; }; struct Evaluation { static constexpr StageEnum Enum = StageEnum::Evaluation; - static const char* Name; + static constexpr char Name[] = "Evaluation"; }; } // namespace PipelineStage diff --git a/hecl/lib/Compilers.cpp b/hecl/lib/Compilers.cpp index 1f29236eb..bbac68cec 100644 --- a/hecl/lib/Compilers.cpp +++ b/hecl/lib/Compilers.cpp @@ -17,23 +17,6 @@ extern pD3DCompile D3DCompilePROC; namespace hecl { logvisor::Module Log("hecl::Compilers"); -namespace PlatformType { -const char* OpenGL::Name = "OpenGL"; -const char* Vulkan::Name = "Vulkan"; -const char* D3D11::Name = "D3D11"; -const char* Metal::Name = "Metal"; -const char* NX::Name = "NX"; -} // namespace PlatformType - -namespace PipelineStage { -const char* Null::Name = "Null"; -const char* Vertex::Name = "Vertex"; -const char* Fragment::Name = "Fragment"; -const char* Geometry::Name = "Geometry"; -const char* Control::Name = "Control"; -const char* Evaluation::Name = "Evaluation"; -} // namespace PipelineStage - template struct ShaderCompiler {}; From 73cf8df4090baa91bf8299c9c69961a336ce1996 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 23 Aug 2019 11:49:53 -0400 Subject: [PATCH 2/4] hecl/Compilers: Convert printf call over to fmt::print Same behavior, but properly handles the case where the given string_view may not be null terminated. --- hecl/lib/Compilers.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hecl/lib/Compilers.cpp b/hecl/lib/Compilers.cpp index bbac68cec..f08d0b5c6 100644 --- a/hecl/lib/Compilers.cpp +++ b/hecl/lib/Compilers.cpp @@ -1,14 +1,17 @@ #include "hecl/Compilers.hpp" -#include "boo/graphicsdev/GLSLMacros.hpp" -#include "logvisor/logvisor.hpp" +#include +#include + #include #include #include #include + #if _WIN32 #include extern pD3DCompile D3DCompilePROC; #endif + #if __APPLE__ #include #include @@ -82,7 +85,7 @@ struct ShaderCompiler { ComPtr blobOut; if (FAILED(D3DCompilePROC(text.data(), text.size(), "Boo HLSL Source", nullptr, nullptr, "main", D3DShaderTypes[int(S::Enum)], BOO_D3DCOMPILE_FLAG, 0, &blobOut, &errBlob))) { - printf("%s\n", text.data()); + fmt::print(fmt("{}\n"), text); Log.report(logvisor::Fatal, fmt("error compiling shader: {}"), (char*)errBlob->GetBufferPointer()); return {}; } From d59d453db3de942a32961a12c1924b0161849425 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 23 Aug 2019 11:53:35 -0400 Subject: [PATCH 3/4] hecl/Compilers: Convert fprintf calls over to fmt::print Makes the Metal code more consistent with the other compilers. While we're at it we can also fix accidental printf leftovers within the existing fmt format strings. --- hecl/lib/Compilers.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hecl/lib/Compilers.cpp b/hecl/lib/Compilers.cpp index f08d0b5c6..7fee1b888 100644 --- a/hecl/lib/Compilers.cpp +++ b/hecl/lib/Compilers.cpp @@ -167,7 +167,7 @@ struct ShaderCompiler { "-gline-tables-only", "-MO", #endif "-", NULL); - fprintf(stderr, "execlp fail %s\n", strerror(errno)); + fmt::print(stderr, fmt("execlp fail {}\n"), strerror(errno)); exit(1); } close(compilerIn[0]); @@ -183,7 +183,7 @@ struct ShaderCompiler { /* metallib doesn't like outputting to a pipe, so temp file will have to do */ execlp("xcrun", "xcrun", "-sdk", "macosx", "metallib", "-", "-o", libFile.c_str(), NULL); - fprintf(stderr, "execlp fail %s\n", strerror(errno)); + fmt::print(stderr, fmt("execlp fail {}\n"), strerror(errno)); exit(1); } close(compilerOut[0]); @@ -194,7 +194,7 @@ struct ShaderCompiler { while (inRem) { ssize_t writeRes = write(compilerIn[1], inPtr, inRem); if (writeRes < 0) { - fprintf(stderr, "write fail %s\n", strerror(errno)); + fmt::print(stderr, fmt("write fail {}\n"), strerror(errno)); break; } inPtr += writeRes; @@ -207,7 +207,7 @@ struct ShaderCompiler { while (waitpid(compilerPid, &compilerStat, 0) < 0) { if (errno == EINTR) continue; - Log.report(logvisor::Fatal, fmt("waitpid fail %s"), strerror(errno)); + Log.report(logvisor::Fatal, fmt("waitpid fail {}"), strerror(errno)); return {}; } @@ -219,7 +219,7 @@ struct ShaderCompiler { while (waitpid(linkerPid, &linkerStat, 0) < 0) { if (errno == EINTR) continue; - Log.report(logvisor::Fatal, fmt("waitpid fail %s"), strerror(errno)); + Log.report(logvisor::Fatal, fmt("waitpid fail {}"), strerror(errno)); return {}; } From 2e16f882d201f1ffcc5d45aa7c1485f8b5501c2d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 23 Aug 2019 11:56:34 -0400 Subject: [PATCH 4/4] hecl/Compilers: Make use of nullptr over NULL Same behavior, stricter type. --- hecl/lib/Compilers.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hecl/lib/Compilers.cpp b/hecl/lib/Compilers.cpp index 7fee1b888..a3b1751f9 100644 --- a/hecl/lib/Compilers.cpp +++ b/hecl/lib/Compilers.cpp @@ -110,7 +110,7 @@ struct ShaderCompiler { pid_t pid = fork(); if (!pid) { - execlp("xcrun", "xcrun", "-sdk", "macosx", "metal", "--version", NULL); + execlp("xcrun", "xcrun", "-sdk", "macosx", "metal", "--version", nullptr); /* xcrun returns 72 if metal command not found; * emulate that if xcrun not found */ exit(72); @@ -166,7 +166,7 @@ struct ShaderCompiler { #ifndef NDEBUG "-gline-tables-only", "-MO", #endif - "-", NULL); + "-", nullptr); fmt::print(stderr, fmt("execlp fail {}\n"), strerror(errno)); exit(1); } @@ -182,7 +182,7 @@ struct ShaderCompiler { close(compilerIn[1]); /* metallib doesn't like outputting to a pipe, so temp file will have to do */ - execlp("xcrun", "xcrun", "-sdk", "macosx", "metallib", "-", "-o", libFile.c_str(), NULL); + execlp("xcrun", "xcrun", "-sdk", "macosx", "metallib", "-", "-o", libFile.c_str(), nullptr); fmt::print(stderr, fmt("execlp fail {}\n"), strerror(errno)); exit(1); }