From 667589f1f62331b1ad1846cd8807eb5bc31c1daa Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 16 Jun 2026 08:49:31 +0200 Subject: [PATCH] Move Win32 async context ownership and trim canvas draw setup --- docs/modernization/debt.md | 11 +++ docs/modernization/roadmap.md | 22 ++--- docs/modernization/tasks.md | 8 ++ src/legacy_canvas_draw_merge_services.h | 39 +++++++++ src/main.cpp | 82 ++++++------------- src/node_canvas.cpp | 34 ++------ .../windows_platform_services.cpp | 78 ++++++++++++++++++ 7 files changed, 181 insertions(+), 93 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 690f331a..60d8ca7c 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,17 @@ agent or engineer to remove them without reconstructing context from chat. ## Reductions +- 2026-06-16: `DEBT-0003` was narrowed again. The Win32 async GL/context lock + state now lives in `src/platform_windows/windows_platform_services.cpp` + instead of `src/main.cpp` retained state, and `main.cpp` only seeds the + created `HDC`/`HGLRC` into that platform-owned helper during initialization + and context recreation; broader Win32 bootstrap/runtime ownership remains. +- 2026-06-16: `DEBT-0036` was narrowed again. `NodeCanvas` merged-path + per-plane merged-texture draw callback setup now routes through + `make_legacy_canvas_draw_merge_layer_texture_draw(...)` in + `src/legacy_canvas_draw_merge_services.h` instead of keeping that callback + setup inline in `NodeCanvas::draw()`; broader canvas draw orchestration and + retained GL resource ownership remain. - 2026-06-16: `DEBT-0037` was narrowed again. `App::rec_loop()` in `src/app.cpp` now delegates recording worker iteration orchestration into `process_legacy_recording_worker_iteration(...)` in diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 22ec169c..6293bcda 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -117,9 +117,9 @@ Current architecture mismatches that must be treated as real blockers: checkerboard background setup now route through retained draw-merge helpers, with the cache-to-screen checkerboard-plane callback setup also reduced and the merged-path checkerboard background-plane callback plus per-plane - merged-texture draw plus the smoothing-mask face shader/draw pass plus - heightmap, current-mode, and grid-mode callback setup now routed through the - same retained helper family. + merged-texture draw callback plus the smoothing-mask face shader/draw pass + plus heightmap, current-mode, and grid-mode callback setup now routed + through the same retained helper family. - `app_layout.cpp` and `app_dialogs.cpp` are still mixed shell/controller files rather than thin composition/binding surfaces. - `App`, `Canvas`, `Node`, retained workers, and platform entrypoints still use @@ -135,13 +135,15 @@ Current architecture mismatches that must be treated as real blockers: coordination flags now use `std::atomic` instead of unsynchronized globals, while the main Win32 entrypoint now groups window/GL/task/VR state behind a retained local state object instead of separate process-wide globals, the - canvas async worker now sits behind a named retained local worker-state - helper instead of a bare static accessor, the prepared-file worker and the - canvas async import/export/save/open worker now live under `AppRuntime` - instead of retained static app-events/canvas workers, and `App::rec_loop()` - now delegates worker-iteration orchestration into the retained recording - bridge even though that retained recording path still owns the worker-side - readback flow. + Win32 async GL/context lock state now lives under + `src/platform_windows/windows_platform_services.cpp` instead of `main.cpp` + retained state, the canvas async worker now sits behind a named retained + local worker-state helper instead of a bare static accessor, the + prepared-file worker and the canvas async import/export/save/open worker now + live under `AppRuntime` instead of retained static app-events/canvas + workers, and `App::rec_loop()` now delegates worker-iteration orchestration + into the retained recording bridge even though that retained recording path + still owns the worker-side readback flow. - Modern C++23 usage exists in extracted components, especially `std::span`, explicit result/status objects, and a few concepts, but the live app still does not consistently express ownership, thread affinity, or renderer diff --git a/docs/modernization/tasks.md b/docs/modernization/tasks.md index 549a46e0..4ad8cb8f 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -155,6 +155,10 @@ Current slice: through `make_legacy_canvas_draw_merge_background_checkerboard_plane(...)`, but the node still owns broader live layer traversal and renderer-state sequencing. +- `NodeCanvas` merged-path per-plane merged-texture draw callback setup now + also routes through `make_legacy_canvas_draw_merge_layer_texture_draw(...)`, + but the node still owns broader live layer traversal and renderer-state + sequencing. - `NodeCanvas` smoothing-mask face shader setup plus per-face draw execution now also route through `execute_legacy_canvas_draw_merge_smask_faces(...)`, but the node still owns @@ -356,6 +360,10 @@ Current slice: - `main.cpp` Win32 window handles, GL task/mutex state, splash-dialog state, stylus timers, and VR worker state now sit behind one retained local state object instead of separate file-scope globals +- Win32 async GL/context lock state now lives in + `src/platform_windows/windows_platform_services.cpp` instead of `main.cpp` + retained state, and `main.cpp` only seeds that platform-owned context handle + pair during initialization and context recreation - prepared-file background work now runs through an `AppRuntime`-owned worker queue instead of a retained static worker in `src/app_events.cpp` - canvas async import/export/save/open background work now also runs through an diff --git a/src/legacy_canvas_draw_merge_services.h b/src/legacy_canvas_draw_merge_services.h index 755de7bb..b3b5aea3 100644 --- a/src/legacy_canvas_draw_merge_services.h +++ b/src/legacy_canvas_draw_merge_services.h @@ -494,6 +494,45 @@ inline void execute_legacy_canvas_draw_merge_layer_texture( execution.unbind_layer_texture(); } +template +[[nodiscard]] inline auto make_legacy_canvas_draw_merge_layer_texture_draw( + LayerMergeT* layer_merge, + SamplerT* sampler, + FacePlaneT* face_plane, + SetActiveTextureUnit set_active_texture_unit, + glm::mat4 proj, + glm::mat4 camera, + PlaneTransform plane_transform) +{ + return [layer_merge, sampler, face_plane, set_active_texture_unit, proj, camera, plane_transform](int plane_index) { + const auto mvp = proj * camera * + plane_transform[plane_index] * + glm::translate(glm::vec3(0, 0, -1)); + execute_legacy_canvas_draw_merge_layer_texture( + { + .mvp = mvp, + .texture_slot = 0, + .alpha = 1.f, + .highlight = false, + }, + { + .bind_sampler = [sampler, set_active_texture_unit] { + sampler->bind(0); + set_active_texture_unit(0); + }, + .bind_layer_texture = [layer_merge, plane_index] { + layer_merge->rtt(plane_index).bindTexture(); + }, + .draw = [face_plane] { + face_plane->draw_fill(); + }, + .unbind_layer_texture = [layer_merge, plane_index] { + layer_merge->rtt(plane_index).unbindTexture(); + }, + }); + }; +} + inline void execute_legacy_canvas_draw_merge_layer_composite( bool is_temporary_erase, bool is_temporary_paint, diff --git a/src/main.cpp b/src/main.cpp index f8cc2a74..ffc5eeb5 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -38,21 +38,22 @@ #define WM_USER_WAKEUP (WM_USER + 2) LRESULT CALLBACK WndProc(HWND hWnd, UINT msg, WPARAM wp, LPARAM lp); +namespace pp::platform::windows { + void set_async_render_context(HDC hdc, HGLRC hrc); + void lock_async_render_context(); + bool try_lock_async_render_context(); + void unlock_async_render_context(); + void swap_async_render_context(); +} struct RetainedState { HINSTANCE hInst{}; HWND hWnd{}; - HDC hDC{}; - HGLRC hRC{}; const wchar_t* className{}; bool keys[256]{}; - std::mutex gl_mutex; - std::mutex async_mutex; - std::thread::id gl_thread{}; std::map vkey_map; wchar_t window_title[512]{}; std::jthread hmd_renderer; - int gl_count = 0; std::deque> main_tasklist; std::mutex main_task_mutex; float timer_stylus = 0; @@ -170,57 +171,22 @@ void destroy_window() void async_lock() { - //std::lock_guard _lock(async_mutex); - auto& state = retained_state(); - if (state.gl_count == 0 || state.gl_thread != std::this_thread::get_id()) - { - state.gl_mutex.lock(); - bool ret = wglMakeCurrent(state.hDC, state.hRC); - if (ret == false) - LOG("FAILED wglMakeCurrent: %s", GetLastErrorAsString().c_str()); - state.gl_thread = std::this_thread::get_id(); - //LOG("lock"); - } - state.gl_count++; - //assert(ret == true); + pp::platform::windows::lock_async_render_context(); } bool async_lock_try() { - //std::lock_guard _lock(async_mutex); - auto& state = retained_state(); - if (state.gl_count == 0 || state.gl_thread != std::this_thread::get_id()) - { - if (!state.gl_mutex.try_lock()) - return false; - bool ret = wglMakeCurrent(state.hDC, state.hRC); - if (ret == false) - LOG("FAILED wglMakeCurrent: %s", GetLastErrorAsString().c_str()); - state.gl_thread = std::this_thread::get_id(); - //LOG("lock"); - } - state.gl_count++; - //assert(ret == true); - return true; + return pp::platform::windows::try_lock_async_render_context(); } void win32_async_swap() { - SwapBuffers(retained_state().hDC); - //LOG("swap"); + pp::platform::windows::swap_async_render_context(); } void async_unlock() { - //std::lock_guard _lock(async_mutex); - auto& state = retained_state(); - state.gl_count--; - if (state.gl_count == 0) - { - //LOG("unlock"); - wglMakeCurrent(0, 0); - state.gl_mutex.unlock(); - } + pp::platform::windows::unlock_async_render_context(); } void win32_update_stylus(float dt) @@ -866,11 +832,12 @@ int main(int argc, char** argv) pfd.cDepthBits = 24; pfd.iLayerType = PFD_MAIN_PLANE; - state.hDC = GetDC(state.hWnd); - int pxfmt = ChoosePixelFormat(state.hDC, &pfd); - SetPixelFormat(state.hDC, pxfmt, &pfd); - state.hRC = wglCreateContext(state.hDC); // Create OpenGL 2.1 or less - wglMakeCurrent(state.hDC, state.hRC); + HDC hDC = GetDC(state.hWnd); + int pxfmt = ChoosePixelFormat(hDC, &pfd); + SetPixelFormat(hDC, pxfmt, &pfd); + HGLRC hRC = wglCreateContext(hDC); // Create OpenGL 2.1 or less + wglMakeCurrent(hDC, hRC); + pp::platform::windows::set_async_render_context(hDC, hRC); // Initialize extensions if (!gladLoadGL()) @@ -878,7 +845,7 @@ int main(int argc, char** argv) LOG("gladLoadGL() failed"); return 0; } - if (!gladLoadWGL(state.hDC)) + if (!gladLoadWGL(hDC)) { LOG("gladLoadWGL() failed"); return 0; @@ -912,18 +879,19 @@ int main(int argc, char** argv) UINT numFormat; wglMakeCurrent(NULL, NULL); - wglDeleteContext(state.hRC); + wglDeleteContext(hRC); DestroyWindow(state.hWnd); state.hWnd = CreateWindow(wc.lpszClassName, state.window_title, wnd_style, clientPos.x, clientPos.y, (float)(clientRect.right - clientRect.left), (float)(clientRect.bottom - clientRect.top), 0, 0, state.hInst, 0); - state.hDC = GetDC(state.hWnd); - wglChoosePixelFormatARB(state.hDC, wgl_config.pixel_format_attributes.data(), nullptr, 1, &pxfmt, &numFormat); - SetPixelFormat(state.hDC, pxfmt, &pfd); - state.hRC = wglCreateContextAttribsARB(state.hDC, NULL, wgl_config.context_attributes.data()); - wglMakeCurrent(state.hDC, state.hRC); + hDC = GetDC(state.hWnd); + wglChoosePixelFormatARB(hDC, wgl_config.pixel_format_attributes.data(), nullptr, 1, &pxfmt, &numFormat); + SetPixelFormat(hDC, pxfmt, &pfd); + hRC = wglCreateContextAttribsARB(hDC, NULL, wgl_config.context_attributes.data()); + wglMakeCurrent(hDC, hRC); + pp::platform::windows::set_async_render_context(hDC, hRC); } else { diff --git a/src/node_canvas.cpp b/src/node_canvas.cpp index 82cacf87..e5636a68 100644 --- a/src/node_canvas.cpp +++ b/src/node_canvas.cpp @@ -385,32 +385,14 @@ void NodeCanvas::draw() }), }); - const auto draw_merged_texture_plane = [&](int plane_index) { - pp::panopainter::execute_legacy_canvas_draw_merge_layer_texture( - { - .mvp = proj * camera * - m_canvas->m_plane_transform[plane_index] * - glm::translate(glm::vec3(0, 0, -1)), - .texture_slot = 0, - .alpha = 1.f, - .highlight = false, - }, - { - .bind_sampler = [&] { - m_sampler.bind(0); - set_active_texture_unit(0); - }, - .bind_layer_texture = [&] { - m_canvas->m_layers_merge.rtt(plane_index).bindTexture(); - }, - .draw = [&] { - m_face_plane.draw_fill(); - }, - .unbind_layer_texture = [&] { - m_canvas->m_layers_merge.rtt(plane_index).unbindTexture(); - }, - }); - }; + const auto draw_merged_texture_plane = pp::panopainter::make_legacy_canvas_draw_merge_layer_texture_draw( + &m_canvas->m_layers_merge, + &m_sampler, + &m_face_plane, + set_active_texture_unit, + proj, + camera, + m_canvas->m_plane_transform); for (int plane_index = 0; plane_index < 6; plane_index++) { diff --git a/src/platform_windows/windows_platform_services.cpp b/src/platform_windows/windows_platform_services.cpp index 95f7bad2..c59bb863 100644 --- a/src/platform_windows/windows_platform_services.cpp +++ b/src/platform_windows/windows_platform_services.cpp @@ -26,6 +26,84 @@ void win32_update_stylus(float dt); void win32_save_window_state(); bool win32_vr_start(); void win32_vr_stop(); +std::string GetLastErrorAsString(); + +namespace pp::platform::windows { + +namespace { + +struct RetainedWin32AsyncRenderContextState final { + HDC hdc{}; + HGLRC hrc{}; + std::mutex gl_mutex; + std::thread::id gl_thread{}; + int gl_count = 0; +}; + +[[nodiscard]] RetainedWin32AsyncRenderContextState& retained_win32_async_render_context_state() +{ + static RetainedWin32AsyncRenderContextState state; + return state; +} + +} + +void set_async_render_context(HDC hdc, HGLRC hrc) +{ + auto& state = retained_win32_async_render_context_state(); + state.hdc = hdc; + state.hrc = hrc; + state.gl_thread = {}; + state.gl_count = 0; +} + +void lock_async_render_context() +{ + auto& state = retained_win32_async_render_context_state(); + if (state.gl_count == 0 || state.gl_thread != std::this_thread::get_id()) + { + state.gl_mutex.lock(); + const bool ret = wglMakeCurrent(state.hdc, state.hrc); + if (!ret) + LOG("FAILED wglMakeCurrent: %s", GetLastErrorAsString().c_str()); + state.gl_thread = std::this_thread::get_id(); + } + state.gl_count++; +} + +bool try_lock_async_render_context() +{ + auto& state = retained_win32_async_render_context_state(); + if (state.gl_count == 0 || state.gl_thread != std::this_thread::get_id()) + { + if (!state.gl_mutex.try_lock()) + return false; + const bool ret = wglMakeCurrent(state.hdc, state.hrc); + if (!ret) + LOG("FAILED wglMakeCurrent: %s", GetLastErrorAsString().c_str()); + state.gl_thread = std::this_thread::get_id(); + } + state.gl_count++; + return true; +} + +void unlock_async_render_context() +{ + auto& state = retained_win32_async_render_context_state(); + state.gl_count--; + if (state.gl_count == 0) + { + wglMakeCurrent(0, 0); + state.gl_mutex.unlock(); + } +} + +void swap_async_render_context() +{ + SwapBuffers(retained_win32_async_render_context_state().hdc); +} + +} namespace {