From 1dba3d2d638cd1dd112bdb19f973d89430c7b169 Mon Sep 17 00:00:00 2001 From: Kevin Trogant Date: Fri, 13 Oct 2023 23:15:23 +0200 Subject: [PATCH] fix, feat: various issues - [win32] FIO thread no longer hangs during application exit - Signal if a file operation was actually successfull. - Add a logging function (currently identical to vyReportError) --- include/fio.h | 11 ++++ include/voyage.h | 2 + meson.build | 11 +++- src/error_report.c | 9 +++ src/fio.c | 37 ++++++++--- src/gfx_shader_loading.c | 132 +++++++++++++++++++++++++-------------- 6 files changed, 144 insertions(+), 58 deletions(-) diff --git a/include/fio.h b/include/fio.h index 7b41767..71fd3e7 100644 --- a/include/fio.h +++ b/include/fio.h @@ -7,11 +7,22 @@ #include "voyage.h" +enum +{ + VY_FILE_BUFFER_FLAG_FILE_NOT_FOUND = 0x1, + VY_FILE_BUFFER_FLAG_READ_FAILED = 0x2, +}; + typedef struct { void *data; size_t size; + uint32_t flags; } vy_file_buffer; +inline bool vyWasFileBufferSuccessful(const vy_file_buffer *fb) { + return fb->flags == 0; +} + /* used to identify a file (XXH3 hash of the path) */ typedef uint64_t vy_file_id; diff --git a/include/voyage.h b/include/voyage.h index 5e91ccd..f66b2e9 100644 --- a/include/voyage.h +++ b/include/voyage.h @@ -14,4 +14,6 @@ typedef struct { void vyReportError(const char *subsystem, const char *fmt, ...); +void vyLog(const char *subsystem, const char *fmt, ...); + #endif diff --git a/meson.build b/meson.build index c948746..b31120e 100644 --- a/meson.build +++ b/meson.build @@ -1,5 +1,5 @@ project('voyage', 'c', - default_options: ['buildtype=debug', 'b_sanitize=address', 'c_std=c17', 'warning_level=3']) + default_options: ['buildtype=debugoptimized', 'b_sanitize=address', 'c_std=c17', 'warning_level=3']) compiler = meson.get_compiler('c') buildtype = get_option('buildtype') @@ -13,7 +13,7 @@ if compiler.get_argument_syntax() == 'gcc' ) elif compiler.get_argument_syntax() == 'msvc' add_project_arguments( - ['/wd4146', '/wd4245', '/wd4100', '/D_CRT_SECURE_NO_WARNINGS', '/RTcsu'], + ['/wd4146', '/wd4245', '/wd4100', '/D_CRT_SECURE_NO_WARNINGS', '/RTCsu'], language: 'c' ) endif @@ -30,13 +30,18 @@ add_project_arguments([ '-DRENDERER_GL'], language : 'c') # Gather dependencies thread_dep = dependency('threads') m_dep = compiler.find_library('m', required : false) -glfw_proj = subproject('glfw') +glfw_proj = subproject('glfw', default_options : 'default_library=static') glfw_dep = glfw_proj.get_variable('glfw_dep') incdir = include_directories(['contrib', 'include']) executable('voyage', # Project Sources + 'include/voyage.h', + 'include/gfx.h', + 'include/gfx_backend.h', + 'include/fio.h', + 'src/voyage.c', 'src/fio.c', 'src/error_report.c', diff --git a/src/error_report.c b/src/error_report.c index 0bb9606..85f96a1 100644 --- a/src/error_report.c +++ b/src/error_report.c @@ -13,3 +13,12 @@ void vyReportError(const char *subsystem, const char *fmt, ...) { fprintf(stderr, "\n"); va_end(ap); } + +void vyLog(const char *subsystem, const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + fprintf(stderr, "[%s] ", subsystem); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n"); + va_end(ap); +} \ No newline at end of file diff --git a/src/fio.c b/src/fio.c index 55cacd2..3641c42 100644 --- a/src/fio.c +++ b/src/fio.c @@ -153,12 +153,13 @@ bool vyInitFIO(const vy_fio_config *config) { if (pthread_create(&_thread, NULL, linuxFIOThreadProc, NULL) != 0) return false; #elif defined(_WIN32) - _fio_term_event = CreateEvent(NULL, TRUE, FALSE, NULL); + _fio_term_event = CreateEventW(NULL, FALSE, FALSE, NULL); if (!_fio_term_event) return false; _fio_thread = CreateThread(NULL, 0, win32FIOThreadProc, NULL, 0, NULL); if (!_fio_thread) return false; + SetThreadDescription(_fio_thread, L"FIO Thread"); #endif return true; @@ -169,8 +170,14 @@ void vyShutdownFIO(void) { pthread_cancel(_thread); pthread_join(_thread, NULL); #elif defined(_WIN32) - WaitForSingleObject(_fio_thread, INFINITE); - CloseHandle(_fio_thread); + if (SetEvent(_fio_term_event)) { + WakeAllConditionVariable(&_queue.pending_cond); + WaitForSingleObject(_fio_thread, INFINITE); + CloseHandle(_fio_thread); + CloseHandle(_fio_term_event); + } else { + vyReportError("FIO", "Failed to signal the termination event."); + } #endif ShutdownFIOQueue(); ShutdownFileTab(); @@ -374,6 +381,7 @@ bool vyRetrieveReadBuffer(vy_fio_handle fio, vy_file_buffer *buffer) { return false; buffer->size = sz; memcpy(buffer->data, _queue.ops[fio - 1].buffer.data, sz); + buffer->flags = _queue.ops[fio - 1].buffer.flags; _queue.ops[fio - 1].flags |= FLAGS_RETRIEVED; } @@ -399,10 +407,12 @@ static void ProcessRead(vy_file_op *op) { op->buffer.data = NULL; op->buffer.size = 0; + op->buffer.flags = 0; FILE *file = fopen(path, "rb"); if (!file) { op->flags |= FLAGS_FINISHED; + op->buffer.flags = VY_FILE_BUFFER_FLAG_FILE_NOT_FOUND; return; } fseek(file, 0, SEEK_END); @@ -415,6 +425,7 @@ static void ProcessRead(vy_file_op *op) { free(op->buffer.data); op->buffer.data = NULL; op->buffer.size = 0; + op->buffer.flags = VY_FILE_BUFFER_FLAG_READ_FAILED; } fclose(file); @@ -438,17 +449,29 @@ static void *linuxFIOThreadProc(void *_param) { static DWORD WINAPI win32FIOThreadProc(_In_ LPVOID lpParam) { VY_UNUSED(lpParam); - while (WaitForSingleObject(&_fio_term_event, 0) != WAIT_OBJECT_0) { + bool keep_running = true; + while (keep_running) { EnterCriticalSection(&_queue.critical_section); - while (_queue.write_pos == _queue.read_pos) { + while (_queue.read_pos == _queue.write_pos && keep_running) { SleepConditionVariableCS(&_queue.pending_cond, &_queue.critical_section, INFINITE); + DWORD wfs = WaitForSingleObject(_fio_term_event, 0); + if (wfs != WAIT_FAILED) { + keep_running = wfs != WAIT_OBJECT_0; + } else { + vyLog("FIO", "ThreadProc wait error: %d", GetLastError()); + keep_running = false; + } + } + /* It's possible that we were awoken during application shutdown. */ + if (_queue.write_pos != _queue.read_pos) { + ProcessRead(&_queue.ops[_queue.read_pos]); + _queue.read_pos = (_queue.read_pos + 1) % _queue.size; } - ProcessRead(&_queue.ops[_queue.read_pos]); - _queue.read_pos = (_queue.read_pos + 1) % _queue.size; LeaveCriticalSection(&_queue.critical_section); } + vyLog("FIO", "Exit FIO thread"); return 0; } #endif \ No newline at end of file diff --git a/src/gfx_shader_loading.c b/src/gfx_shader_loading.c index eee2334..e6eb65e 100644 --- a/src/gfx_shader_loading.c +++ b/src/gfx_shader_loading.c @@ -54,7 +54,7 @@ static bool IsAllowedChar(char c) { } static bool IsWhitespace(char c) { - return c == ' ' || c == '\t' || c == '\n'; + return c == ' ' || c == '\t' || c == '\n' || c == '\r'; } static void SkipWhitespace(vy_parse_state *state) { @@ -233,7 +233,7 @@ static void DbgPrintShaderFile(const vy_parse_state *state, } stmt_index = stmt->next; } - assert(stmt_index = UINT_MAX); + assert(stmt_index == UINT_MAX); } static bool CompareSpanToString(vy_text_span span, const char *cmp) { @@ -315,7 +315,7 @@ static vy_gfx_pipeline_id LinkProgram(vy_parse_state *state, /* wait */ } vy_file_buffer compute_code; - if (!vyRetrieveReadBuffer(compute_read, &compute_code)) { + if (!vyRetrieveReadBuffer(compute_read, &compute_code) || !vyWasFileBufferSuccessful(&compute_code)) { vyReportError("GFX", "Failed to load compute shader required by: %s", file_path); @@ -356,7 +356,7 @@ static vy_gfx_pipeline_id LinkProgram(vy_parse_state *state, int remaining = 2; while (remaining > 0) { if (vyIsFIOFinished(vertex_read)) { - if (!vyRetrieveReadBuffer(vertex_read, &vertex_code)) { + if (!vyRetrieveReadBuffer(vertex_read, &vertex_code) || !vyWasFileBufferSuccessful(&vertex_code)) { vyReportError( "GFX", "Failed to load vertex shader required by: %s", @@ -371,7 +371,7 @@ static vy_gfx_pipeline_id LinkProgram(vy_parse_state *state, } if (vyIsFIOFinished(fragment_read)) { - if (!vyRetrieveReadBuffer(fragment_read, &fragment_code)) { + if (!vyRetrieveReadBuffer(fragment_read, &fragment_code) || !vyWasFileBufferSuccessful(&fragment_code)) { vyReportError( "GFX", "Failed to load fragment shader required by: %s", @@ -435,6 +435,59 @@ static vy_attribute_value ParseBindingValue(vy_text_span span) { return VY_ATTRIBUTE_VALUE_UNDEFINED; } +static bool ParseBindings(vy_parse_state *state, + unsigned int root_list, + const char *name, + const char *file_path, + vy_attribute_binding **p_bindings, + unsigned int *p_binding_count) { + const vy_parsed_stmt *bindings = FindStatement(state, root_list, name); + if (bindings) { + if (bindings->form != VY_STMT_FORM_LIST) { + vyReportError("GFX", + "Expected list of bindings as the value of " + "\"%s\" in %s", + name, + file_path); + return false; + } + const vy_parsed_stmt_list *binding_list = + &state->statement_lists[bindings->list_index]; + vy_attribute_binding *shader_bindings = + malloc(sizeof(vy_attribute_binding) * binding_list->count); + if (!bindings) { + vyReportError("GFX", "Out of memory"); + return false; + } + unsigned int binding_count = binding_list->count; + + unsigned int stmt_index = binding_list->first; + for (unsigned int i = 0; i < binding_list->count; ++i) { + const vy_parsed_stmt *stmt = &state->statements[stmt_index]; + if (!ParseBindingIndex(stmt->attribute, + &shader_bindings[i].index)) { + free(shader_bindings); + return false; + } + + shader_bindings[i].value = ParseBindingValue(stmt->value); + if (shader_bindings[i].value == VY_ATTRIBUTE_VALUE_UNDEFINED) { + free(shader_bindings); + return false; + } + stmt_index = stmt->next; + } + + *p_bindings = shader_bindings; + *p_binding_count = binding_count; + return true; + } else { + *p_bindings = NULL; + *p_binding_count = 0; + return true; + } +} + static bool ParseShaderFile(vy_file_id fid, vy_file_buffer fbuf, vy_shader *shader) { /* This is the grammar for shader files: @@ -476,52 +529,35 @@ ParseShaderFile(vy_file_id fid, vy_file_buffer fbuf, vy_shader *shader) { shader->storage_bindings = NULL; shader->storage_binding_count = 0; - const vy_parsed_stmt *texture_bindings = - FindStatement(&state, root_list, "texture_bindings"); - if (texture_bindings) { - if (texture_bindings->form != VY_STMT_FORM_LIST) { - vyReportError("GFX", - "Expected list of bindings as the value of " - "\"texture_bindings\" in %s", - file_path); - goto out; - } - const vy_parsed_stmt_list *binding_list = - &state.statement_lists[texture_bindings->list_index]; - shader->texture_bindings = - malloc(sizeof(vy_attribute_binding) * binding_list->count); - if (!shader->texture_bindings) { - vyReportError("GFX", "Out of memory"); - goto out; - } - shader->texture_binding_count = binding_list->count; - - unsigned int stmt_index = binding_list->first; - for (unsigned int i = 0; i < binding_list->count; ++i) { - const vy_parsed_stmt *stmt = &state.statements[stmt_index]; - if (!ParseBindingIndex(stmt->attribute, - &shader->texture_bindings[i].index)) { - free(shader->texture_bindings); - result = false; - goto out; - } - - shader->texture_bindings[i].value = ParseBindingValue(stmt->value); - if (shader->texture_bindings[i].value == - VY_ATTRIBUTE_VALUE_UNDEFINED) { - free(shader->texture_bindings); - result = false; - goto out; - } - stmt_index = stmt->next; - } + if (!ParseBindings(&state, + root_list, + "texture_bindings", + file_path, + &shader->texture_bindings, + &shader->texture_binding_count)) { + result = false; + goto out; } - const vy_parsed_stmt *uniform_bindings = - FindStatement(&state, root_list, "uniform_bindings"); + if (!ParseBindings(&state, + root_list, + "uniform_bindings", + file_path, + &shader->uniform_bindings, + &shader->uniform_binding_count)) { + result = false; + goto out; + } - const vy_parsed_stmt *storage_bindings = - FindStatement(&state, root_list, "storage_bindings"); + if (!ParseBindings(&state, + root_list, + "storage_bindings", + file_path, + &shader->storage_bindings, + &shader->storage_binding_count)) { + result = false; + goto out; + } out: free(state.statements);