From e3e0a4bb2c5bf6c0e03366b5032acd615efc762d Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 14:09:00 +0100 Subject: [PATCH 01/30] Rewrite cache manager to be general purpose --- src/ammonite/ammonite.hpp | 1 - src/ammonite/core/fileManager.cpp | 191 +++++++++++ src/ammonite/core/fileManager.hpp | 28 ++ src/ammonite/enums.hpp | 7 +- src/ammonite/graphics/renderInterface.cpp | 8 +- src/ammonite/graphics/renderer.hpp | 1 + src/ammonite/graphics/shaders.cpp | 314 +++++++++++------- src/ammonite/types.hpp | 2 + src/ammonite/utils/cacheManager.cpp | 147 -------- src/ammonite/utils/cacheManager.hpp | 13 - src/ammonite/utils/fileManager.cpp | 33 -- .../utils/internal/internalCacheManager.hpp | 21 -- .../utils/internal/internalFileManager.hpp | 17 - src/demo.cpp | 2 +- 14 files changed, 422 insertions(+), 363 deletions(-) create mode 100644 src/ammonite/core/fileManager.cpp create mode 100644 src/ammonite/core/fileManager.hpp delete mode 100644 src/ammonite/utils/cacheManager.cpp delete mode 100644 src/ammonite/utils/cacheManager.hpp delete mode 100644 src/ammonite/utils/fileManager.cpp delete mode 100644 src/ammonite/utils/internal/internalCacheManager.hpp delete mode 100644 src/ammonite/utils/internal/internalFileManager.hpp diff --git a/src/ammonite/ammonite.hpp b/src/ammonite/ammonite.hpp index 29e22fa1..6498bf65 100644 --- a/src/ammonite/ammonite.hpp +++ b/src/ammonite/ammonite.hpp @@ -9,7 +9,6 @@ #include "models/modelInterface.hpp" #include "models/modelStorage.hpp" -#include "utils/cacheManager.hpp" #include "utils/controls.hpp" #include "utils/debug.hpp" #include "utils/logging.hpp" diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp new file mode 100644 index 00000000..92dc854c --- /dev/null +++ b/src/ammonite/core/fileManager.cpp @@ -0,0 +1,191 @@ +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "../enums.hpp" +#include "../types.hpp" + +#include "../utils/debug.hpp" +#include "../utils/logging.hpp" + +namespace ammonite { + namespace files { + namespace internal { + namespace { + //Whether or not this manager is ready for cache use + bool cacheEnabled = false; + std::string dataCachePath = std::string(""); + } + + namespace { + //Hash input filenames together to create a unique cache string + static std::string generateCacheString(std::string* filePaths, + unsigned int fileCount) { + std::string inputString = std::string(""); + for (unsigned int i = 0; i < fileCount; i++) { + inputString += filePaths[i] + std::string(";"); + } + + return std::to_string(std::hash{}(inputString)); + } + } + + void deleteFile(std::string filePath) { + if (std::filesystem::exists(filePath)) { + std::remove(filePath.c_str()); + } + } + + bool getFileMetadata(std::string filePath, long long int* filesize, + long long int* timestamp) { + //Give up if the file doesn't exist + std::filesystem::path filesystemPath = std::string(filePath); + if (!std::filesystem::exists(filesystemPath)) { + return false; + } + + //Get a time point for last write time of the file + std::filesystem::file_time_type fileTimestamp = + std::filesystem::last_write_time(filesystemPath); + + //Convert time point to unix time + *timestamp = + std::chrono::system_clock::to_time_t(std::chrono::file_clock::to_sys(fileTimestamp)); + + //Get filesize of the file + *filesize = std::filesystem::file_size(filesystemPath); + return true; + } + + //Attempt to setup targetCachePath for caching, and return whether it can be used + bool useDataCache(std::string targetCachePath) { + //Attempt to create the cache directory if it doesn't already exist + if (!std::filesystem::is_directory(targetCachePath)) { + ammonite::utils::warning << "Couldn't find cache directory '" << targetCachePath \ + << "', creating it instead" << std::endl; + try { + std::filesystem::create_directory(targetCachePath); + } catch (const std::filesystem::filesystem_error&) { + ammonite::utils::warning << "Failed to create cache directory '" \ + << targetCachePath << "'" << std::endl; + cacheEnabled = false; + dataCachePath = std::string(""); + return false; + } + } + + //Ensure path has a trailing slash + dataCachePath = targetCachePath; + if (dataCachePath.back() != '/') { + dataCachePath.push_back('/'); + } + + ammonite::utils::status << "Data caching enabled ('" << dataCachePath << "')" << std::endl; + cacheEnabled = true; + return true; + } + + //Return whether or not this manager is ready for cache use + bool getCacheEnabled() { + return cacheEnabled; + } + + std::string getCachedFilePath(std::string* filePaths, unsigned int fileCount) { + return dataCachePath + generateCacheString(filePaths, fileCount) + std::string(".cache"); + } + + /* + - Read in filePath, then return the data and write to size + - Returned data must be freed + - If the read fails, return null + - In this case, nothing needs to be freed + */ + unsigned char* loadFile(std::string filePath, std::size_t* size) { + unsigned char* data = nullptr; + int descriptor = open(filePath.c_str(), O_RDONLY); + if (descriptor == -1) { + ammonite::utils::warning << "Error while opening '" << filePath \ + << "' (" << errno << ")" << std::endl; + } + posix_fadvise(descriptor, 0, 0, POSIX_FADV_SEQUENTIAL); + + struct stat statBuf; + fstat(descriptor, &statBuf); + *size = statBuf.st_size; + data = new unsigned char[statBuf.st_size]; + + off_t bytesRead = 0; + while (bytesRead < statBuf.st_size) { + off_t newBytesRead = read(descriptor, data + bytesRead, + statBuf.st_size - bytesRead); + if (newBytesRead == 0) { + break; + } else if (newBytesRead < 0) { + ammonite::utils::warning << "Error while reading '" << filePath \ + << "' (" << errno << ")" << std::endl; + close(descriptor); + delete [] data; + return nullptr; + } + bytesRead += newBytesRead; + } + + if (bytesRead != statBuf.st_size) { + ammonite::utils::warning << "Unexpected file size while reading '" << filePath \ + << "'" << std::endl; + close(descriptor); + delete [] data; + return nullptr; + } + + close(descriptor); + return data; + } + + /* + - Attempt to read a cached file, and validate it with validator + - If the cache was valid, write to size and cacheState, then return cacheData + - In this case, cacheData needs to be freed after use + - If the cache was invalid write to cacheState and return nullptr + - In this case, cacheData and size should be ignored, and the cache will be cleared + */ + unsigned char* getCachedFile(std::string cacheFilePath, AmmoniteValidator validator, + std::size_t* size, AmmoniteEnum* cacheState, void* userPtr) { + //Check cache file exists + if (!std::filesystem::exists(cacheFilePath)) { + ammoniteInternalDebug << "Couldn't find " << cacheFilePath << std::endl; + *cacheState = AMMONITE_CACHE_MISS; + return nullptr; + } + + //Attempt to read the cache if it exists, writes to size + unsigned char* cacheData = loadFile(cacheFilePath, size); + if (cacheData == nullptr) { + ammonite::utils::warning << "Failed to read '" << cacheFilePath << "'" << std::endl; + *cacheState = AMMONITE_CACHE_MISS; + return nullptr; + } + + //Validate the loaded cache + if (!validator(cacheData, *size, userPtr)) { + ammonite::utils::warning << "Failed to validate '" << cacheFilePath << "'" << std::endl; + ammonite::utils::status << "Clearing '" << cacheFilePath << "'" << std::endl; + deleteFile(cacheFilePath); + + delete [] cacheData; + *cacheState = AMMONITE_CACHE_INVALID; + return nullptr; + } + + *cacheState = AMMONITE_CACHE_HIT; + return cacheData; + } + } + } +} diff --git a/src/ammonite/core/fileManager.hpp b/src/ammonite/core/fileManager.hpp new file mode 100644 index 00000000..05e69fbc --- /dev/null +++ b/src/ammonite/core/fileManager.hpp @@ -0,0 +1,28 @@ +#ifndef FILEMANAGER +#define FILEMANAGER + +#include +#include + +#include "../enums.hpp" +#include "../types.hpp" + +namespace ammonite { + namespace files { + namespace internal { + void deleteFile(std::string filePath); + bool getFileMetadata(std::string filePath, long long int* filesize, long long int* timestamp); + + bool useDataCache(std::string dataCachePath); + bool getCacheEnabled(); + std::string generateCachePath(std::string* filePaths, unsigned int fileCount); + std::string getCachedFilePath(std::string* filePaths, unsigned int fileCount); + + unsigned char* loadFile(std::string filePath, std::size_t* size); + unsigned char* getCachedFile(std::string cacheFilePath, AmmoniteValidator validator, + std::size_t* size, AmmoniteEnum* cacheState, void* userPtr); + } + } +} + +#endif diff --git a/src/ammonite/enums.hpp b/src/ammonite/enums.hpp index 86f95ba3..227f3fe9 100644 --- a/src/ammonite/enums.hpp +++ b/src/ammonite/enums.hpp @@ -26,7 +26,12 @@ enum AmmoniteEnum : unsigned char { AMMONITE_ALLOW_OVERRIDE, AMMONITE_ALLOW_RELEASE, AMMONITE_FORCE_RELEASE, - AMMONITE_RESPECT_BLOCK + AMMONITE_RESPECT_BLOCK, + + //Cache return values + AMMONITE_CACHE_HIT, + AMMONITE_CACHE_MISS, + AMMONITE_CACHE_INVALID }; #endif diff --git a/src/ammonite/graphics/renderInterface.cpp b/src/ammonite/graphics/renderInterface.cpp index 9c3c821c..03158eaa 100644 --- a/src/ammonite/graphics/renderInterface.cpp +++ b/src/ammonite/graphics/renderInterface.cpp @@ -2,10 +2,12 @@ #include +#include "../core/threadManager.hpp" +#include "../core/fileManager.hpp" + #include "../utils/debug.hpp" #include "../utils/timer.hpp" #include "../utils/logging.hpp" -#include "../core/threadManager.hpp" #include "internal/internalRenderCore.hpp" #include "../internal/interfaceTracker.hpp" @@ -64,6 +66,10 @@ namespace ammonite { internal::destroyOpenGLObjects(); internal::deleteModelCache(); } + + bool useShaderCache(const char* shaderCachePath) { + return ammonite::files::internal::useDataCache(std::string(shaderCachePath)); + } } long long int getTotalFrames() { diff --git a/src/ammonite/graphics/renderer.hpp b/src/ammonite/graphics/renderer.hpp index fba7f45e..b59b917b 100644 --- a/src/ammonite/graphics/renderer.hpp +++ b/src/ammonite/graphics/renderer.hpp @@ -6,6 +6,7 @@ namespace ammonite { namespace setup { void setupRenderer(const char* shaderPath, bool* externalSuccess); void destroyRenderer(); + bool useShaderCache(const char* shaderCachePath); } long long getTotalFrames(); diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index f5633223..b7f43bf4 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -1,35 +1,36 @@ -#include -#include -#include #include -#include +#include +#include #include +#include +#include #include -#include +#include +#include -#include +//Used by shader cache validator +#include +#include +#include -#include "internal/internalExtensions.hpp" -#include "../utils/internal/internalFileManager.hpp" -#include "../utils/internal/internalCacheManager.hpp" +#include +#include "../core/fileManager.hpp" #include "../utils/logging.hpp" -#include "../utils/cacheManager.hpp" +#include "internal/internalExtensions.hpp" namespace ammonite { //Static helper functions namespace { static void deleteCacheFile(std::string cacheFilePath) { - //Delete the cache and cacheinfo files + //Delete the cache file ammonite::utils::status << "Clearing '" << cacheFilePath << "'" << std::endl; - - ammonite::utils::internal::deleteFile(cacheFilePath); + ammonite::files::internal::deleteFile(cacheFilePath); } - static void cacheProgram(const GLuint programId, - const char* shaderPaths[], const int shaderCount) { - std::string cacheFilePath = - ammonite::utils::cache::internal::requestNewCachePath(shaderPaths, shaderCount); + static void cacheProgram(const GLuint programId, std::string* shaderPaths, int shaderCount) { + std::string cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, + shaderCount); ammonite::utils::status << "Caching '" << cacheFilePath << "'" << std::endl; int binaryLength; @@ -50,7 +51,7 @@ namespace ammonite { if (cacheFile.is_open()) { for (int i = 0; i < shaderCount; i++) { long long int filesize = 0, modificationTime = 0; - ammonite::utils::internal::getFileMetadata(shaderPaths[i], &filesize, &modificationTime); + ammonite::files::internal::getFileMetadata(shaderPaths[i], &filesize, &modificationTime); cacheFile << "input;" << shaderPaths[i] << ";" << filesize << ";" \ << modificationTime << "\n"; @@ -96,9 +97,7 @@ namespace ammonite { return false; } - static GLenum attemptIdentifyShaderType(const char* shaderPath) { - std::string shaderPathString = std::string(shaderPath); - + static GLenum attemptIdentifyShaderType(std::string shaderPath) { std::map shaderMatches = { {"vert", GL_VERTEX_SHADER}, {"frag", GL_FRAGMENT_SHADER}, @@ -109,8 +108,8 @@ namespace ammonite { }; std::string lowerShaderPath; - for (unsigned int i = 0; i < shaderPathString.size(); i++) { - lowerShaderPath += std::tolower(shaderPathString[i]); + for (unsigned int i = 0; i < shaderPath.size(); i++) { + lowerShaderPath += std::tolower(shaderPath[i]); } //Try and match the filename to a supported shader @@ -123,14 +122,21 @@ namespace ammonite { return GL_FALSE; } + struct CacheInfo { + int fileCount; + std::string* filePaths; + GLenum binaryFormat; + GLsizei binaryLength; + }; + //Set by updateCacheSupport(), when GLEW loads bool isBinaryCacheSupported = false; } //Shader compilation and cache functions, local to this file - namespace shaders { + namespace { //Take shader source code, compile it and load it - static int loadShader(const char* shaderPath, const GLenum shaderType, bool* externalSuccess) { + static int loadShader(std::string shaderPath, const GLenum shaderType, bool* externalSuccess) { //Create the shader GLuint shaderId = glCreateShader(shaderType); @@ -181,8 +187,8 @@ namespace ammonite { } //Take multiple shader files, hand off to loadShader and create a program - static int createProgramObject(GLuint shaderIds[], - const int shaderCount, bool* externalSuccess) { + static int createProgramObject(GLuint shaderIds[], const int shaderCount, + bool* externalSuccess) { //Create the program GLuint programId = glCreateProgram(); @@ -208,81 +214,105 @@ namespace ammonite { return programId; } - //Attempt to find cached program or hand off to loadShader and createProgramObject - static int createProgramCached(const char* shaderPaths[], const GLenum shaderTypes[], - const int shaderCount, bool* externalSuccess) { - //Used later as the return value - GLuint programId; + bool validateCache(unsigned char* data, std::size_t size, void* userPtr) { + CacheInfo* cacheInfoPtr = (CacheInfo*)userPtr; + unsigned char* dataCopy = new unsigned char[size]; + std::memcpy(dataCopy, data, size); + dataCopy[size - 1] = '\0'; + + /* + - Decide whether the cache file can be used + - Uses input files, sizes and timestamps + */ + char* state; + int fileCount = 1; + char* token = strtok_r((char*)dataCopy, "\n", &state); + do { + //Give up if token is null, we didn't find enough files + if (token == nullptr) { + delete [] dataCopy; + return false; + } - //Check for OpenGL and engine cache support - const bool isCacheSupported = isBinaryCacheSupported and - ammonite::utils::cache::getCacheEnabled(); + //Check first token is 'input' + std::string currentFilePath = cacheInfoPtr->filePaths[fileCount - 1]; + char* nestedToken = strtok(token, ";"); + if ((nestedToken == nullptr) || + (std::string(nestedToken) != std::string("input"))) { + delete [] dataCopy; + return false; + } - if (isCacheSupported) { - bool cacheValid = false; - std::string cacheFilePath = - ammonite::utils::cache::internal::requestCachedDataPath(shaderPaths, - shaderCount, - &cacheValid); - - //Attempt to get the shader format and size - GLenum cachedBinaryFormat = 0; - GLsizei cachedBinaryLength = 0; - - std::ifstream cacheFile(cacheFilePath, std::ios::binary); - if (cacheValid) { - std::string line; - if (cacheFile.is_open()) { - try { - //Skip input files, as they're handled by the cache manager - for (int i = 0; i < shaderCount; i++) { - getline(cacheFile, line); - } + //Check token matches shader path + nestedToken = strtok(nullptr, ";"); + if ((nestedToken == nullptr) || + (std::string(nestedToken) != currentFilePath)) { + delete [] dataCopy; + return false; + } - //Get the binary format - getline(cacheFile, line); - cachedBinaryFormat = std::stoi(line); - - //Get the length of the binary - getline(cacheFile, line); - cachedBinaryLength = std::stoi(line); - } catch (const std::out_of_range&) { - cacheValid = false; - } catch (const std::invalid_argument&) { - cacheValid = false; - } - } + //Get filesize and time of last modification of the shader source + long long int filesize = 0, modificationTime = 0; + if (!ammonite::files::internal::getFileMetadata(currentFilePath, &filesize, + &modificationTime)) { + //Failed to get the metadata + delete [] dataCopy; + return false; } - //If cache is still valid, attempt to use it - if (cacheValid) { - //Read the cached data from file - char* cachedBinaryData = new char[cachedBinaryLength]; - cacheFile.read(cachedBinaryData, cachedBinaryLength); + //Check token matches file size + nestedToken = strtok(nullptr, ";"); + if ((nestedToken == nullptr) || + (std::atoll(nestedToken) != filesize)) { + delete [] dataCopy; + return false; + } - //Load the cached binary data - programId = glCreateProgram(); - glProgramBinary(programId, cachedBinaryFormat, - cachedBinaryData, cachedBinaryLength); - delete [] cachedBinaryData; + //Check token matches timestamp + nestedToken = strtok(nullptr, ";"); + if ((nestedToken == nullptr) || + (std::atoll(nestedToken) != modificationTime)) { + delete [] dataCopy; + return false; + } - //Return the program ID, unless the cache was faulty, then delete and carry on - if (checkProgram(programId)) { - return programId; - } else { - ammonite::utils::warning << "Failed to process '" << cacheFilePath << "'" << std::endl; - glDeleteProgram(programId); - deleteCacheFile(cacheFilePath); - } - } else { - //Shader source doesn't match cache, delete the old cache - if (cacheFilePath != "") { - deleteCacheFile(cacheFilePath); - } + //Get the next line + if (cacheInfoPtr->fileCount > fileCount) { + token = strtok_r(nullptr, "\n", &state); } - cacheFile.close(); + fileCount += 1; + } while (cacheInfoPtr->fileCount >= fileCount); + + //Find binary format + token = strtok_r(nullptr, "\n", &state); + if (token != nullptr) { + cacheInfoPtr->binaryFormat = std::atoll(token); + } else { + delete [] dataCopy; + return false; + } + + //Find binary length + token = strtok_r(nullptr, "\n", &state); + if (token != nullptr) { + cacheInfoPtr->binaryLength = std::atoll(token); + } else { + delete [] dataCopy; + return false; } + if (cacheInfoPtr->binaryFormat == 0 || cacheInfoPtr->binaryLength == 0) { + delete [] dataCopy; + return false; + } + + delete [] dataCopy; + return true; + } + + //Create a program from shader source with loadShader() and createProgramObject() + static int createProgramUncached(std::string* shaderPaths, GLenum* shaderTypes, + int shaderCount, bool* externalSuccess) { //Since cache wasn't available, generate fresh shaders GLuint* shaderIds = new GLuint[shaderCount]; bool hasCreatedShaders = true; @@ -294,14 +324,15 @@ namespace ammonite { } } - //Create the program like normal, as a valid cache wasn't found + //Create the program from the shaders + GLuint programId; bool hasCreatedProgram = false; if (hasCreatedShaders) { hasCreatedProgram = true; programId = createProgramObject(shaderIds, shaderCount, &hasCreatedProgram); } - //Cleanup on failure + //Clean up if (!hasCreatedProgram || !hasCreatedShaders) { *externalSuccess = false; for (int i = 0; i < shaderCount; i++) { @@ -314,8 +345,56 @@ namespace ammonite { } delete [] shaderIds; - //Cache the binary if enabled + return programId; + } + + + //Attempt to find cached program or hand off to createProgramUncached() + static int createProgramCached(std::string* shaderPaths, GLenum* shaderTypes, + int shaderCount, bool* externalSuccess) { + //Check for OpenGL and engine cache support + bool isCacheSupported = ammonite::files::internal::getCacheEnabled(); + isCacheSupported = isCacheSupported && isBinaryCacheSupported; + + //Try and fetch the cache, then try and load it into a program + int programId; if (isCacheSupported) { + std::size_t cacheDataSize; + AmmoniteEnum cacheState; + + //Attempt to load the cached program + CacheInfo cacheInfo; + cacheInfo.fileCount = shaderCount; + cacheInfo.filePaths = shaderPaths; + std::string cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, + shaderCount); + unsigned char* cacheData = ammonite::files::internal::getCachedFile(cacheFilePath, + validateCache, &cacheDataSize, &cacheState, &cacheInfo); + unsigned char* dataStart = cacheData + cacheDataSize - cacheInfo.binaryLength; + + if (cacheState == AMMONITE_CACHE_HIT) { + //Load the cached binary data into a program + programId = glCreateProgram(); + glProgramBinary(programId, cacheInfo.binaryFormat, dataStart, cacheInfo.binaryLength); + delete [] cacheData; + + //Return the program ID, unless the cache was faulty, then delete and carry on + if (checkProgram(programId)) { + return programId; + } else { + ammonite::utils::warning << "Failed to process '" << cacheFilePath \ + << "'" << std::endl; + glDeleteProgram(programId); + deleteCacheFile(cacheFilePath); + } + } + } + + //Cache wasn't useable, compile a fresh program + programId = createProgramUncached(shaderPaths, shaderTypes, shaderCount, externalSuccess); + + //Cache the binary if enabled + if (isCacheSupported && programId != -1) { cacheProgram(programId, shaderPaths, shaderCount); } @@ -345,8 +424,8 @@ namespace ammonite { } //Find shader types and hand off to createProgramCached(paths, types) - int createProgram(const char* inputShaderPaths[], - const int inputShaderCount, bool* externalSuccess) { + int createProgram(std::string* inputShaderPaths, int inputShaderCount, + bool* externalSuccess) { //Convert file extensions to shader types std::map shaderExtensions = { {".vert", GL_VERTEX_SHADER}, {".vs", GL_VERTEX_SHADER}, @@ -359,8 +438,8 @@ namespace ammonite { }; //Find all shaders - std::vector shaders(0); - std::vector types(0); + std::vector shaderPaths(0); + std::vector shaderTypes(0); for (int i = 0; i < inputShaderCount; i++) { std::filesystem::path filePath{inputShaderPaths[i]}; std::string extension = filePath.extension(); @@ -399,30 +478,18 @@ namespace ammonite { } } - shaders.push_back(std::string(filePath)); - types.push_back(shaderType); + shaderPaths.push_back(std::string(filePath)); + shaderTypes.push_back(shaderType); } } - //Repack shaders - const int shaderCount = shaders.size(); - const char** shaderPaths = new const char*[shaderCount]; - GLenum* shaderTypes = new GLenum[shaderCount]; - - for (unsigned int i = 0; i < shaders.size(); i++) { - shaderPaths[i] = shaders[i].c_str(); - shaderTypes[i] = types[i]; - } - //Create the program and return the ID - int programId = createProgramCached(shaderPaths, shaderTypes, - shaderCount, externalSuccess); - delete [] shaderTypes; - delete [] shaderPaths; + int programId = createProgramCached(&shaderPaths[0], &shaderTypes[0], + shaderPaths.size(), externalSuccess); return programId; } - //Load all shaders in a directory and hand off to createProgram(paths) + //Load all shaders in a directory and hand off to createProgram() int loadDirectory(const char* directoryPath, bool* externalSuccess) { //Create filesystem directory iterator std::filesystem::directory_iterator it; @@ -436,23 +503,14 @@ namespace ammonite { } //Find files to send to next stage - std::vector shaders(0); + std::vector shaderPaths(0); for (auto const& fileName : it) { std::filesystem::path filePath{fileName}; - shaders.push_back(std::string(filePath)); - } - - //Repack shaders - const int shaderCount = shaders.size(); - const char** shaderPaths = new const char*[shaderCount]; - - for (unsigned int i = 0; i < shaders.size(); i++) { - shaderPaths[i] = shaders[i].c_str(); + shaderPaths.push_back(std::string(filePath)); } //Create the program and return the ID - int programId = createProgram(shaderPaths, shaderCount, externalSuccess); - delete [] shaderPaths; + int programId = createProgram(&shaderPaths[0], shaderPaths.size(), externalSuccess); return programId; } } diff --git a/src/ammonite/types.hpp b/src/ammonite/types.hpp index 8c293dfc..26a51cc9 100644 --- a/src/ammonite/types.hpp +++ b/src/ammonite/types.hpp @@ -1,8 +1,10 @@ #ifndef TYPES #define TYPES +#include #include +typedef bool (*AmmoniteValidator)(unsigned char* data, std::size_t size, void* userPtr); typedef void (*AmmoniteKeyCallback)(std::vector keycodes, int action, void* userPtr); typedef void (*AmmoniteWork)(void* userPtr); diff --git a/src/ammonite/utils/cacheManager.cpp b/src/ammonite/utils/cacheManager.cpp deleted file mode 100644 index b7530e04..00000000 --- a/src/ammonite/utils/cacheManager.cpp +++ /dev/null @@ -1,147 +0,0 @@ -#include -#include -#include -#include -#include -#include - -#include "internal/internalFileManager.hpp" -#include "../utils/logging.hpp" - -namespace ammonite { - namespace utils { - namespace cache { - namespace { - bool cacheData = false; - std::string dataCacheDir; - } - - namespace { - //Hash input filenames together to create a unique cache string - static std::string generateCacheString(const char* inputNames[], - const int inputCount) { - std::string inputString = ""; - for (int i = 0; i < inputCount; i++) { - inputString += std::string(inputNames[i]) + std::string(";"); - } - - return std::to_string(std::hash{}(inputString)); - } - } - - namespace internal { - std::string requestNewCachePath(const char* filePaths[], const int fileCount) { - return std::string(dataCacheDir + generateCacheString(filePaths, fileCount) + ".cache"); - } - - std::string requestCachedDataPath(const char* filePaths[], - const int fileCount, bool* found) { - //Generate path to cache file from cache string - std::string cacheFilePath = requestNewCachePath(filePaths, fileCount); - - //Check cache file exists - if (!std::filesystem::exists(cacheFilePath)) { - *found = false; - return std::string(""); - } - - //Validate filesizes and timestamps for each input file - bool isCacheValid = true; - std::string line; - std::ifstream cacheFile(cacheFilePath); - if (cacheFile.is_open()) { - for (int i = 0; i < fileCount; i++) { - //Get expected filename, filesize and timestamp - std::vector strings; - getline(cacheFile, line); - std::stringstream rawLine(line); - - while (getline(rawLine, line, ';')) { - strings.push_back(line); - } - - if (strings.size() == 4) { - if (strings[0] != "input" or strings[1] != filePaths[i]) { - //Cache made from different files, invalidate - isCacheValid = false; - break; - } - - //Get filesize and time of last modification of the shader source - long long int filesize = 0, modificationTime = 0; - if (!ammonite::utils::internal::getFileMetadata(filePaths[i], - &filesize, &modificationTime)) { - //Failed to get the metadata - isCacheValid = false; - break; - } - - try { - if (std::stoi(strings[2]) != filesize or - std::stoi(strings[3]) != modificationTime) { - //Shader source code has changed, invalidate - isCacheValid = false; - break; - } - } catch (const std::out_of_range&) { - isCacheValid = false; - break; - } - } else { - //Cache info file broken, invalidate - isCacheValid = false; - break; - } - } - - cacheFile.close(); - } else { - //Failed to open the cache info - isCacheValid = false; - } - - //If cache failed to validate, set found and return nothing - if (!isCacheValid) { - *found = false; - return std::string(""); - } - - *found = true; - return cacheFilePath; - } - } - - bool useDataCache(const char* dataCachePath) { - //Attempt to create a cache directory - try { - std::filesystem::create_directory(dataCachePath); - } catch (const std::filesystem::filesystem_error&) { - ammonite::utils::warning << "Failed to create cache directory: '" << dataCachePath << "'" << std::endl; - cacheData = false; - return false; - } - - //If the cache directory doesn't exist, disable caching and exit - if (!std::filesystem::is_directory(dataCachePath)) { - ammonite::utils::warning << "Couldn't find cache directory: '" << dataCachePath << "'" << std::endl; - cacheData = false; - return false; - } - - //Enable cache and ensure path has a trailing slash - cacheData = true; - dataCacheDir = std::string(dataCachePath); - if (dataCacheDir.back() != '/') { - dataCacheDir.push_back('/'); - } - - ammonite::utils::status << "Data caching enabled ('" << dataCacheDir << "')" << std::endl; - return true; - } - - bool getCacheEnabled() { - return cacheData; - } - } - } -} diff --git a/src/ammonite/utils/cacheManager.hpp b/src/ammonite/utils/cacheManager.hpp deleted file mode 100644 index a7b95646..00000000 --- a/src/ammonite/utils/cacheManager.hpp +++ /dev/null @@ -1,13 +0,0 @@ -#ifndef CACHE -#define CACHE - -namespace ammonite { - namespace utils { - namespace cache { - bool useDataCache(const char* dataCachePath); - bool getCacheEnabled(); - } - } -} - -#endif diff --git a/src/ammonite/utils/fileManager.cpp b/src/ammonite/utils/fileManager.cpp deleted file mode 100644 index 2386d21c..00000000 --- a/src/ammonite/utils/fileManager.cpp +++ /dev/null @@ -1,33 +0,0 @@ -#include -#include -#include - -namespace ammonite { - namespace utils { - namespace internal { - void deleteFile(std::string filePath) { - if (std::filesystem::exists(filePath)) { - std::remove(filePath.c_str()); - } - } - - bool getFileMetadata(const char* filePath, long long int* filesize, long long int* timestamp) { - //Give up if the file doesn't exist - std::filesystem::path filesystemPath = std::string(filePath); - if (!std::filesystem::exists(filesystemPath)) { - return false; - } - - //Get a time point for last write time of the file - std::filesystem::file_time_type fileTimestamp = std::filesystem::last_write_time(filesystemPath); - - //Convert time point to unix time - *timestamp = std::chrono::system_clock::to_time_t(std::chrono::file_clock::to_sys(fileTimestamp)); - - //Get filesize of the file - *filesize = std::filesystem::file_size(filesystemPath); - return true; - } - } - } -} diff --git a/src/ammonite/utils/internal/internalCacheManager.hpp b/src/ammonite/utils/internal/internalCacheManager.hpp deleted file mode 100644 index bfdb1dbb..00000000 --- a/src/ammonite/utils/internal/internalCacheManager.hpp +++ /dev/null @@ -1,21 +0,0 @@ -#ifndef INTERNALCACHE -#define INTERNALCACHE - -/* Internally exposed header: - - Expose cache string requesting internally -*/ - -#include - -namespace ammonite { - namespace utils { - namespace cache { - namespace internal { - std::string requestNewCachePath(const char* filePaths[], const int fileCount); - std::string requestCachedDataPath(const char* filePaths[], const int fileCount, bool* found); - } - } - } -} - -#endif diff --git a/src/ammonite/utils/internal/internalFileManager.hpp b/src/ammonite/utils/internal/internalFileManager.hpp deleted file mode 100644 index e9811db3..00000000 --- a/src/ammonite/utils/internal/internalFileManager.hpp +++ /dev/null @@ -1,17 +0,0 @@ -#ifndef INTERNALFILES -#define INTERNALFILES - -/* Internally exposed header: - - Expose some file handling methods internally -*/ - -namespace ammonite { - namespace utils { - namespace internal { - void deleteFile(std::string filePath); - bool getFileMetadata(const char* filePath, long long int* filesize, long long int* timestamp); - } - } -} - -#endif diff --git a/src/demo.cpp b/src/demo.cpp index f27c648e..28806952 100644 --- a/src/demo.cpp +++ b/src/demo.cpp @@ -230,7 +230,7 @@ int main(int argc, char* argv[]) { //Enable engine caching, setup renderer and initialise controls bool success = true; - ammonite::utils::cache::useDataCache("cache"); + ammonite::renderer::setup::useShaderCache("cache"); ammonite::renderer::setup::setupRenderer("shaders/", &success); setupBits |= HAS_SETUP_RENDERER; From 0b160b32520f94c680090d97c768a3d9fd212332 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 15:33:53 +0100 Subject: [PATCH 02/30] Move shader cache validator into its own file --- .../internal/internalShaderValidator.hpp | 28 +++++ src/ammonite/graphics/shaderValidator.cpp | 110 +++++++++++++++++ src/ammonite/graphics/shaders.cpp | 113 +----------------- 3 files changed, 141 insertions(+), 110 deletions(-) create mode 100644 src/ammonite/graphics/internal/internalShaderValidator.hpp create mode 100644 src/ammonite/graphics/shaderValidator.cpp diff --git a/src/ammonite/graphics/internal/internalShaderValidator.hpp b/src/ammonite/graphics/internal/internalShaderValidator.hpp new file mode 100644 index 00000000..d451d78d --- /dev/null +++ b/src/ammonite/graphics/internal/internalShaderValidator.hpp @@ -0,0 +1,28 @@ +#ifndef INTERNALSHADERVALIDATOR +#define INTERNALSHADERVALIDATOR + +/* Internally exposed header: + - Allow shader caches to be validated +*/ + +#include +#include + +#include + +namespace ammonite { + namespace shaders { + namespace internal { + struct CacheInfo { + int fileCount; + std::string* filePaths; + GLenum binaryFormat; + GLsizei binaryLength; + }; + + bool validateCache(unsigned char* data, std::size_t size, void* userPtr); + } + } +} + +#endif diff --git a/src/ammonite/graphics/shaderValidator.cpp b/src/ammonite/graphics/shaderValidator.cpp new file mode 100644 index 00000000..59723189 --- /dev/null +++ b/src/ammonite/graphics/shaderValidator.cpp @@ -0,0 +1,110 @@ +#include +#include +#include +#include +#include + +#include "../core/fileManager.hpp" +#include "internal/internalShaderValidator.hpp" + +namespace ammonite { + namespace shaders { + namespace internal { + bool validateCache(unsigned char* data, std::size_t size, void* userPtr) { + CacheInfo* cacheInfoPtr = (CacheInfo*)userPtr; + unsigned char* dataCopy = new unsigned char[size]; + std::memcpy(dataCopy, data, size); + dataCopy[size - 1] = '\0'; + + /* + - Decide whether the cache file can be used + - Uses input files, sizes and timestamps + */ + char* state; + int fileCount = 1; + char* token = strtok_r((char*)dataCopy, "\n", &state); + do { + //Give up if token is null, we didn't find enough files + if (token == nullptr) { + delete [] dataCopy; + return false; + } + + //Check first token is 'input' + std::string currentFilePath = cacheInfoPtr->filePaths[fileCount - 1]; + char* nestedToken = strtok(token, ";"); + if ((nestedToken == nullptr) || + (std::string(nestedToken) != std::string("input"))) { + delete [] dataCopy; + return false; + } + + //Check token matches shader path + nestedToken = strtok(nullptr, ";"); + if ((nestedToken == nullptr) || + (std::string(nestedToken) != currentFilePath)) { + delete [] dataCopy; + return false; + } + + //Get filesize and time of last modification of the shader source + long long int filesize = 0, modificationTime = 0; + if (!ammonite::files::internal::getFileMetadata(currentFilePath, &filesize, + &modificationTime)) { + //Failed to get the metadata + delete [] dataCopy; + return false; + } + + //Check token matches file size + nestedToken = strtok(nullptr, ";"); + if ((nestedToken == nullptr) || + (std::atoll(nestedToken) != filesize)) { + delete [] dataCopy; + return false; + } + + //Check token matches timestamp + nestedToken = strtok(nullptr, ";"); + if ((nestedToken == nullptr) || + (std::atoll(nestedToken) != modificationTime)) { + delete [] dataCopy; + return false; + } + + //Get the next line + if (cacheInfoPtr->fileCount > fileCount) { + token = strtok_r(nullptr, "\n", &state); + } + fileCount += 1; + } while (cacheInfoPtr->fileCount >= fileCount); + + //Find binary format + token = strtok_r(nullptr, "\n", &state); + if (token != nullptr) { + cacheInfoPtr->binaryFormat = std::atoll(token); + } else { + delete [] dataCopy; + return false; + } + + //Find binary length + token = strtok_r(nullptr, "\n", &state); + if (token != nullptr) { + cacheInfoPtr->binaryLength = std::atoll(token); + } else { + delete [] dataCopy; + return false; + } + + if (cacheInfoPtr->binaryFormat == 0 || cacheInfoPtr->binaryLength == 0) { + delete [] dataCopy; + return false; + } + + delete [] dataCopy; + return true; + } + } + } +} diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index b7f43bf4..08ee55a6 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -8,16 +8,12 @@ #include #include -//Used by shader cache validator -#include -#include -#include - #include #include "../core/fileManager.hpp" #include "../utils/logging.hpp" #include "internal/internalExtensions.hpp" +#include "internal/internalShaderValidator.hpp" namespace ammonite { //Static helper functions @@ -122,13 +118,6 @@ namespace ammonite { return GL_FALSE; } - struct CacheInfo { - int fileCount; - std::string* filePaths; - GLenum binaryFormat; - GLsizei binaryLength; - }; - //Set by updateCacheSupport(), when GLEW loads bool isBinaryCacheSupported = false; } @@ -214,102 +203,6 @@ namespace ammonite { return programId; } - bool validateCache(unsigned char* data, std::size_t size, void* userPtr) { - CacheInfo* cacheInfoPtr = (CacheInfo*)userPtr; - unsigned char* dataCopy = new unsigned char[size]; - std::memcpy(dataCopy, data, size); - dataCopy[size - 1] = '\0'; - - /* - - Decide whether the cache file can be used - - Uses input files, sizes and timestamps - */ - char* state; - int fileCount = 1; - char* token = strtok_r((char*)dataCopy, "\n", &state); - do { - //Give up if token is null, we didn't find enough files - if (token == nullptr) { - delete [] dataCopy; - return false; - } - - //Check first token is 'input' - std::string currentFilePath = cacheInfoPtr->filePaths[fileCount - 1]; - char* nestedToken = strtok(token, ";"); - if ((nestedToken == nullptr) || - (std::string(nestedToken) != std::string("input"))) { - delete [] dataCopy; - return false; - } - - //Check token matches shader path - nestedToken = strtok(nullptr, ";"); - if ((nestedToken == nullptr) || - (std::string(nestedToken) != currentFilePath)) { - delete [] dataCopy; - return false; - } - - //Get filesize and time of last modification of the shader source - long long int filesize = 0, modificationTime = 0; - if (!ammonite::files::internal::getFileMetadata(currentFilePath, &filesize, - &modificationTime)) { - //Failed to get the metadata - delete [] dataCopy; - return false; - } - - //Check token matches file size - nestedToken = strtok(nullptr, ";"); - if ((nestedToken == nullptr) || - (std::atoll(nestedToken) != filesize)) { - delete [] dataCopy; - return false; - } - - //Check token matches timestamp - nestedToken = strtok(nullptr, ";"); - if ((nestedToken == nullptr) || - (std::atoll(nestedToken) != modificationTime)) { - delete [] dataCopy; - return false; - } - - //Get the next line - if (cacheInfoPtr->fileCount > fileCount) { - token = strtok_r(nullptr, "\n", &state); - } - fileCount += 1; - } while (cacheInfoPtr->fileCount >= fileCount); - - //Find binary format - token = strtok_r(nullptr, "\n", &state); - if (token != nullptr) { - cacheInfoPtr->binaryFormat = std::atoll(token); - } else { - delete [] dataCopy; - return false; - } - - //Find binary length - token = strtok_r(nullptr, "\n", &state); - if (token != nullptr) { - cacheInfoPtr->binaryLength = std::atoll(token); - } else { - delete [] dataCopy; - return false; - } - - if (cacheInfoPtr->binaryFormat == 0 || cacheInfoPtr->binaryLength == 0) { - delete [] dataCopy; - return false; - } - - delete [] dataCopy; - return true; - } - //Create a program from shader source with loadShader() and createProgramObject() static int createProgramUncached(std::string* shaderPaths, GLenum* shaderTypes, int shaderCount, bool* externalSuccess) { @@ -363,13 +256,13 @@ namespace ammonite { AmmoniteEnum cacheState; //Attempt to load the cached program - CacheInfo cacheInfo; + ammonite::shaders::internal::CacheInfo cacheInfo; cacheInfo.fileCount = shaderCount; cacheInfo.filePaths = shaderPaths; std::string cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, shaderCount); unsigned char* cacheData = ammonite::files::internal::getCachedFile(cacheFilePath, - validateCache, &cacheDataSize, &cacheState, &cacheInfo); + ammonite::shaders::internal::validateCache, &cacheDataSize, &cacheState, &cacheInfo); unsigned char* dataStart = cacheData + cacheDataSize - cacheInfo.binaryLength; if (cacheState == AMMONITE_CACHE_HIT) { From ea0421cdd56a27490dd499d53e434a8e77ac09b9 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 16:58:32 +0100 Subject: [PATCH 03/30] Reimplement strtok_r and strtok, use goto for clean up --- src/ammonite/graphics/shaderValidator.cpp | 108 +++++++++++++--------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/src/ammonite/graphics/shaderValidator.cpp b/src/ammonite/graphics/shaderValidator.cpp index 59723189..e4a30f1a 100644 --- a/src/ammonite/graphics/shaderValidator.cpp +++ b/src/ammonite/graphics/shaderValidator.cpp @@ -1,5 +1,4 @@ #include -#include #include #include #include @@ -10,6 +9,41 @@ namespace ammonite { namespace shaders { namespace internal { + namespace { + /* + - Similar to strtok_r, except it only supports single character delimiters + - Unsafe if called with both input and (save or *save) as nullptr / undefined + - save must never be nullptr + */ + static char* parseToken(char* input, char delim, char** save) { + //Restore from next byte to search + if (input == nullptr) { + input = *save; + } + + //Programmer error or no more tokens + if (*input == '\0') { + return nullptr; + } + + char* end = input; + //Search for delimiter or end of string + while (*end != delim && *end != '\0') { + end++; + } + + //Last token found + if (*end == '\0') { + return input; + } + + //Create a string, save state and return it + *end = '\0'; + *save = end + 1; + return input; + } + } + bool validateCache(unsigned char* data, std::size_t size, void* userPtr) { CacheInfo* cacheInfoPtr = (CacheInfo*)userPtr; unsigned char* dataCopy = new unsigned char[size]; @@ -20,90 +54,78 @@ namespace ammonite { - Decide whether the cache file can be used - Uses input files, sizes and timestamps */ - char* state; + char* state = nullptr; int fileCount = 1; - char* token = strtok_r((char*)dataCopy, "\n", &state); + long long int filesize = 0, modificationTime = 0; + char* token = parseToken((char*)dataCopy, '\n', &state); do { //Give up if token is null, we didn't find enough files if (token == nullptr) { - delete [] dataCopy; - return false; + goto failed; } //Check first token is 'input' std::string currentFilePath = cacheInfoPtr->filePaths[fileCount - 1]; - char* nestedToken = strtok(token, ";"); + char* nestedState = nullptr; + char* nestedToken = parseToken(token, ';', &nestedState); if ((nestedToken == nullptr) || (std::string(nestedToken) != std::string("input"))) { - delete [] dataCopy; - return false; + goto failed; } //Check token matches shader path - nestedToken = strtok(nullptr, ";"); - if ((nestedToken == nullptr) || - (std::string(nestedToken) != currentFilePath)) { - delete [] dataCopy; - return false; + nestedToken = parseToken(nullptr, ';', &nestedState); + if ((nestedToken == nullptr) || (std::string(nestedToken) != currentFilePath)) { + goto failed; } //Get filesize and time of last modification of the shader source - long long int filesize = 0, modificationTime = 0; if (!ammonite::files::internal::getFileMetadata(currentFilePath, &filesize, &modificationTime)) { - //Failed to get the metadata - delete [] dataCopy; - return false; + goto failed; } //Check token matches file size - nestedToken = strtok(nullptr, ";"); - if ((nestedToken == nullptr) || - (std::atoll(nestedToken) != filesize)) { - delete [] dataCopy; - return false; + nestedToken = parseToken(nullptr, ';', &nestedState); + if ((nestedToken == nullptr) || (std::atoll(nestedToken) != filesize)) { + goto failed; } //Check token matches timestamp - nestedToken = strtok(nullptr, ";"); - if ((nestedToken == nullptr) || - (std::atoll(nestedToken) != modificationTime)) { - delete [] dataCopy; - return false; + nestedToken = parseToken(nullptr, ';', &nestedState); + if ((nestedToken == nullptr) || (std::atoll(nestedToken) != modificationTime)) { + goto failed; } //Get the next line if (cacheInfoPtr->fileCount > fileCount) { - token = strtok_r(nullptr, "\n", &state); + token = parseToken(nullptr, '\n', &state); } fileCount += 1; } while (cacheInfoPtr->fileCount >= fileCount); //Find binary format - token = strtok_r(nullptr, "\n", &state); - if (token != nullptr) { - cacheInfoPtr->binaryFormat = std::atoll(token); - } else { - delete [] dataCopy; - return false; + token = parseToken(nullptr, '\n', &state); + if (token == nullptr) { + goto failed; } + cacheInfoPtr->binaryFormat = std::atoll(token); //Find binary length - token = strtok_r(nullptr, "\n", &state); - if (token != nullptr) { - cacheInfoPtr->binaryLength = std::atoll(token); - } else { - delete [] dataCopy; - return false; + token = parseToken(nullptr, '\n', &state); + if (token == nullptr) { + goto failed; } + cacheInfoPtr->binaryLength = std::atoll(token); - if (cacheInfoPtr->binaryFormat == 0 || cacheInfoPtr->binaryLength == 0) { + if (cacheInfoPtr->binaryFormat != 0 && cacheInfoPtr->binaryLength != 0) { delete [] dataCopy; - return false; + return true; } +failed: delete [] dataCopy; - return true; + return false; } } } From 445a38d5786ed644e64a30a79e649c30b4679e71 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 17:04:38 +0100 Subject: [PATCH 04/30] Sanity check binary cache size --- src/ammonite/graphics/shaderValidator.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ammonite/graphics/shaderValidator.cpp b/src/ammonite/graphics/shaderValidator.cpp index e4a30f1a..0e90d792 100644 --- a/src/ammonite/graphics/shaderValidator.cpp +++ b/src/ammonite/graphics/shaderValidator.cpp @@ -118,9 +118,13 @@ namespace ammonite { } cacheInfoPtr->binaryLength = std::atoll(token); + //Check fetched binary info if (cacheInfoPtr->binaryFormat != 0 && cacheInfoPtr->binaryLength != 0) { - delete [] dataCopy; - return true; + //Sanity check binary size + if ((std::size_t)cacheInfoPtr->binaryLength < size && cacheInfoPtr->binaryLength > 0) { + delete [] dataCopy; + return true; + } } failed: From d13472686e2a5688eec0478572cd5f127231a52c Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 17:10:17 +0100 Subject: [PATCH 05/30] Avoid extra sysclass while reading shader cache --- src/ammonite/core/fileManager.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index 92dc854c..4c0c9ff7 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -133,7 +133,11 @@ namespace ammonite { delete [] data; return nullptr; } + bytesRead += newBytesRead; + if (bytesRead == statBuf.st_size) { + break; + } } if (bytesRead != statBuf.st_size) { From 43ba7a44c2fd28f728c4c5888ce6e79bb8106462 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 17:37:25 +0100 Subject: [PATCH 06/30] Use core/fileManager to read shader source code --- src/ammonite/graphics/shaders.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 08ee55a6..58ab7570 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include @@ -129,24 +128,18 @@ namespace ammonite { //Create the shader GLuint shaderId = glCreateShader(shaderType); - std::string shaderCode; - std::ifstream shaderCodeStream(shaderPath, std::ios::in); - std::stringstream sstr; - //Read the shader's source code - if (shaderCodeStream.is_open()) { - sstr << shaderCodeStream.rdbuf(); - shaderCode = sstr.str(); - shaderCodeStream.close(); - } else { + std::size_t shaderCodeSize; + char* shaderCodePtr = (char*)ammonite::files::internal::loadFile(shaderPath, + &shaderCodeSize); + if (shaderCodePtr == nullptr) { ammonite::utils::warning << "Failed to open '" << shaderPath << "'" << std::endl; *externalSuccess = false; return -1; } - //Provide a shader source and compile the shader - const char* shaderCodePointer = shaderCode.c_str(); - glShaderSource(shaderId, 1, &shaderCodePointer, nullptr); + glShaderSource(shaderId, 1, &shaderCodePtr, (int*)&shaderCodeSize); + delete [] shaderCodePtr; glCompileShader(shaderId); //Test whether the shader compiled From d381c7ad3943d0ea9a41ac2081d20f4930dc0c48 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 17:39:49 +0100 Subject: [PATCH 07/30] Remove unused include --- src/ammonite/graphics/shaders.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 58ab7570..b53f4f20 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -1,4 +1,3 @@ -#include #include #include #include From af5da118929e146cc951e3f50a249ea5d48e7b04 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 29 Jun 2024 17:49:52 +0100 Subject: [PATCH 08/30] Warn if posix_fadvise fails --- src/ammonite/core/fileManager.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index 4c0c9ff7..af18cb22 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -113,13 +113,16 @@ namespace ammonite { ammonite::utils::warning << "Error while opening '" << filePath \ << "' (" << errno << ")" << std::endl; } - posix_fadvise(descriptor, 0, 0, POSIX_FADV_SEQUENTIAL); struct stat statBuf; fstat(descriptor, &statBuf); *size = statBuf.st_size; data = new unsigned char[statBuf.st_size]; + if (posix_fadvise(descriptor, 0, 0, POSIX_FADV_SEQUENTIAL)) { + ammonite::utils::warning << "Error while advising kernel, continuing" << std::endl; + } + off_t bytesRead = 0; while (bytesRead < statBuf.st_size) { off_t newBytesRead = read(descriptor, data + bytesRead, From d371a5b2a7328365c592f26d31a0ffa40329fea6 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 14:08:11 +0100 Subject: [PATCH 09/30] Add writeFile() to file manager, use it for shader caching --- src/ammonite/core/fileManager.cpp | 53 +++++++++++++++++++++++++++--- src/ammonite/core/fileManager.hpp | 1 + src/ammonite/graphics/shaders.cpp | 54 +++++++++++++++++++------------ 3 files changed, 84 insertions(+), 24 deletions(-) diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index af18cb22..5bf05bf6 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -111,7 +111,7 @@ namespace ammonite { int descriptor = open(filePath.c_str(), O_RDONLY); if (descriptor == -1) { ammonite::utils::warning << "Error while opening '" << filePath \ - << "' (" << errno << ")" << std::endl; + << "' (" << -errno << ")" << std::endl; } struct stat statBuf; @@ -125,13 +125,12 @@ namespace ammonite { off_t bytesRead = 0; while (bytesRead < statBuf.st_size) { - off_t newBytesRead = read(descriptor, data + bytesRead, - statBuf.st_size - bytesRead); + off_t newBytesRead = read(descriptor, data + bytesRead, statBuf.st_size - bytesRead); if (newBytesRead == 0) { break; } else if (newBytesRead < 0) { ammonite::utils::warning << "Error while reading '" << filePath \ - << "' (" << errno << ")" << std::endl; + << "' (" << -errno << ")" << std::endl; close(descriptor); delete [] data; return nullptr; @@ -155,6 +154,52 @@ namespace ammonite { return data; } + /* + - Write size bytes of data to filePath + - Create the file if missing + - Erase the file if present + - Returns true on success, false on failure + */ + bool writeFile(std::string filePath, unsigned char* data, std::size_t size) { + int descriptor = creat(filePath.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH); + if (descriptor == -1) { + ammonite::utils::warning << "Error while opening '" << filePath \ + << "' (" << -errno << ")" << std::endl; + } + + if (posix_fadvise(descriptor, 0, 0, POSIX_FADV_SEQUENTIAL)) { + ammonite::utils::warning << "Error while advising kernel, continuing" << std::endl; + } + + std::size_t bytesWritten = 0; + while (bytesWritten < size) { + off_t newBytesWritten = write(descriptor, data + bytesWritten, size - bytesWritten); + if (newBytesWritten == 0) { + break; + } else if (newBytesWritten < 0) { + ammonite::utils::warning << "Error while writing to '" << filePath \ + << "' (" << -errno << ")" << std::endl; + close(descriptor); + return false; + } + + bytesWritten += newBytesWritten; + if (bytesWritten == size) { + break; + } + } + + if (bytesWritten != size) { + ammonite::utils::warning << "Unexpected file size while writing to '" << filePath \ + << "'" << std::endl; + close(descriptor); + return false; + } + + close(descriptor); + return true; + } + /* - Attempt to read a cached file, and validate it with validator - If the cache was valid, write to size and cacheState, then return cacheData diff --git a/src/ammonite/core/fileManager.hpp b/src/ammonite/core/fileManager.hpp index 05e69fbc..75831a54 100644 --- a/src/ammonite/core/fileManager.hpp +++ b/src/ammonite/core/fileManager.hpp @@ -19,6 +19,7 @@ namespace ammonite { std::string getCachedFilePath(std::string* filePaths, unsigned int fileCount); unsigned char* loadFile(std::string filePath, std::size_t* size); + bool writeFile(std::string filePath, unsigned char* data, std::size_t size); unsigned char* getCachedFile(std::string cacheFilePath, AmmoniteValidator validator, std::size_t* size, AmmoniteEnum* cacheState, void* userPtr); } diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index b53f4f20..8238c70b 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -25,44 +26,57 @@ namespace ammonite { static void cacheProgram(const GLuint programId, std::string* shaderPaths, int shaderCount) { std::string cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, shaderCount); - ammonite::utils::status << "Caching '" << cacheFilePath << "'" << std::endl; - int binaryLength; - GLenum binaryFormat; - //Get binary length and data of linked program + //Get binary length of linked program + int binaryLength; glGetProgramiv(programId, GL_PROGRAM_BINARY_LENGTH, &binaryLength); if (binaryLength == 0) { ammonite::utils::warning << "Failed to cache '" << cacheFilePath << "'" << std::endl; return; } + //Get binary format and binary data + GLenum binaryFormat; + int actualBytes = 0; char* binaryData = new char[binaryLength]; - glGetProgramBinary(programId, binaryLength, nullptr, &binaryFormat, binaryData); + glGetProgramBinary(programId, binaryLength, &actualBytes, &binaryFormat, binaryData); + if (actualBytes != binaryLength) { + ammonite::utils::warning << "Program length doesn't match expected length (ID " \ + << programId << ")" << std::endl; + delete [] binaryData; + return; + } - //Write the cache info and data to cache file - std::ofstream cacheFile(cacheFilePath, std::ios::binary); - if (cacheFile.is_open()) { - for (int i = 0; i < shaderCount; i++) { - long long int filesize = 0, modificationTime = 0; - ammonite::files::internal::getFileMetadata(shaderPaths[i], &filesize, &modificationTime); + //Generate cache info to write + std::string extraData; + for (int i = 0; i < shaderCount; i++) { + long long int filesize = 0, modificationTime = 0; + ammonite::files::internal::getFileMetadata(shaderPaths[i], &filesize, &modificationTime); - cacheFile << "input;" << shaderPaths[i] << ";" << filesize << ";" \ - << modificationTime << "\n"; - } + extraData.append("input;" + shaderPaths[i]); + extraData.append(";" + std::to_string(filesize)); + extraData.append(";" + std::to_string(modificationTime) + "\n"); + } + + extraData.append(std::to_string(binaryFormat) + "\n"); + extraData.append(std::to_string(binaryLength) + "\n"); - cacheFile << binaryFormat << "\n"; - cacheFile << binaryLength << "\n"; + int extraLength = extraData.length(); + char* fileData = new char[extraLength + binaryLength]; - //Write the data - cacheFile.write(binaryData, binaryLength); + //Write the cache info and binary data to the buffer + extraData.copy(fileData, extraLength, 0); + std::memcpy(fileData + extraLength, binaryData, binaryLength); - cacheFile.close(); - } else { + //Write the cache info and data to the cache file + if (!ammonite::files::internal::writeFile(cacheFilePath, (unsigned char*)fileData, + extraLength + binaryLength)) { ammonite::utils::warning << "Failed to cache '" << cacheFilePath << "'" << std::endl; deleteCacheFile(cacheFilePath); } + delete [] fileData; delete [] binaryData; } From 5a45ab3d379ab80a9ffdc43be463cb397d98fe64 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 14:09:14 +0100 Subject: [PATCH 10/30] Bail out if the open / creat call fails --- src/ammonite/core/fileManager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index 5bf05bf6..a987b91d 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -112,6 +112,7 @@ namespace ammonite { if (descriptor == -1) { ammonite::utils::warning << "Error while opening '" << filePath \ << "' (" << -errno << ")" << std::endl; + return nullptr; } struct stat statBuf; @@ -165,6 +166,7 @@ namespace ammonite { if (descriptor == -1) { ammonite::utils::warning << "Error while opening '" << filePath \ << "' (" << -errno << ")" << std::endl; + return false; } if (posix_fadvise(descriptor, 0, 0, POSIX_FADV_SEQUENTIAL)) { From e653d04a7227b85907ab757aeea49da4d1ff43d5 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 14:10:24 +0100 Subject: [PATCH 11/30] Remove from shader loader --- src/ammonite/graphics/shaders.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 8238c70b..a9adee51 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include From 1a5c7760d11aa1684c46316d8c1f0a1a33164716 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 14:23:17 +0100 Subject: [PATCH 12/30] Check permissions of cache directory before use --- src/ammonite/core/fileManager.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index a987b91d..e5667d06 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -80,6 +80,13 @@ namespace ammonite { } } + //Check for read and write permissions + if (access(targetCachePath.c_str(), R_OK | W_OK)) { + ammonite::utils::warning << "Insufficient permissions to use cache directory '" \ + << targetCachePath << "'" << std::endl; + return false; + } + //Ensure path has a trailing slash dataCachePath = targetCachePath; if (dataCachePath.back() != '/') { From 96ac0f4ea89ab1efca0f612719ffb05d4324acb0 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 14:23:27 +0100 Subject: [PATCH 13/30] Drop extra whitespace --- src/ammonite/graphics/shaders.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index a9adee51..1c9b0f31 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -246,7 +246,6 @@ namespace ammonite { return programId; } - //Attempt to find cached program or hand off to createProgramUncached() static int createProgramCached(std::string* shaderPaths, GLenum* shaderTypes, int shaderCount, bool* externalSuccess) { From 9d636d7bc3f84d60c6c79d574d4549ff83a627ec Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 14:23:36 +0100 Subject: [PATCH 14/30] Add trailing slash to cache directory for consistency --- src/demo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/demo.cpp b/src/demo.cpp index 28806952..b37b1244 100644 --- a/src/demo.cpp +++ b/src/demo.cpp @@ -230,7 +230,7 @@ int main(int argc, char* argv[]) { //Enable engine caching, setup renderer and initialise controls bool success = true; - ammonite::renderer::setup::useShaderCache("cache"); + ammonite::renderer::setup::useShaderCache("cache/"); ammonite::renderer::setup::setupRenderer("shaders/", &success); setupBits |= HAS_SETUP_RENDERER; From cec25ceedda252c1e19a3a593310e45844cf0b5a Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 17:57:35 +0100 Subject: [PATCH 15/30] Handle no available program log, ignore written length --- src/ammonite/graphics/shaders.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 1c9b0f31..c01f21fb 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -89,13 +89,21 @@ namespace ammonite { return true; } - //Otherwise, print a log - GLint maxLength = 0; + //Get length of a log, if available + GLsizei maxLength = 0; glGetProgramiv(programId, GL_INFO_LOG_LENGTH, &maxLength); + if (maxLength == 0) { + ammonite::utils::warning << "Failed to link shader program (ID " << programId \ + << "), no log available" << std::endl; + return false; + } - //Not strictly required, but add an extra byte to be safe - GLchar* errorLogBuffer = new GLchar[++maxLength]; - glGetProgramInfoLog(programId, maxLength, &maxLength, errorLogBuffer); + /* + - Fetch and print the log + - The extra byte isn't strictly required, but some drivers are buggy + */ + GLchar* errorLogBuffer = new GLchar[maxLength + 1]; + glGetProgramInfoLog(programId, maxLength, nullptr, errorLogBuffer); errorLogBuffer[maxLength] = '\0'; ammonite::utils::warning << "Failed to link shader program (ID " << programId \ << "):\n" << (char*)errorLogBuffer << std::endl; From f6ad16a7fbaa5f7ced8ad0f93a86438aae78b5a5 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 20:49:25 +0100 Subject: [PATCH 16/30] Apply logging fixed from program linking to shader compilation --- src/ammonite/graphics/shaders.cpp | 58 ++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index c01f21fb..df4fbbb0 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -112,6 +112,39 @@ namespace ammonite { return false; } + static bool checkShader(GLuint shaderId) { + //Test whether the shader compiled + GLint success = GL_FALSE; + glGetShaderiv(shaderId, GL_COMPILE_STATUS, &success); + + //If the shader compiled successfully, return + if (success == GL_TRUE) { + return true; + } + + //Get length of a log, if available + GLsizei maxLength = 0; + glGetShaderiv(shaderId, GL_INFO_LOG_LENGTH, &maxLength); + if (maxLength == 0) { + ammonite::utils::warning << "Failed to compile shader stage (ID " << shaderId \ + << "), no log available" << std::endl; + return false; + } + + /* + - Fetch and print the log + - The extra byte isn't strictly required, but some drivers are buggy + */ + GLchar* errorLogBuffer = new GLchar[maxLength + 1]; + glGetShaderInfoLog(shaderId, maxLength, nullptr, errorLogBuffer); + errorLogBuffer[maxLength] = '\0'; + ammonite::utils::warning << "Failed to compile shader stage (ID " << shaderId \ + << "):\n" << (char*)errorLogBuffer << std::endl; + + delete [] errorLogBuffer; + return false; + } + static GLenum attemptIdentifyShaderType(std::string shaderPath) { std::map shaderMatches = { {"vert", GL_VERTEX_SHADER}, @@ -159,27 +192,11 @@ namespace ammonite { } glShaderSource(shaderId, 1, &shaderCodePtr, (int*)&shaderCodeSize); - delete [] shaderCodePtr; glCompileShader(shaderId); + delete [] shaderCodePtr; - //Test whether the shader compiled - GLint success = GL_FALSE; - glGetShaderiv(shaderId, GL_COMPILE_STATUS, &success); - - //If the shader failed to compile, print a log - if (success == GL_FALSE) { - GLint maxLength = 0; - glGetShaderiv(shaderId, GL_INFO_LOG_LENGTH, &maxLength); - - //Not strictly required, but add an extra byte to be safe - GLchar* errorLogBuffer = new GLchar[++maxLength]; - glGetShaderInfoLog(shaderId, maxLength, &maxLength, errorLogBuffer); - errorLogBuffer[maxLength] = '\0'; - ammonite::utils::warning << shaderPath << ":\n" \ - << (char*)errorLogBuffer << std::endl; - - //Clean up and exit - delete [] errorLogBuffer; + //Check whether the shader compiled, log if relevant + if (!checkShader(shaderId)) { glDeleteShader(shaderId); *externalSuccess = false; return -1; @@ -194,11 +211,10 @@ namespace ammonite { //Create the program GLuint programId = glCreateProgram(); - //Attach all passed shader ids + //Attach and link all passed shader ids for (int i = 0; i < shaderCount; i++) { glAttachShader(programId, shaderIds[i]); } - glLinkProgram(programId); //Detach and remove all passed shader ids From 60f89ecacb46081d22076fbe34d0f2ccb5fdfcb2 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 30 Jun 2024 23:02:06 +0100 Subject: [PATCH 17/30] Share program and shader checking code --- src/ammonite/graphics/shaders.cpp | 55 ++++++++++--------------------- 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index df4fbbb0..68cb7e27 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -79,10 +79,12 @@ namespace ammonite { delete [] binaryData; } - static bool checkProgram(GLuint programId) { + static bool checkObject(GLuint objectId, const char* actionString, GLenum statusEnum, + void (*objectQuery)(GLuint, GLenum, GLint*), + void (*objectLog)(GLuint, GLsizei, GLsizei*, GLchar*)) { //Test whether the program linked GLint success = GL_FALSE; - glGetProgramiv(programId, GL_LINK_STATUS, &success); + objectQuery(objectId, statusEnum, &success); //If the program linked successfully, return if (success == GL_TRUE) { @@ -91,9 +93,9 @@ namespace ammonite { //Get length of a log, if available GLsizei maxLength = 0; - glGetProgramiv(programId, GL_INFO_LOG_LENGTH, &maxLength); + objectQuery(objectId, GL_INFO_LOG_LENGTH, &maxLength); if (maxLength == 0) { - ammonite::utils::warning << "Failed to link shader program (ID " << programId \ + ammonite::utils::warning << "Failed to " << actionString << " (ID " << objectId \ << "), no log available" << std::endl; return false; } @@ -103,46 +105,23 @@ namespace ammonite { - The extra byte isn't strictly required, but some drivers are buggy */ GLchar* errorLogBuffer = new GLchar[maxLength + 1]; - glGetProgramInfoLog(programId, maxLength, nullptr, errorLogBuffer); + objectLog(objectId, maxLength, nullptr, errorLogBuffer); errorLogBuffer[maxLength] = '\0'; - ammonite::utils::warning << "Failed to link shader program (ID " << programId \ + ammonite::utils::warning << "Failed to " << actionString << " (ID " << objectId \ << "):\n" << (char*)errorLogBuffer << std::endl; delete [] errorLogBuffer; return false; } - static bool checkShader(GLuint shaderId) { - //Test whether the shader compiled - GLint success = GL_FALSE; - glGetShaderiv(shaderId, GL_COMPILE_STATUS, &success); - - //If the shader compiled successfully, return - if (success == GL_TRUE) { - return true; - } - - //Get length of a log, if available - GLsizei maxLength = 0; - glGetShaderiv(shaderId, GL_INFO_LOG_LENGTH, &maxLength); - if (maxLength == 0) { - ammonite::utils::warning << "Failed to compile shader stage (ID " << shaderId \ - << "), no log available" << std::endl; - return false; - } - - /* - - Fetch and print the log - - The extra byte isn't strictly required, but some drivers are buggy - */ - GLchar* errorLogBuffer = new GLchar[maxLength + 1]; - glGetShaderInfoLog(shaderId, maxLength, nullptr, errorLogBuffer); - errorLogBuffer[maxLength] = '\0'; - ammonite::utils::warning << "Failed to compile shader stage (ID " << shaderId \ - << "):\n" << (char*)errorLogBuffer << std::endl; + static bool checkProgram(GLuint programId) { + return checkObject(programId, "link shader program", GL_LINK_STATUS, + glGetProgramiv, glGetProgramInfoLog); + } - delete [] errorLogBuffer; - return false; + static bool checkShader(GLuint shaderId) { + return checkObject(shaderId, "compile shader stage", GL_COMPILE_STATUS, + glGetShaderiv, glGetShaderInfoLog); } static GLenum attemptIdentifyShaderType(std::string shaderPath) { @@ -205,8 +184,8 @@ namespace ammonite { return shaderId; } - //Take multiple shader files, hand off to loadShader and create a program - static int createProgramObject(GLuint shaderIds[], const int shaderCount, + //Take multiple shader objects and create a program + static int createProgramObject(GLuint* shaderIds, const int shaderCount, bool* externalSuccess) { //Create the program GLuint programId = glCreateProgram(); From 54a942927e09af198c29c8e32b6733c762dadd36 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Tue, 2 Jul 2024 16:50:53 +0100 Subject: [PATCH 18/30] Fix shader cache being enabled regardless of OpenGL support --- src/ammonite/graphics/shaders.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 68cb7e27..301f1644 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -311,6 +311,7 @@ namespace ammonite { glGetIntegerv(GL_NUM_PROGRAM_BINARY_FORMATS, &numBinaryFormats); //Check support for collecting the program binary + isBinaryCacheSupported = true; if (!graphics::internal::checkExtension("GL_ARB_get_program_binary", "GL_VERSION_4_1")) { ammonite::utils::warning << "Program caching unsupported" << std::endl; isBinaryCacheSupported = false; @@ -319,8 +320,6 @@ namespace ammonite { << std::endl; isBinaryCacheSupported = false; } - - isBinaryCacheSupported = true; } //Find shader types and hand off to createProgramCached(paths, types) From e01b2215f854d179ee14394ed3a25d1073d71396 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Tue, 2 Jul 2024 17:03:51 +0100 Subject: [PATCH 19/30] Describe exposed shader functions --- src/ammonite/graphics/shaders.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 301f1644..a3e664aa 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -305,6 +305,7 @@ namespace ammonite { //Internally exposed functions namespace shaders { namespace internal { + //Set isBinaryCacheSupported according to OpenGL support void updateCacheSupport() { //Get number of supported formats GLint numBinaryFormats = 0; @@ -322,7 +323,12 @@ namespace ammonite { } } - //Find shader types and hand off to createProgramCached(paths, types) + /* + - Take an array of shader paths, create a program and return the ID + - Shaders with unidentifiable types will be ignored + - If possible, load and store a cache + - Writes 'false' to externalSuccess on failure and returns -1 + */ int createProgram(std::string* inputShaderPaths, int inputShaderCount, bool* externalSuccess) { //Convert file extensions to shader types @@ -388,7 +394,11 @@ namespace ammonite { return programId; } - //Load all shaders in a directory and hand off to createProgram() + /* + - Create a program from shaders in a directory and return the ID + - If possible, load and store a cache + - Writes 'false' to externalSuccess on failure and returns -1 + */ int loadDirectory(const char* directoryPath, bool* externalSuccess) { //Create filesystem directory iterator std::filesystem::directory_iterator it; From 70ccdfd84bb1093d63503dabfca7377e6028c728 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Tue, 2 Jul 2024 17:05:39 +0100 Subject: [PATCH 20/30] Directly return results of createProgram and createProgramCached --- src/ammonite/graphics/shaders.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index a3e664aa..f018a216 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -389,9 +389,8 @@ namespace ammonite { } //Create the program and return the ID - int programId = createProgramCached(&shaderPaths[0], &shaderTypes[0], - shaderPaths.size(), externalSuccess); - return programId; + return createProgramCached(&shaderPaths[0], &shaderTypes[0], shaderPaths.size(), + externalSuccess); } /* @@ -419,8 +418,7 @@ namespace ammonite { } //Create the program and return the ID - int programId = createProgram(&shaderPaths[0], shaderPaths.size(), externalSuccess); - return programId; + return createProgram(&shaderPaths[0], shaderPaths.size(), externalSuccess); } } } From 842ee46653eb463112fc865a3d55254da2fd93d2 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Tue, 2 Jul 2024 18:42:29 +0100 Subject: [PATCH 21/30] Do shader cache writes using the thread pool --- src/ammonite/graphics/shaders.cpp | 125 +++++++++++++++++++----------- 1 file changed, 79 insertions(+), 46 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index f018a216..de452552 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -9,74 +9,111 @@ #include #include "../core/fileManager.hpp" +#include "../core/threadManager.hpp" + #include "../utils/logging.hpp" #include "internal/internalExtensions.hpp" #include "internal/internalShaderValidator.hpp" namespace ammonite { - //Static helper functions + //Static helper functions and anonymous data namespace { + namespace { + //Set by updateCacheSupport(), when GLEW loads + bool isBinaryCacheSupported = false; + + //Data required by cache worker + struct CacheWorkerData { + int shaderCount; + std::string* shaderPaths; + std::string cacheFilePath; + GLenum binaryFormat; + int binaryLength; + char* binaryData; + }; + } + static void deleteCacheFile(std::string cacheFilePath) { //Delete the cache file ammonite::utils::status << "Clearing '" << cacheFilePath << "'" << std::endl; ammonite::files::internal::deleteFile(cacheFilePath); } - static void cacheProgram(const GLuint programId, std::string* shaderPaths, int shaderCount) { - std::string cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, - shaderCount); - ammonite::utils::status << "Caching '" << cacheFilePath << "'" << std::endl; - - //Get binary length of linked program - int binaryLength; - glGetProgramiv(programId, GL_PROGRAM_BINARY_LENGTH, &binaryLength); - if (binaryLength == 0) { - ammonite::utils::warning << "Failed to cache '" << cacheFilePath << "'" << std::endl; - return; - } - - //Get binary format and binary data - GLenum binaryFormat; - int actualBytes = 0; - char* binaryData = new char[binaryLength]; - glGetProgramBinary(programId, binaryLength, &actualBytes, &binaryFormat, binaryData); - if (actualBytes != binaryLength) { - ammonite::utils::warning << "Program length doesn't match expected length (ID " \ - << programId << ")" << std::endl; - delete [] binaryData; - return; - } + //Thread pool work to cache a program + static void doCacheWork(void* userPtr) { + CacheWorkerData* data = (CacheWorkerData*)userPtr; //Generate cache info to write std::string extraData; - for (int i = 0; i < shaderCount; i++) { + for (int i = 0; i < data->shaderCount; i++) { long long int filesize = 0, modificationTime = 0; - ammonite::files::internal::getFileMetadata(shaderPaths[i], &filesize, &modificationTime); + ammonite::files::internal::getFileMetadata(data->shaderPaths[i], &filesize, + &modificationTime); - extraData.append("input;" + shaderPaths[i]); + extraData.append("input;" + data->shaderPaths[i]); extraData.append(";" + std::to_string(filesize)); extraData.append(";" + std::to_string(modificationTime) + "\n"); } - extraData.append(std::to_string(binaryFormat) + "\n"); - extraData.append(std::to_string(binaryLength) + "\n"); + extraData.append(std::to_string(data->binaryFormat) + "\n"); + extraData.append(std::to_string(data->binaryLength) + "\n"); int extraLength = extraData.length(); - char* fileData = new char[extraLength + binaryLength]; + char* fileData = new char[extraLength + data->binaryLength]; //Write the cache info and binary data to the buffer extraData.copy(fileData, extraLength, 0); - std::memcpy(fileData + extraLength, binaryData, binaryLength); + std::memcpy(fileData + extraLength, data->binaryData, data->binaryLength); //Write the cache info and data to the cache file - if (!ammonite::files::internal::writeFile(cacheFilePath, (unsigned char*)fileData, - extraLength + binaryLength)) { - ammonite::utils::warning << "Failed to cache '" << cacheFilePath << "'" << std::endl; - deleteCacheFile(cacheFilePath); + if (!ammonite::files::internal::writeFile(data->cacheFilePath, (unsigned char*)fileData, + extraLength + data->binaryLength)) { + ammonite::utils::warning << "Failed to cache '" << data->cacheFilePath << "'" << std::endl; + deleteCacheFile(data->cacheFilePath); } delete [] fileData; - delete [] binaryData; + delete [] data->binaryData; + delete [] data->shaderPaths; + delete data; + } + + static void cacheProgram(const GLuint programId, std::string* shaderPaths, int shaderCount, + std::string* cacheFilePath) { + CacheWorkerData* data = new CacheWorkerData; + data->shaderCount = shaderCount; + + data->cacheFilePath = *cacheFilePath; + ammonite::utils::status << "Caching '" << data->cacheFilePath << "'" << std::endl; + + //Get binary length of linked program + glGetProgramiv(programId, GL_PROGRAM_BINARY_LENGTH, &data->binaryLength); + if (data->binaryLength == 0) { + ammonite::utils::warning << "Failed to cache '" << data->cacheFilePath << "'" << std::endl; + delete data; + return; + } + + //Get binary format and binary data + int actualBytes = 0; + data->binaryData = new char[data->binaryLength]; + glGetProgramBinary(programId, data->binaryLength, &actualBytes, &data->binaryFormat, + data->binaryData); + if (actualBytes != data->binaryLength) { + ammonite::utils::warning << "Program length doesn't match expected length (ID " \ + << programId << ")" << std::endl; + delete [] data->binaryData; + delete data; + return; + } + + //Pack data for writing program cache + data->shaderPaths = new std::string[shaderCount]; + for (int i = 0; i < shaderCount; i++) { + data->shaderPaths[i] = shaderPaths[i]; + } + + ammonite::thread::internal::submitWork(doCacheWork, data, nullptr); } static bool checkObject(GLuint objectId, const char* actionString, GLenum statusEnum, @@ -148,9 +185,6 @@ namespace ammonite { return GL_FALSE; } - - //Set by updateCacheSupport(), when GLEW loads - bool isBinaryCacheSupported = false; } //Shader compilation and cache functions, local to this file @@ -249,7 +283,7 @@ namespace ammonite { return programId; } - //Attempt to find cached program or hand off to createProgramUncached() + //Attempt to use a cached program or hand off to createProgramUncached() static int createProgramCached(std::string* shaderPaths, GLenum* shaderTypes, int shaderCount, bool* externalSuccess) { //Check for OpenGL and engine cache support @@ -258,15 +292,14 @@ namespace ammonite { //Try and fetch the cache, then try and load it into a program int programId; + std::string cacheFilePath; if (isCacheSupported) { std::size_t cacheDataSize; AmmoniteEnum cacheState; //Attempt to load the cached program - ammonite::shaders::internal::CacheInfo cacheInfo; - cacheInfo.fileCount = shaderCount; - cacheInfo.filePaths = shaderPaths; - std::string cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, + ammonite::shaders::internal::CacheInfo cacheInfo = {shaderCount, shaderPaths, 0, 0}; + cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, shaderCount); unsigned char* cacheData = ammonite::files::internal::getCachedFile(cacheFilePath, ammonite::shaders::internal::validateCache, &cacheDataSize, &cacheState, &cacheInfo); @@ -295,7 +328,7 @@ namespace ammonite { //Cache the binary if enabled if (isCacheSupported && programId != -1) { - cacheProgram(programId, shaderPaths, shaderCount); + cacheProgram(programId, shaderPaths, shaderCount, &cacheFilePath); } return programId; From c6c0106d5893ad5b0f99af6be179c42d23067c9d Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Thu, 4 Jul 2024 01:50:10 +0100 Subject: [PATCH 22/30] Optimise shader cache string generation --- src/ammonite/core/fileManager.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index e5667d06..4c3fad76 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -24,15 +25,30 @@ namespace ammonite { } namespace { - //Hash input filenames together to create a unique cache string + //Don't use this for security, you'll lose your job static std::string generateCacheString(std::string* filePaths, unsigned int fileCount) { - std::string inputString = std::string(""); + uint8_t output[8] = {0}; + uint8_t prev = 0; + + /* + - XOR the first byte of the hash with each character of each path + - After each character, sequentially XOR every byte of the hash with the result of + the previous XOR + */ for (unsigned int i = 0; i < fileCount; i++) { - inputString += filePaths[i] + std::string(";"); + uint8_t* filePath = (uint8_t*)filePaths[i].c_str(); + int pathLength = filePaths[i].length(); + for (int i = 0; i < pathLength; i++) { + output[0] ^= filePath[i]; + for (int j = 0; j < 8; j++) { + output[j] ^= prev; + prev = output[j]; + } + } } - return std::to_string(std::hash{}(inputString)); + return std::to_string(*(uint64_t*)output); } } From b97031306d630bfe6dad9608f925a2538b5aa8f9 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Thu, 4 Jul 2024 20:57:08 +0100 Subject: [PATCH 23/30] Order shaders before caching to avoid re-caching due to order changes --- src/ammonite/graphics/shaders.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index de452552..a777e589 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -359,6 +360,7 @@ namespace ammonite { /* - Take an array of shader paths, create a program and return the ID - Shaders with unidentifiable types will be ignored + - The order of shaders may be changed without re-caching - If possible, load and store a cache - Writes 'false' to externalSuccess on failure and returns -1 */ @@ -421,6 +423,9 @@ namespace ammonite { } } + //Ensure shaders don't get rebuilt due to a file order change + std::sort(shaderPaths.begin(), shaderPaths.end()); + //Create the program and return the ID return createProgramCached(&shaderPaths[0], &shaderTypes[0], shaderPaths.size(), externalSuccess); From 911a0a697e1521a3bba13fb56ee7ebada67a3a3a Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Fri, 5 Jul 2024 20:41:17 +0100 Subject: [PATCH 24/30] Align hash output buffer with uint64_t to ensure safe conversions --- src/ammonite/core/fileManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index 4c3fad76..f0f76efa 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -28,7 +28,7 @@ namespace ammonite { //Don't use this for security, you'll lose your job static std::string generateCacheString(std::string* filePaths, unsigned int fileCount) { - uint8_t output[8] = {0}; + alignas(uint64_t) uint8_t output[8] = {0}; uint8_t prev = 0; /* From 307f03cb902186f3ddb688acbfd2a05ce306e2f9 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Fri, 5 Jul 2024 20:51:34 +0100 Subject: [PATCH 25/30] Move shader load reordering to loadDirectory() --- src/ammonite/graphics/shaders.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index a777e589..2339e7db 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -360,8 +360,7 @@ namespace ammonite { /* - Take an array of shader paths, create a program and return the ID - Shaders with unidentifiable types will be ignored - - The order of shaders may be changed without re-caching - - If possible, load and store a cache + - If possible, load and store a cache - Writes 'false' to externalSuccess on failure and returns -1 */ int createProgram(std::string* inputShaderPaths, int inputShaderCount, @@ -423,9 +422,6 @@ namespace ammonite { } } - //Ensure shaders don't get rebuilt due to a file order change - std::sort(shaderPaths.begin(), shaderPaths.end()); - //Create the program and return the ID return createProgramCached(&shaderPaths[0], &shaderTypes[0], shaderPaths.size(), externalSuccess); @@ -433,7 +429,8 @@ namespace ammonite { /* - Create a program from shaders in a directory and return the ID - - If possible, load and store a cache + - The order of shaders may be changed without re-caching + - If possible, load and store a cache - Writes 'false' to externalSuccess on failure and returns -1 */ int loadDirectory(const char* directoryPath, bool* externalSuccess) { @@ -455,6 +452,9 @@ namespace ammonite { shaderPaths.push_back(std::string(filePath)); } + //Ensure shaders don't get rebuilt due to a file order change + std::sort(shaderPaths.begin(), shaderPaths.end()); + //Create the program and return the ID return createProgram(&shaderPaths[0], shaderPaths.size(), externalSuccess); } From 31452b6ed80dc54762b7c2729934adcf88819c4d Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Fri, 5 Jul 2024 20:57:53 +0100 Subject: [PATCH 26/30] Debug log which shader is being compiled --- src/ammonite/graphics/shaders.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 2339e7db..90f80b8f 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -12,6 +12,7 @@ #include "../core/fileManager.hpp" #include "../core/threadManager.hpp" +#include "../utils/debug.hpp" #include "../utils/logging.hpp" #include "internal/internalExtensions.hpp" #include "internal/internalShaderValidator.hpp" @@ -205,6 +206,7 @@ namespace ammonite { return -1; } + ammoniteInternalDebug << "Compiling '" << shaderPath << "'" << std::endl; glShaderSource(shaderId, 1, &shaderCodePtr, (int*)&shaderCodeSize); glCompileShader(shaderId); delete [] shaderCodePtr; @@ -237,6 +239,7 @@ namespace ammonite { glDeleteShader(shaderIds[i]); } + //Check whether the program linked, log if relevant if (!checkProgram(programId)) { glDeleteProgram(programId); *externalSuccess = false; From 2abfe17d3178f1ed131680767168222935f48684 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Fri, 5 Jul 2024 22:53:21 +0100 Subject: [PATCH 27/30] Use external success pointer to test shader compilation success --- src/ammonite/graphics/shaders.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 90f80b8f..05b063cf 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -256,8 +256,9 @@ namespace ammonite { GLuint* shaderIds = new GLuint[shaderCount]; bool hasCreatedShaders = true; for (int i = 0; i < shaderCount; i++) { - shaderIds[i] = loadShader(shaderPaths[i], shaderTypes[i], externalSuccess); - if (shaderIds[i] == unsigned(-1)) { + bool passed = true; + shaderIds[i] = loadShader(shaderPaths[i], shaderTypes[i], &passed); + if (!passed) { hasCreatedShaders = false; break; } From 9d577c1b8eee959747224e21c6acdbfabc7a2876 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sat, 6 Jul 2024 00:47:44 +0100 Subject: [PATCH 28/30] Document shader caching support and 64-bit requirement --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index e83f5465..84409976 100644 --- a/README.md +++ b/README.md @@ -12,11 +12,13 @@ ## Features: - Model loading, using `libassimp` - Internal thread pool + - Shader program caching - Keyboard and mouse input handling ## Requirements: - A `c++23` compatible compiler (`g++ 12+` / `clang 17+`) - All build and runtime dependencies installed + - A 64-bit Linux system - An OpenGL 4.5+ compatible driver - Alternatively, an OpenGL 3.2+ driver supporting the following extensions can be used - `ARB_direct_state_access` From 1c883580b0dad16e49079438e5600b17981350b8 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 7 Jul 2024 01:47:33 +0100 Subject: [PATCH 29/30] Move cache writing and validation entirely within the file manager --- src/ammonite/core/fileManager.cpp | 218 +++++++++++++++++- src/ammonite/core/fileManager.hpp | 9 +- .../internal/internalShaderValidator.hpp | 28 --- src/ammonite/graphics/shaderValidator.cpp | 136 ----------- src/ammonite/graphics/shaders.cpp | 78 +++---- src/ammonite/types.hpp | 2 - 6 files changed, 249 insertions(+), 222 deletions(-) delete mode 100644 src/ammonite/graphics/internal/internalShaderValidator.hpp delete mode 100644 src/ammonite/graphics/shaderValidator.cpp diff --git a/src/ammonite/core/fileManager.cpp b/src/ammonite/core/fileManager.cpp index f0f76efa..8041aaab 100644 --- a/src/ammonite/core/fileManager.cpp +++ b/src/ammonite/core/fileManager.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -15,6 +16,8 @@ #include "../utils/debug.hpp" #include "../utils/logging.hpp" +#include "fileManager.hpp" + namespace ammonite { namespace files { namespace internal { @@ -50,6 +53,112 @@ namespace ammonite { return std::to_string(*(uint64_t*)output); } + + /* + - Similar to strtok_r, except it only supports single character delimiters + - Unsafe if called with both input and (save or *save) as nullptr / undefined + - save must never be nullptr + */ + static char* parseToken(char* input, char delim, char** save) { + //Restore from next byte to search + if (input == nullptr) { + input = *save; + } + + //Programmer error or no more tokens + if (*input == '\0') { + return nullptr; + } + + char* end = input; + //Search for delimiter or end of string + while (*end != delim && *end != '\0') { + end++; + } + + //Last token found + if (*end == '\0') { + return input; + } + + //Create a string, save state and return it + *end = '\0'; + *save = end + 1; + return input; + } + + //Check paths, times and file sizes are correct + static AmmoniteEnum validateInputs(std::string* filePaths, unsigned int fileCount, + unsigned char* extraData, std::size_t extraDataSize) { + unsigned char* extraDataCopy = new unsigned char[extraDataSize + 1]; + std::memcpy(extraDataCopy, extraData, extraDataSize); + extraDataCopy[extraDataSize] = '\0'; + + /* + - Decide whether the cache file can be used + - Uses input files, sizes and timestamps + */ + AmmoniteEnum result = AMMONITE_CACHE_INVALID; + char* state = nullptr; + unsigned int internalFileCount = 1; + long long int filesize = 0, modificationTime = 0; + char* token = parseToken((char*)extraDataCopy, '\n', &state); + do { + //Give up if token is null, we didn't find enough files + if (token == nullptr) { + break; + } + + //Check first token is 'input' + std::string currentFilePath = filePaths[internalFileCount - 1]; + char* nestedState = nullptr; + char* nestedToken = parseToken(token, ';', &nestedState); + if ((nestedToken == nullptr) || + (std::string(nestedToken) != std::string("input"))) { + break; + } + + //Check token matches shader path + nestedToken = parseToken(nullptr, ';', &nestedState); + if ((nestedToken == nullptr) || (std::string(nestedToken) != currentFilePath)) { + break; + } + + //Get filesize and time of last modification of the shader source + if (!ammonite::files::internal::getFileMetadata(currentFilePath, &filesize, + &modificationTime)) { + break; + } + + //Check token matches file size + nestedToken = parseToken(nullptr, ';', &nestedState); + if ((nestedToken == nullptr) || (std::atoll(nestedToken) != filesize)) { + break; + } + + //Check token matches timestamp + nestedToken = parseToken(nullptr, ';', &nestedState); + if ((nestedToken == nullptr) || (std::atoll(nestedToken) != modificationTime)) { + break; + } + + //Get the next line + if (fileCount > internalFileCount) { + token = parseToken(nullptr, '\n', &state); + } else { + result = AMMONITE_CACHE_HIT; + } + internalFileCount += 1; + } while (fileCount >= internalFileCount); + + delete [] extraDataCopy; + return result; + } + + static void deleteCacheFile(std::string filePath) { + ammonite::utils::status << "Clearing '" << filePath << "'" << std::endl; + deleteFile(filePath); + } } void deleteFile(std::string filePath) { @@ -226,14 +335,20 @@ namespace ammonite { } /* - - Attempt to read a cached file, and validate it with validator - - If the cache was valid, write to size and cacheState, then return cacheData + - Attempt to read a cached file, checking file paths, timestamps and file sizes + - If the cache was valid, write to dataSize and cacheState, then return cacheData - In this case, cacheData needs to be freed after use - If the cache was invalid write to cacheState and return nullptr - - In this case, cacheData and size should be ignored, and the cache will be cleared + - In this case, cache data, dataSize, *userData and userDataSize should be ignored, + and the cache will be cleared + - Write the address of any supplied user data to userData, and its size to userDataSize + - If userDataSize is 0, no user data was supplied + - *userData doesn't need to be freed, it's part of the return value's allocation */ - unsigned char* getCachedFile(std::string cacheFilePath, AmmoniteValidator validator, - std::size_t* size, AmmoniteEnum* cacheState, void* userPtr) { + unsigned char* getCachedFile(std::string cacheFilePath, std::string* filePaths, + unsigned int fileCount, std::size_t* dataSize, + unsigned char** userData, std::size_t* userDataSize, + AmmoniteEnum* cacheState) { //Check cache file exists if (!std::filesystem::exists(cacheFilePath)) { ammoniteInternalDebug << "Couldn't find " << cacheFilePath << std::endl; @@ -242,16 +357,44 @@ namespace ammonite { } //Attempt to read the cache if it exists, writes to size - unsigned char* cacheData = loadFile(cacheFilePath, size); - if (cacheData == nullptr) { + std::size_t size; + std::size_t blockSizes[3]; + unsigned char* cacheData = loadFile(cacheFilePath, &size); + if (cacheData == nullptr || size < sizeof(blockSizes)) { ammonite::utils::warning << "Failed to read '" << cacheFilePath << "'" << std::endl; *cacheState = AMMONITE_CACHE_MISS; + + //Cache data may or may not have been returned, due to size check + if (cacheData != nullptr) { + delete [] cacheData; + *cacheState = AMMONITE_CACHE_INVALID; + } return nullptr; } - //Validate the loaded cache - if (!validator(cacheData, *size, userPtr)) { - ammonite::utils::warning << "Failed to validate '" << cacheFilePath << "'" << std::endl; + //Get the sizes and start addresses of the data, user and extra blocks + std::memcpy(blockSizes, cacheData + size - sizeof(blockSizes), sizeof(blockSizes)); + *dataSize = blockSizes[0]; + *userData = cacheData + blockSizes[0]; + *userDataSize = blockSizes[1]; + unsigned char* extraData = *userData + *userDataSize; + std::size_t extraDataSize = blockSizes[2] - sizeof(blockSizes); + + //Check size of data is as expected, then validate the loaded cache + bool failed = false; + if (blockSizes[0] + blockSizes[1] + blockSizes[2] != size) { + ammonite::utils::warning << "Incorrect size information for '" << cacheFilePath \ + << "'" << std::endl; + failed = true; + } else if (validateInputs(filePaths, fileCount, extraData, extraDataSize) != + AMMONITE_CACHE_HIT) { + ammonite::utils::warning << "Failed to validate '" << cacheFilePath \ + << "'" << std::endl; + failed = true; + } + + //Clean up after a failure + if (failed) { ammonite::utils::status << "Clearing '" << cacheFilePath << "'" << std::endl; deleteFile(cacheFilePath); @@ -263,6 +406,61 @@ namespace ammonite { *cacheState = AMMONITE_CACHE_HIT; return cacheData; } + + /* + - Write dataSize bytes of data to cacheFilePath, using filePaths to generate the + cache information + - Also write userDataSize bytes of userData to the cache file + - Returns true on success, false on failure + */ + bool writeCacheFile(std::string cacheFilePath, std::string* filePaths, + unsigned int fileCount, unsigned char* data, std::size_t dataSize, + unsigned char* userData, std::size_t userDataSize) { + //Generate data to write + std::string extraData; + std::size_t blockSizes[3]; + for (unsigned int i = 0; i < fileCount; i++) { + long long int filesize = 0, modificationTime = 0; + ammonite::files::internal::getFileMetadata(filePaths[i], &filesize, &modificationTime); + extraData.append("input;" + filePaths[i]); + extraData.append(";" + std::to_string(filesize)); + extraData.append(";" + std::to_string(modificationTime) + "\n"); + } + + std::size_t extraSize = extraData.length(); + std::size_t totalDataSize = dataSize + userDataSize + extraSize + sizeof(blockSizes); + unsigned char* fileData = new unsigned char[totalDataSize]; + + //blockSize and its size gets special handling, as it's not written to extraData + blockSizes[0] = dataSize; + blockSizes[1] = userDataSize; + blockSizes[2] = extraSize + sizeof(blockSizes); + + /* + - Write the binary data, user data and cache info to the buffer + - The structure should be as follows: + - Binary cache data block + - User data block + - Extra data block (for path, timestamp and size validation) + - Includes sizes of each block + */ + std::memcpy(fileData, data, dataSize); + std::memcpy(fileData + dataSize, userData, userDataSize); + extraData.copy((char*)fileData + dataSize + userDataSize, extraSize, 0); + std::memcpy(fileData + dataSize + userDataSize + extraSize, blockSizes, + sizeof(blockSizes)); + + //Write the data, user data and cache info to the cache file + if (!ammonite::files::internal::writeFile(cacheFilePath, fileData, totalDataSize)) { + ammonite::utils::warning << "Failed to cache '" << cacheFilePath << "'" << std::endl; + deleteCacheFile(cacheFilePath); + delete [] fileData; + return false; + } + + delete [] fileData; + return true; + } } } } diff --git a/src/ammonite/core/fileManager.hpp b/src/ammonite/core/fileManager.hpp index 75831a54..66511391 100644 --- a/src/ammonite/core/fileManager.hpp +++ b/src/ammonite/core/fileManager.hpp @@ -20,8 +20,13 @@ namespace ammonite { unsigned char* loadFile(std::string filePath, std::size_t* size); bool writeFile(std::string filePath, unsigned char* data, std::size_t size); - unsigned char* getCachedFile(std::string cacheFilePath, AmmoniteValidator validator, - std::size_t* size, AmmoniteEnum* cacheState, void* userPtr); + unsigned char* getCachedFile(std::string cacheFilePath, std::string* filePaths, + unsigned int fileCount, std::size_t* dataSize, + unsigned char** userData, std::size_t* userDataSize, + AmmoniteEnum* cacheState); + bool writeCacheFile(std::string cacheFilePath, std::string* filePaths, + unsigned int fileCount, unsigned char* data, std::size_t dataSize, + unsigned char* userData, std::size_t userDataSize); } } } diff --git a/src/ammonite/graphics/internal/internalShaderValidator.hpp b/src/ammonite/graphics/internal/internalShaderValidator.hpp deleted file mode 100644 index d451d78d..00000000 --- a/src/ammonite/graphics/internal/internalShaderValidator.hpp +++ /dev/null @@ -1,28 +0,0 @@ -#ifndef INTERNALSHADERVALIDATOR -#define INTERNALSHADERVALIDATOR - -/* Internally exposed header: - - Allow shader caches to be validated -*/ - -#include -#include - -#include - -namespace ammonite { - namespace shaders { - namespace internal { - struct CacheInfo { - int fileCount; - std::string* filePaths; - GLenum binaryFormat; - GLsizei binaryLength; - }; - - bool validateCache(unsigned char* data, std::size_t size, void* userPtr); - } - } -} - -#endif diff --git a/src/ammonite/graphics/shaderValidator.cpp b/src/ammonite/graphics/shaderValidator.cpp deleted file mode 100644 index 0e90d792..00000000 --- a/src/ammonite/graphics/shaderValidator.cpp +++ /dev/null @@ -1,136 +0,0 @@ -#include -#include -#include -#include - -#include "../core/fileManager.hpp" -#include "internal/internalShaderValidator.hpp" - -namespace ammonite { - namespace shaders { - namespace internal { - namespace { - /* - - Similar to strtok_r, except it only supports single character delimiters - - Unsafe if called with both input and (save or *save) as nullptr / undefined - - save must never be nullptr - */ - static char* parseToken(char* input, char delim, char** save) { - //Restore from next byte to search - if (input == nullptr) { - input = *save; - } - - //Programmer error or no more tokens - if (*input == '\0') { - return nullptr; - } - - char* end = input; - //Search for delimiter or end of string - while (*end != delim && *end != '\0') { - end++; - } - - //Last token found - if (*end == '\0') { - return input; - } - - //Create a string, save state and return it - *end = '\0'; - *save = end + 1; - return input; - } - } - - bool validateCache(unsigned char* data, std::size_t size, void* userPtr) { - CacheInfo* cacheInfoPtr = (CacheInfo*)userPtr; - unsigned char* dataCopy = new unsigned char[size]; - std::memcpy(dataCopy, data, size); - dataCopy[size - 1] = '\0'; - - /* - - Decide whether the cache file can be used - - Uses input files, sizes and timestamps - */ - char* state = nullptr; - int fileCount = 1; - long long int filesize = 0, modificationTime = 0; - char* token = parseToken((char*)dataCopy, '\n', &state); - do { - //Give up if token is null, we didn't find enough files - if (token == nullptr) { - goto failed; - } - - //Check first token is 'input' - std::string currentFilePath = cacheInfoPtr->filePaths[fileCount - 1]; - char* nestedState = nullptr; - char* nestedToken = parseToken(token, ';', &nestedState); - if ((nestedToken == nullptr) || - (std::string(nestedToken) != std::string("input"))) { - goto failed; - } - - //Check token matches shader path - nestedToken = parseToken(nullptr, ';', &nestedState); - if ((nestedToken == nullptr) || (std::string(nestedToken) != currentFilePath)) { - goto failed; - } - - //Get filesize and time of last modification of the shader source - if (!ammonite::files::internal::getFileMetadata(currentFilePath, &filesize, - &modificationTime)) { - goto failed; - } - - //Check token matches file size - nestedToken = parseToken(nullptr, ';', &nestedState); - if ((nestedToken == nullptr) || (std::atoll(nestedToken) != filesize)) { - goto failed; - } - - //Check token matches timestamp - nestedToken = parseToken(nullptr, ';', &nestedState); - if ((nestedToken == nullptr) || (std::atoll(nestedToken) != modificationTime)) { - goto failed; - } - - //Get the next line - if (cacheInfoPtr->fileCount > fileCount) { - token = parseToken(nullptr, '\n', &state); - } - fileCount += 1; - } while (cacheInfoPtr->fileCount >= fileCount); - - //Find binary format - token = parseToken(nullptr, '\n', &state); - if (token == nullptr) { - goto failed; - } - cacheInfoPtr->binaryFormat = std::atoll(token); - - //Find binary length - token = parseToken(nullptr, '\n', &state); - if (token == nullptr) { - goto failed; - } - cacheInfoPtr->binaryLength = std::atoll(token); - - //Check fetched binary info - if (cacheInfoPtr->binaryFormat != 0 && cacheInfoPtr->binaryLength != 0) { - //Sanity check binary size - if ((std::size_t)cacheInfoPtr->binaryLength < size && cacheInfoPtr->binaryLength > 0) { - delete [] dataCopy; - return true; - } - } - -failed: - delete [] dataCopy; - return false; - } - } - } -} diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 05b063cf..7897f54a 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -15,7 +16,6 @@ #include "../utils/debug.hpp" #include "../utils/logging.hpp" #include "internal/internalExtensions.hpp" -#include "internal/internalShaderValidator.hpp" namespace ammonite { //Static helper functions and anonymous data @@ -29,52 +29,28 @@ namespace ammonite { int shaderCount; std::string* shaderPaths; std::string cacheFilePath; - GLenum binaryFormat; int binaryLength; + GLenum binaryFormat; char* binaryData; }; } - static void deleteCacheFile(std::string cacheFilePath) { - //Delete the cache file - ammonite::utils::status << "Clearing '" << cacheFilePath << "'" << std::endl; - ammonite::files::internal::deleteFile(cacheFilePath); - } - //Thread pool work to cache a program static void doCacheWork(void* userPtr) { CacheWorkerData* data = (CacheWorkerData*)userPtr; - //Generate cache info to write - std::string extraData; - for (int i = 0; i < data->shaderCount; i++) { - long long int filesize = 0, modificationTime = 0; - ammonite::files::internal::getFileMetadata(data->shaderPaths[i], &filesize, - &modificationTime); - - extraData.append("input;" + data->shaderPaths[i]); - extraData.append(";" + std::to_string(filesize)); - extraData.append(";" + std::to_string(modificationTime) + "\n"); - } - - extraData.append(std::to_string(data->binaryFormat) + "\n"); - extraData.append(std::to_string(data->binaryLength) + "\n"); + //Prepare user data required to load the cache again + std::string userData = std::to_string(data->binaryFormat) + "\n"; + std::size_t userDataSize = userData.length(); + unsigned char* userBuffer = new unsigned char[userDataSize]; + userData.copy((char*)userBuffer, userDataSize, 0); - int extraLength = extraData.length(); - char* fileData = new char[extraLength + data->binaryLength]; + //Write the cache file, failure messages are also handled by it + ammonite::files::internal::writeCacheFile(data->cacheFilePath, data->shaderPaths, + data->shaderCount, (unsigned char*)data->binaryData, data->binaryLength, userBuffer, + userDataSize); - //Write the cache info and binary data to the buffer - extraData.copy(fileData, extraLength, 0); - std::memcpy(fileData + extraLength, data->binaryData, data->binaryLength); - - //Write the cache info and data to the cache file - if (!ammonite::files::internal::writeFile(data->cacheFilePath, (unsigned char*)fileData, - extraLength + data->binaryLength)) { - ammonite::utils::warning << "Failed to cache '" << data->cacheFilePath << "'" << std::endl; - deleteCacheFile(data->cacheFilePath); - } - - delete [] fileData; + delete [] userBuffer; delete [] data->binaryData; delete [] data->shaderPaths; delete data; @@ -109,7 +85,7 @@ namespace ammonite { return; } - //Pack data for writing program cache + //Pack shader paths into worker data data->shaderPaths = new std::string[shaderCount]; for (int i = 0; i < shaderCount; i++) { data->shaderPaths[i] = shaderPaths[i]; @@ -299,21 +275,34 @@ namespace ammonite { int programId; std::string cacheFilePath; if (isCacheSupported) { + unsigned char* userData; std::size_t cacheDataSize; + std::size_t userDataSize; AmmoniteEnum cacheState; //Attempt to load the cached program - ammonite::shaders::internal::CacheInfo cacheInfo = {shaderCount, shaderPaths, 0, 0}; - cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, - shaderCount); + cacheFilePath = ammonite::files::internal::getCachedFilePath(shaderPaths, shaderCount); unsigned char* cacheData = ammonite::files::internal::getCachedFile(cacheFilePath, - ammonite::shaders::internal::validateCache, &cacheDataSize, &cacheState, &cacheInfo); - unsigned char* dataStart = cacheData + cacheDataSize - cacheInfo.binaryLength; + shaderPaths, shaderCount, &cacheDataSize, &userData, &userDataSize, &cacheState); + + //Fetch and validate binary format + GLenum binaryFormat = 0; + if (cacheState == AMMONITE_CACHE_HIT && userDataSize != 0) { + long long rawFormat = std::atoll((char*)userData); + binaryFormat = rawFormat; + + if (binaryFormat == 0) { + ammonite::utils::warning << "Failed to get binary format for cached program" \ + << std::endl; + cacheState = AMMONITE_CACHE_INVALID; + delete [] cacheData; + } + } if (cacheState == AMMONITE_CACHE_HIT) { //Load the cached binary data into a program programId = glCreateProgram(); - glProgramBinary(programId, cacheInfo.binaryFormat, dataStart, cacheInfo.binaryLength); + glProgramBinary(programId, binaryFormat, cacheData, cacheDataSize); delete [] cacheData; //Return the program ID, unless the cache was faulty, then delete and carry on @@ -322,8 +311,9 @@ namespace ammonite { } else { ammonite::utils::warning << "Failed to process '" << cacheFilePath \ << "'" << std::endl; + ammonite::utils::status << "Clearing '" << cacheFilePath << "'" << std::endl; glDeleteProgram(programId); - deleteCacheFile(cacheFilePath); + ammonite::files::internal::deleteFile(cacheFilePath); } } } diff --git a/src/ammonite/types.hpp b/src/ammonite/types.hpp index 26a51cc9..8c293dfc 100644 --- a/src/ammonite/types.hpp +++ b/src/ammonite/types.hpp @@ -1,10 +1,8 @@ #ifndef TYPES #define TYPES -#include #include -typedef bool (*AmmoniteValidator)(unsigned char* data, std::size_t size, void* userPtr); typedef void (*AmmoniteKeyCallback)(std::vector keycodes, int action, void* userPtr); typedef void (*AmmoniteWork)(void* userPtr); From b45a5c32d536c74cc1df70fbb7883dd419fc6d84 Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Sun, 7 Jul 2024 01:55:43 +0100 Subject: [PATCH 30/30] Share shader extension and string identification tables --- src/ammonite/graphics/shaders.cpp | 41 ++++++++++++++----------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/ammonite/graphics/shaders.cpp b/src/ammonite/graphics/shaders.cpp index 7897f54a..46c3daeb 100644 --- a/src/ammonite/graphics/shaders.cpp +++ b/src/ammonite/graphics/shaders.cpp @@ -24,6 +24,19 @@ namespace ammonite { //Set by updateCacheSupport(), when GLEW loads bool isBinaryCacheSupported = false; + //Identify shader types by extensions / contained strings + std::map shaderMatches = { + {".vert", GL_VERTEX_SHADER}, {".vs", GL_VERTEX_SHADER}, {"vert", GL_VERTEX_SHADER}, + {".frag", GL_FRAGMENT_SHADER}, {".fs", GL_FRAGMENT_SHADER}, {"frag", GL_FRAGMENT_SHADER}, + {".geom", GL_GEOMETRY_SHADER}, {".gs", GL_GEOMETRY_SHADER}, {"geom", GL_GEOMETRY_SHADER}, + {".tessc", GL_TESS_CONTROL_SHADER}, {".tsc", GL_TESS_CONTROL_SHADER}, + {"tessc", GL_TESS_CONTROL_SHADER}, {"control", GL_TESS_CONTROL_SHADER}, + {".tesse", GL_TESS_EVALUATION_SHADER}, {".tes", GL_TESS_EVALUATION_SHADER}, + {"tesse", GL_TESS_EVALUATION_SHADER}, {"eval", GL_TESS_EVALUATION_SHADER}, + {".comp", GL_COMPUTE_SHADER}, {".cs", GL_COMPUTE_SHADER}, {"compute", GL_COMPUTE_SHADER}, + {".glsl", GL_FALSE} //Don't ignore generic shaders + }; + //Data required by cache worker struct CacheWorkerData { int shaderCount; @@ -140,15 +153,6 @@ namespace ammonite { } static GLenum attemptIdentifyShaderType(std::string shaderPath) { - std::map shaderMatches = { - {"vert", GL_VERTEX_SHADER}, - {"frag", GL_FRAGMENT_SHADER}, - {"geom", GL_GEOMETRY_SHADER}, - {"tessc", GL_TESS_CONTROL_SHADER}, {"control", GL_TESS_CONTROL_SHADER}, - {"tesse", GL_TESS_EVALUATION_SHADER}, {"eval", GL_TESS_EVALUATION_SHADER}, - {"compute", GL_COMPUTE_SHADER} - }; - std::string lowerShaderPath; for (unsigned int i = 0; i < shaderPath.size(); i++) { lowerShaderPath += std::tolower(shaderPath[i]); @@ -157,7 +161,9 @@ namespace ammonite { //Try and match the filename to a supported shader for (auto it = shaderMatches.begin(); it != shaderMatches.end(); it++) { if (lowerShaderPath.contains(it->first)) { - return it->second; + if (it->second != GL_FALSE) { + return it->second; + } } } @@ -359,17 +365,6 @@ namespace ammonite { */ int createProgram(std::string* inputShaderPaths, int inputShaderCount, bool* externalSuccess) { - //Convert file extensions to shader types - std::map shaderExtensions = { - {".vert", GL_VERTEX_SHADER}, {".vs", GL_VERTEX_SHADER}, - {".frag", GL_FRAGMENT_SHADER}, {".fs", GL_FRAGMENT_SHADER}, - {".geom", GL_GEOMETRY_SHADER}, {".gs", GL_GEOMETRY_SHADER}, - {".tessc", GL_TESS_CONTROL_SHADER}, {".tsc", GL_TESS_CONTROL_SHADER}, - {".tesse", GL_TESS_EVALUATION_SHADER}, {".tes", GL_TESS_EVALUATION_SHADER}, - {".comp", GL_COMPUTE_SHADER}, {".cs", GL_COMPUTE_SHADER}, - {".glsl", GL_FALSE} //Detect generic shaders, attempt to identify further - }; - //Find all shaders std::vector shaderPaths(0); std::vector shaderTypes(0); @@ -377,8 +372,8 @@ namespace ammonite { std::filesystem::path filePath{inputShaderPaths[i]}; std::string extension = filePath.extension(); - if (shaderExtensions.contains(extension)) { - GLenum shaderType = shaderExtensions[extension]; + if (shaderMatches.contains(extension)) { + GLenum shaderType = shaderMatches[extension]; //Shader can't be identified through extension, use filename if (shaderType == GL_FALSE) {