diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index eb1a30e4..e73ca9ab 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,27 @@ agent or engineer to remove them without reconstructing context from chat. ## Reductions +- 2026-06-16: `DEBT-0051`/`DEBT-0052`/`DEBT-0055` were narrowed again. + `src/platform_apple/apple_platform_services.*` no longer reaches `App::I` + for clipboard, display/share, cursor-visibility, or save-ui-state behavior; + those calls now flow through narrow injected Apple bridge callbacks from + `src/platform_legacy/legacy_platform_services.cpp`, while retained Apple + bridge construction and broader platform singleton reach remain. +- 2026-06-16: `DEBT-0036` was narrowed again. `NodeCanvas` density-resolve + display execution now routes through + `legacy_canvas_draw_merge_services.h` instead of living inline in + `NodeCanvas::draw()`; the cache-to-screen composite block and broader canvas + draw orchestration remain retained. +- 2026-06-16: `DEBT-0053` was narrowed again. `App::pick_file_save(...)` no + longer launches a detached worker for background prepared-file writes; it + now uses a service-owned `std::jthread` queue and posts prepared-file save + completion back to the UI thread, while retained platform save/download + handoff execution remains. +- 2026-06-16: `DEBT-0036` was narrowed again. The retained grid lightmap + launch in `src/legacy_grid_ui_services.cpp` no longer uses a detached + worker thread; it now uses a service-owned `std::jthread` queue with + UI-thread state handoff, while retained bake execution and grid rendering + ownership remain. - 2026-06-16: `DEBT-0048` was narrowed again. The retained ABR/PPBR import bridge in `src/legacy_brush_package_import_services.cpp` no longer launches detached worker threads; it now uses a service-owned `std::jthread` queue, diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index f6bd81a6..18229af5 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -94,10 +94,10 @@ Current architecture mismatches that must be treated as real blockers: - `pp_platform_api` still compiles Apple implementation files instead of only platform-neutral policy and interface code. -- `src/platform_apple/apple_platform_services.cpp` and - parts of the concrete platform layer still reach `App::I`; Linux FPS title - reporting now uses an injected callback, but Apple singleton reach and other - platform/app coupling remain. +- `src/platform_apple/apple_platform_services.cpp` no longer reaches `App::I` + directly, and Linux FPS title reporting now uses an injected callback, but + retained Apple bridging in `platform_legacy` and other platform/app coupling + remain. - `src/platform_legacy/legacy_platform_services.*` is still part of the live app shell. - `pp_panopainter_ui` still depends on `pp_legacy_app`. @@ -107,7 +107,8 @@ Current architecture mismatches that must be treated as real blockers: rather than thin composition/binding surfaces. - `App`, `Canvas`, `Node`, retained workers, and platform entrypoints still use global singleton reach, raw observer pointers, detached `std::thread` - launches, and ad hoc mutex/condition-variable ownership. + launches in several canvas/export/preview paths, and ad hoc + mutex/condition-variable ownership. - 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 8ed83d47..cc4c923d 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -42,13 +42,15 @@ Completed, blocked, and superseded task history moved to `src/node_canvas.cpp`, `src/app.cpp`, and `src/app_dialogs.cpp`. - The platform boundary is not finished: - `pp_platform_api` still compiles Apple implementation files - - Apple platform services still reach `App::I` - - Linux FPS title reporting now uses an injected callback, but broader - platform-to-app singleton reach is still open + - `platform_apple` no longer reaches `App::I` directly, and Linux FPS title + reporting now uses an injected callback, but retained Apple bridging and + broader platform-to-app singleton reach are still open in + `platform_legacy` - `platform_legacy` is still part of the live app shell - The app runtime boundary is not finished: - render/UI queues are static `App` state - - detached workers still launch from canvas, grid, preview, and event code + - detached workers still launch from canvas, preview, document export, and + recording code - thread-affinity rules are enforced by convention and asserts instead of explicit runtime contracts - The UI ownership boundary is not finished: @@ -125,6 +127,9 @@ Current slice: - `NodeStrokePreview` final composite plus preview-texture copy now route through `legacy_node_stroke_preview_execution_services.h`, but the preview node still owns most live-pass and retained GL resource execution. +- `NodeCanvas` display resolve for the `m_density != 1.f` path now routes + through `legacy_canvas_draw_merge_services.h`, but the cache-to-screen + composite block and broader canvas draw orchestration are still inline. Write scope: - `src/node_stroke_preview.cpp` @@ -348,7 +353,10 @@ Current slice: moving behind owned runtime/service objects - brush package import/export now use service-owned `std::jthread` workers and UI-thread completion handoff -- canvas, grid, preview, and event-side detached work is still open +- prepared-file save work and grid lightmap launch now also use service-owned + workers with explicit UI-thread handoff +- canvas, preview, document export, and recording-side detached work are still + open Write scope: - `src/canvas.cpp` @@ -547,8 +555,10 @@ which means the platform layer is not a platform layer yet. Current slice: - Linux FPS title updates now route through an injected callback installed from `App::set_platform_services()` -- Apple singleton reach and the remaining platform callback surface are still - open +- `platform_apple` clipboard, display/share, cursor, and save-ui-state calls + now route through injected Apple bridge callbacks instead of `App::I` +- retained Apple callback injection and broader `platform_legacy` singleton + reach are still open Write scope: - `src/platform_apple/*` diff --git a/src/app_events.cpp b/src/app_events.cpp index 96f32c65..c2f643e8 100644 --- a/src/app_events.cpp +++ b/src/app_events.cpp @@ -5,6 +5,14 @@ #include "app_core/document_platform_io.h" #include "app_core/document_sharing.h" #include "platform_api/platform_services.h" + +#include +#include +#include +#include +#include +#include + #ifdef __LINUX__ #include #include "platform_linux/linux_platform_services.h" @@ -14,6 +22,79 @@ namespace { +class LegacyPreparedFileWorker final { +public: + LegacyPreparedFileWorker() + : worker_([this](std::stop_token stop_token) { + run(stop_token); + }) + { + } + + ~LegacyPreparedFileWorker() + { + shutdown(); + } + + void post(std::function task) + { + { + std::lock_guard lock(mutex_); + if (stopping_) + return; + tasks_.push_back(std::move(task)); + } + cv_.notify_one(); + } + +private: + void shutdown() + { + { + std::lock_guard lock(mutex_); + stopping_ = true; + } + cv_.notify_all(); + } + + void run(std::stop_token stop_token) + { + for (;;) { + std::function task; + { + std::unique_lock lock(mutex_); + cv_.wait(lock, [&] { + return stopping_ || stop_token.stop_requested() || !tasks_.empty(); + }); + if ((stopping_ || stop_token.stop_requested()) && tasks_.empty()) + break; + task = std::move(tasks_.front()); + tasks_.pop_front(); + } + + if (task) { + try { + task(); + } catch (...) { + LOG("prepared file worker task failed"); + } + } + } + } + + std::mutex mutex_; + std::condition_variable cv_; + std::deque> tasks_; + bool stopping_ = false; + std::jthread worker_; +}; + +LegacyPreparedFileWorker& prepared_file_worker() +{ + static LegacyPreparedFileWorker worker; + return worker; +} + [[nodiscard]] GLint rgba8_internal_format() noexcept { return static_cast(pp::renderer::gl::rgba8_internal_format()); @@ -201,15 +282,31 @@ void App::pick_file_save(const std::string& type, const std::string& default_nam } LOG("App::pick_file_save %s", target.path.c_str()); - auto write_and_save = [=] { - writer(target.path); - save_prepared_file(target.path, target.suggested_name, callback); - }; + if (target.write_on_background_thread) { + auto* app = this; + prepared_file_worker().post([ + app, + writer = std::move(writer), + callback = std::move(callback), + path = target.path, + suggested_name = target.suggested_name + ]() mutable { + writer(path); + app->ui_task([app, + path = std::move(path), + suggested_name = std::move(suggested_name), + callback = std::move(callback)]() mutable { + app->save_prepared_file( + std::move(path), + std::move(suggested_name), + std::move(callback)); + }); + }); + return; + } - if (target.write_on_background_thread) - std::thread(write_and_save).detach(); - else - write_and_save(); + writer(target.path); + save_prepared_file(target.path, target.suggested_name, std::move(callback)); } void App::pick_file_save(std::vector types, std::function callback) diff --git a/src/legacy_grid_ui_services.cpp b/src/legacy_grid_ui_services.cpp index 0b6f86c8..4fce6585 100644 --- a/src/legacy_grid_ui_services.cpp +++ b/src/legacy_grid_ui_services.cpp @@ -7,9 +7,89 @@ #include "image.h" #include "node_panel_grid.h" +#include +#include +#include +#include +#include +#include + namespace pp::panopainter { namespace { +class LegacyGridWorker final { +public: + LegacyGridWorker() + : worker_([this](std::stop_token stop_token) { + run(stop_token); + }) + { + } + + ~LegacyGridWorker() + { + shutdown(); + } + + void post(std::function task) + { + { + std::lock_guard lock(mutex_); + if (stopping_) + return; + tasks_.push_back(std::move(task)); + } + cv_.notify_one(); + } + +private: + void shutdown() + { + { + std::lock_guard lock(mutex_); + stopping_ = true; + } + cv_.notify_all(); + } + + void run(std::stop_token stop_token) + { + for (;;) { + std::function task; + { + std::unique_lock lock(mutex_); + cv_.wait(lock, [&] { + return stopping_ || stop_token.stop_requested() || !tasks_.empty(); + }); + if ((stopping_ || stop_token.stop_requested()) && tasks_.empty()) + break; + task = std::move(tasks_.front()); + tasks_.pop_front(); + } + + if (task) { + try { + task(); + } catch (...) { + LOG("grid worker task failed"); + } + } + } + } + + std::mutex mutex_; + std::condition_variable cv_; + std::deque> tasks_; + bool stopping_ = false; + std::jthread worker_; +}; + +LegacyGridWorker& grid_worker() +{ + static LegacyGridWorker worker; + return worker; +} + class LegacyGridUiServices final : public pp::app::GridUiServices { public: explicit LegacyGridUiServices(NodePanelGrid& panel) noexcept @@ -66,13 +146,17 @@ public: if (!renders_lightmap) return; - auto* panel = &panel_; - std::thread([panel] { + auto panel = std::static_pointer_cast(panel_.shared_from_this()); + grid_worker().post([panel] { BT_SetTerminate(); panel->bake_uvs(); - panel->m_hm_shading->set_index(3); - panel->m_shade_mode = NodePanelGrid::ShadeMode::Textured; - }).detach(); + if (App::I) { + App::I->ui_task([panel] { + panel->m_hm_shading->set_index(3); + panel->m_shade_mode = NodePanelGrid::ShadeMode::Textured; + }); + } + }); } void commit_heightmap(bool updates_ground_opacity) override diff --git a/src/platform_apple/apple_platform_services.cpp b/src/platform_apple/apple_platform_services.cpp index 5efdd5c0..1291e633 100644 --- a/src/platform_apple/apple_platform_services.cpp +++ b/src/platform_apple/apple_platform_services.cpp @@ -6,11 +6,6 @@ #include #include -#if defined(__IOS__) || defined(__OSX__) -#include "app_core/app.h" -#include -#endif - namespace pp::platform::apple { namespace { @@ -54,26 +49,16 @@ std::vector AppleDocumentPlatformServices::document_browse_roots( std::string AppleDocumentPlatformServices::clipboard_text() const { -#if defined(__IOS__) - return [App::I->ios_view clipboard_get_string]; -#elif defined(__OSX__) - return [App::I->osx_view clipboard_get_string]; -#else + if (bridge_.clipboard_text) + return bridge_.clipboard_text(); return {}; -#endif } bool AppleDocumentPlatformServices::set_clipboard_text(std::string_view text) const { - const std::string value(text); -#if defined(__IOS__) - return [App::I->ios_view clipboard_set_string:value]; -#elif defined(__OSX__) - return [App::I->osx_view clipboard_set_string:value]; -#else - (void)value; + if (bridge_.set_clipboard_text) + return bridge_.set_clipboard_text(text); return false; -#endif } void AppleDocumentPlatformServices::pick_image(PickedPathCallback callback) const @@ -158,9 +143,8 @@ void AppleDocumentPlatformServices::display_file(std::string_view path) const { const std::string value(path); #if defined(__IOS__) - dispatch_async(dispatch_get_main_queue(), ^{ - [App::I->ios_view display_file:value]; - }); + if (bridge_.display_file) + bridge_.display_file(value); #elif defined(__OSX__) [[NSWorkspace sharedWorkspace] openFile:[NSString stringWithUTF8String:value.c_str()]]; #else @@ -171,14 +155,9 @@ void AppleDocumentPlatformServices::display_file(std::string_view path) const void AppleDocumentPlatformServices::share_file(std::string_view path) const { const std::string value(path); -#if defined(__IOS__) - dispatch_async(dispatch_get_main_queue(), ^{ - [App::I->ios_view share_file:[NSString stringWithUTF8String:value.c_str()]]; - }); -#elif defined(__OSX__) - dispatch_async(dispatch_get_main_queue(), ^{ - [App::I->osx_view share_file:[NSString stringWithUTF8String:value.c_str()]]; - }); +#if defined(__IOS__) || defined(__OSX__) + if (bridge_.share_file) + bridge_.share_file(value); #else (void)value; #endif @@ -187,7 +166,8 @@ void AppleDocumentPlatformServices::share_file(std::string_view path) const void AppleDocumentPlatformServices::set_cursor_visible(bool visible) const { #if defined(__OSX__) - [App::I->osx_view show_cursor:visible]; + if (bridge_.set_cursor_visible) + bridge_.set_cursor_visible(visible); #else (void)visible; #endif @@ -196,7 +176,8 @@ void AppleDocumentPlatformServices::set_cursor_visible(bool visible) const void AppleDocumentPlatformServices::save_ui_state() const { #if defined(__OSX__) - [App::I->osx_app save_ui_state]; + if (bridge_.save_ui_state) + bridge_.save_ui_state(); #endif } diff --git a/src/platform_apple/apple_platform_services.h b/src/platform_apple/apple_platform_services.h index dd66b3a5..9da9030e 100644 --- a/src/platform_apple/apple_platform_services.h +++ b/src/platform_apple/apple_platform_services.h @@ -16,6 +16,12 @@ struct AppleDocumentPickerBridge { std::function file_types, PickedPathCallback callback)> pick_save_file; std::function pick_directory; std::function format_working_directory_path; + std::function clipboard_text; + std::function set_clipboard_text; + std::function display_file; + std::function share_file; + std::function set_cursor_visible; + std::function save_ui_state; }; class AppleDocumentPlatformServices { diff --git a/src/platform_legacy/legacy_platform_services.cpp b/src/platform_legacy/legacy_platform_services.cpp index 41a3a942..67a20a23 100644 --- a/src/platform_legacy/legacy_platform_services.cpp +++ b/src/platform_legacy/legacy_platform_services.cpp @@ -112,6 +112,23 @@ public: pp::platform::PlatformFamily::ios, [] { pp::platform::apple::AppleDocumentPickerBridge bridge; + bridge.clipboard_text = [] { + return [App::I->ios_view clipboard_get_string]; + }; + bridge.set_clipboard_text = [](std::string_view text) { + const std::string value(text); + return [App::I->ios_view clipboard_set_string:value]; + }; + bridge.display_file = [](std::string path) { + dispatch_async(dispatch_get_main_queue(), ^{ + [App::I->ios_view display_file:path]; + }); + }; + bridge.share_file = [](std::string path) { + dispatch_async(dispatch_get_main_queue(), ^{ + [App::I->ios_view share_file:[NSString stringWithUTF8String:path.c_str()]]; + }); + }; bridge.pick_image = [](pp::platform::PickedPathCallback callback) { dispatch_async(dispatch_get_main_queue(), ^{ [App::I->ios_view pick_photo:callback]; @@ -132,6 +149,24 @@ public: pp::platform::PlatformFamily::macos, [] { pp::platform::apple::AppleDocumentPickerBridge bridge; + bridge.clipboard_text = [] { + return [App::I->osx_view clipboard_get_string]; + }; + bridge.set_clipboard_text = [](std::string_view text) { + const std::string value(text); + return [App::I->osx_view clipboard_set_string:value]; + }; + bridge.share_file = [](std::string path) { + dispatch_async(dispatch_get_main_queue(), ^{ + [App::I->osx_view share_file:[NSString stringWithUTF8String:path.c_str()]]; + }); + }; + bridge.set_cursor_visible = [](bool visible) { + [App::I->osx_view show_cursor:visible]; + }; + bridge.save_ui_state = [] { + [App::I->osx_app save_ui_state]; + }; bridge.pick_file = []( std::vector file_types, pp::platform::PickedPathCallback callback) {