diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index b243a7ee..8eadca22 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,22 @@ agent or engineer to remove them without reconstructing context from chat. ## Reductions +- 2026-06-16: `DEBT-0036` was narrowed again. `NodeCanvas` smoothing-mask + overlay draw, smoothing-mask face pass, grid keepalive draw, heightmap draw, + and current-mode draw now route through + `execute_legacy_canvas_draw_merge_post_draw(...)` in + `src/legacy_canvas_draw_merge_services.h` instead of living inline in + `NodeCanvas::draw()`; broader canvas draw orchestration and retained GL + resource ownership remain. +- 2026-06-16: `DEBT-0036` was narrowed again. The retained + `NodePanelGrid::bake_uvs()` worker now uses scoped `std::jthread` ownership + instead of a raw local `std::thread`; retained bake execution, progress-loop + polling, and grid rendering ownership remain. +- 2026-06-16: `DEBT-0017` was narrowed again. + `LegacyPlatformServices::prepare_storage_paths()` now routes Apple storage + path setup through a local helper instead of reading `App::I` directly in + the method body; the retained Apple fallback adapter and broader + platform-to-app singleton reach remain. - 2026-06-16: `DEBT-0036` was narrowed again. `NodeStrokePreview` background preview execution now owns its worker as `std::jthread` with explicit stop, unblock, and join semantics instead of a raw `std::thread`; retained queue diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 4fe0b851..ec72a073 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -102,13 +102,16 @@ Current architecture mismatches that must be treated as real blockers: app shell. - `pp_panopainter_ui` still depends on `pp_legacy_app`. - `Canvas`, `NodeCanvas`, and `NodeStrokePreview` still own too much live - OpenGL execution around the renderer boundary. + OpenGL execution around the renderer boundary, even though `NodeCanvas` + display resolve, cache-to-screen composite, and post-draw mask/grid/current- + mode sequencing now route through retained draw-merge helpers. - `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 global singleton reach, raw observer pointers, retained static worker ownership in several app families, and ad hoc mutex/condition-variable - ownership. + ownership, even though most previously detached or raw app-facing worker + launches now use owned `std::jthread` or service-owned worker queues. - 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 3689182b..f7b11601 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -132,8 +132,11 @@ Current slice: 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 plus cache-to-screen checkerboard/cache-texture - composite now route through `legacy_canvas_draw_merge_services.h`, but - broader canvas draw orchestration is still inline. + composite now route through `legacy_canvas_draw_merge_services.h`. +- `NodeCanvas` smoothing-mask overlay, smoothing-mask face pass, grid keepalive + draw, heightmap draw, and current-mode draw now also route through + `execute_legacy_canvas_draw_merge_post_draw(...)`, but broader canvas draw + orchestration is still inline. Write scope: - `src/node_stroke_preview.cpp` @@ -348,9 +351,9 @@ Mini-model packet: Status: In Progress Why now: -Canvas imports/exports/saves, cloud transfer, brush import/export, grid -lightmap work, stroke preview, and event persistence still launch detached -threads. That is not a safe modernization foundation. +The biggest app-facing async families have been moved off detached launches, +but retained worker ownership and ad hoc runtime control are still not a safe +modernization foundation. Current slice: - app-owned render/UI runtime queues and cloud worker ownership are already @@ -361,8 +364,9 @@ Current slice: workers with explicit UI-thread handoff - canvas async import/export/save/open and timelapse export now also use owned worker queues instead of detached threads -- preview background rendering and recording thread ownership now also use - `std::jthread`, but their retained loop/control flow is still open +- preview background rendering, recording, and the retained + `NodePanelGrid::bake_uvs()` worker now also use `std::jthread`, but their + retained loop/control flow is still open Write scope: - `src/canvas.cpp` @@ -563,6 +567,9 @@ Current slice: `App::set_platform_services()` - `platform_apple` clipboard, display/share, cursor, and save-ui-state calls now route through injected Apple bridge callbacks instead of `App::I` +- `LegacyPlatformServices::prepare_storage_paths()` now routes Apple path + preparation through a narrow local helper instead of reading `App::I` + directly in that method body - retained Apple callback injection and broader `platform_legacy` singleton reach are still open diff --git a/src/legacy_canvas_draw_merge_services.h b/src/legacy_canvas_draw_merge_services.h index 88223c81..cfba6cbb 100644 --- a/src/legacy_canvas_draw_merge_services.h +++ b/src/legacy_canvas_draw_merge_services.h @@ -203,6 +203,15 @@ struct LegacyCanvasDrawMergeDisplayResolveExecution { std::function unbind_resolve_texture; }; +struct LegacyCanvasDrawMergePostDrawExecution { + std::function draw_mask_free; + std::function draw_mask_line; + std::function draw_smask_faces; + std::function draw_grid_modes; + std::function draw_heightmap; + std::function draw_current_modes; +}; + [[nodiscard]] inline LegacyCanvasDrawMergeShaderExecution legacy_shader_manager_draw_merge_execution() noexcept { return { @@ -492,4 +501,31 @@ inline void execute_legacy_canvas_draw_merge_display_resolve( execution.unbind_resolve_texture(); } +inline void execute_legacy_canvas_draw_merge_post_draw( + bool smask_active, + bool draw_mask_overlay, + int smask_mode, + bool draw_grid_modes, + const LegacyCanvasDrawMergePostDrawExecution& execution) +{ + if (smask_active || draw_mask_overlay) { + if (smask_mode == 1) { + execution.draw_mask_free(); + } else if (smask_mode == 2) { + execution.draw_mask_line(); + } + } + + if (smask_active) { + execution.draw_smask_faces(); + } + + if (draw_grid_modes) { + execution.draw_grid_modes(); + } + + execution.draw_heightmap(); + execution.draw_current_modes(); +} + } // namespace pp::panopainter diff --git a/src/node_canvas.cpp b/src/node_canvas.cpp index f34e247e..91576e29 100644 --- a/src/node_canvas.cpp +++ b/src/node_canvas.cpp @@ -33,7 +33,6 @@ void set_active_texture_unit(std::uint32_t unit_index) { pp::legacy::ui_gl::activate_texture_unit(unit_index, "NodeCanvas"); } - void unbind_texture_2d() { pp::legacy::ui_gl::unbind_texture_2d("NodeCanvas"); @@ -719,50 +718,53 @@ void NodeCanvas::draw() apply_node_canvas_capability(pp::renderer::gl::depth_test_state(), false); - if (m_canvas->m_smask_active || m_canvas->m_current_mode == kCanvasMode::Copy || m_canvas->m_current_mode == kCanvasMode::Cut) - { - if (m_canvas->m_smask_mode == 1) - m_canvas->modes[(int)kCanvasMode::MaskFree][0]->on_Draw(ortho_proj, proj, camera); - else if (m_canvas->m_smask_mode == 2) - m_canvas->modes[(int)kCanvasMode::MaskLine][0]->on_Draw(ortho_proj, proj, camera); - } - - if (m_canvas->m_smask_active) - { - pp::panopainter::setup_legacy_canvas_draw_merge_texture_mask_shader( - pp::panopainter::LegacyCanvasDrawMergeTextureMaskUniforms { - .texture_slot = 0, - .pattern_offset = m_outline_pan, - }); - set_active_texture_unit(0); - apply_node_canvas_capability(pp::renderer::gl::blend_state(), true); - - //draw the cube faces - for (int plane_index = 0; plane_index < 6; plane_index++) + pp::panopainter::execute_legacy_canvas_draw_merge_post_draw( + m_canvas->m_smask_active, + m_canvas->m_current_mode == kCanvasMode::Copy || m_canvas->m_current_mode == kCanvasMode::Cut, + m_canvas->m_smask_mode, + m_canvas->m_current_mode != kCanvasMode::Grid, { - auto plane_mvp = proj * camera * - glm::scale(glm::vec3(m_canvas->m_layers.size() + 500.f)) * - m_canvas->m_plane_transform[plane_index] * - glm::translate(glm::vec3(0, 0, -1.f)); + .draw_mask_free = [&] { + m_canvas->modes[(int)kCanvasMode::MaskFree][0]->on_Draw(ortho_proj, proj, camera); + }, + .draw_mask_line = [&] { + m_canvas->modes[(int)kCanvasMode::MaskLine][0]->on_Draw(ortho_proj, proj, camera); + }, + .draw_smask_faces = [&] { + pp::panopainter::setup_legacy_canvas_draw_merge_texture_mask_shader( + pp::panopainter::LegacyCanvasDrawMergeTextureMaskUniforms { + .texture_slot = 0, + .pattern_offset = m_outline_pan, + }); + set_active_texture_unit(0); + apply_node_canvas_capability(pp::renderer::gl::blend_state(), true); - pp::panopainter::apply_legacy_canvas_draw_merge_mvp(plane_mvp); - m_canvas->m_smask.rtt(plane_index).bindTexture(); - m_face_plane.draw_fill(); - m_canvas->m_smask.rtt(plane_index).unbindTexture(); - } + //draw the cube faces + for (int plane_index = 0; plane_index < 6; plane_index++) + { + auto plane_mvp = proj * camera * + glm::scale(glm::vec3(m_canvas->m_layers.size() + 500.f)) * + m_canvas->m_plane_transform[plane_index] * + glm::translate(glm::vec3(0, 0, -1.f)); - - } - - // keep drawing the grids - if (m_canvas->m_current_mode != kCanvasMode::Grid) - for (auto& mode : Canvas::modes[(int)kCanvasMode::Grid]) - mode->on_Draw(ortho_proj, proj, camera); - - App::I->grid->draw_heightmap(proj, camera, false); - - for (auto& mode : *m_canvas->m_mode) - mode->on_Draw(ortho_proj, proj, camera); + pp::panopainter::apply_legacy_canvas_draw_merge_mvp(plane_mvp); + m_canvas->m_smask.rtt(plane_index).bindTexture(); + m_face_plane.draw_fill(); + m_canvas->m_smask.rtt(plane_index).unbindTexture(); + } + }, + .draw_grid_modes = [&] { + for (auto& mode : Canvas::modes[(int)kCanvasMode::Grid]) + mode->on_Draw(ortho_proj, proj, camera); + }, + .draw_heightmap = [&] { + App::I->grid->draw_heightmap(proj, camera, false); + }, + .draw_current_modes = [&] { + for (auto& mode : *m_canvas->m_mode) + mode->on_Draw(ortho_proj, proj, camera); + }, + }); if (m_density != 1.f) { diff --git a/src/node_panel_grid.cpp b/src/node_panel_grid.cpp index 872fddfb..179bdcaa 100644 --- a/src/node_panel_grid.cpp +++ b/src/node_panel_grid.cpp @@ -449,60 +449,60 @@ void NodePanelGrid::bake_uvs() auto data_out = std::make_unique(fb.getWidth() * fb.getHeight() * 4); const auto samples = get_samples(); const auto radius = get_radius(); - std::thread worker([&] { - BT_SetTerminate(); - float* d_pos = data_pos.get(); - float* d_nor = data_nor.get(); - auto* d_out = reinterpret_cast(data_out.get()); - parallel_for(fb.getHeight(), [&](size_t y) + std::jthread worker([&] { - for (int x = 0; x < fb.getWidth(); x++) + BT_SetTerminate(); + float* d_pos = data_pos.get(); + float* d_nor = data_nor.get(); + auto* d_out = reinterpret_cast(data_out.get()); + parallel_for(fb.getHeight(), [&](size_t y) { - int i = (int)y * fb.getHeight() + x; - auto nor = glm::make_vec3(&d_nor[i * 4]); - auto pos = glm::make_vec3(&d_pos[i * 4]); - auto& out = d_out[i]; - - if (glm::dot(nor, light_dir) <= 0.f) + for (int x = 0; x < fb.getWidth(); x++) { - out = { 50, 50, 50, 255 }; - continue; + int i = (int)y * fb.getHeight() + x; + auto nor = glm::make_vec3(&d_nor[i * 4]); + auto pos = glm::make_vec3(&d_pos[i * 4]); + auto& out = d_out[i]; + + if (glm::dot(nor, light_dir) <= 0.f) + { + out = { 50, 50, 50, 255 }; + continue; + } + + int hit = 0; + for (int s = 0; s < samples; s++) + { + auto dir = glm::normalize(light_dir + glm::sphericalRand(radius)); + + nanort::Ray ray; + ray.org[0] = pos.x;// + nor.x * 0.005; + ray.org[1] = pos.y;// + nor.y * 0.005; + ray.org[2] = pos.z;// + nor.z * 0.005; + ray.dir[0] = dir.x; + ray.dir[1] = dir.y; + ray.dir[2] = dir.z; + + float kFar = 2000.0; + ray.min_t = 0.005f; + ray.max_t = kFar; + + nanort::TriangleIntersector<> triangle_intersector(reinterpret_cast(m_hm_plane.vertices.data()), m_hm_plane.idx.data(), sizeof(vertex_t)); + nanort::TriangleIntersection<> isect; + hit += m_rt_accel.Traverse(ray, triangle_intersector, &isect); + } + out = glm::lerp(glm::vec4(255, 255, 255, 255), glm::vec4(50, 50, 50, 255), hit / (float)samples); } - - int hit = 0; - for (int s = 0; s < samples; s++) - { - auto dir = glm::normalize(light_dir + glm::sphericalRand(radius)); - - nanort::Ray ray; - ray.org[0] = pos.x;// + nor.x * 0.005; - ray.org[1] = pos.y;// + nor.y * 0.005; - ray.org[2] = pos.z;// + nor.z * 0.005; - ray.dir[0] = dir.x; - ray.dir[1] = dir.y; - ray.dir[2] = dir.z; - - float kFar = 2000.0; - ray.min_t = 0.005f; - ray.max_t = kFar; - - nanort::TriangleIntersector<> triangle_intersector(reinterpret_cast(m_hm_plane.vertices.data()), m_hm_plane.idx.data(), sizeof(vertex_t)); - nanort::TriangleIntersection<> isect; - hit += m_rt_accel.Traverse(ray, triangle_intersector, &isect); - } - out = glm::lerp(glm::vec4(255, 255, 255, 255), glm::vec4(50, 50, 50, 255), hit / (float)samples); - } - pb_value++; + pb_value++; + }); }); + while (pb_value < fb.getHeight()) + { + pb->set_progress((float)pb_value / (float)fb.getHeight()); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } } - ); - while (pb_value < fb.getHeight()) - { - pb->set_progress((float)pb_value / (float)fb.getHeight()); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } - worker.join(); pp::panopainter::close_legacy_dialog_node(*pb); //stbi_write_jpg("bake-out.jpg", fb.getWidth(), fb.getHeight(), 4, data_out.get(), 75); m_texture.update(data_out.get()); diff --git a/src/platform_legacy/legacy_platform_services.cpp b/src/platform_legacy/legacy_platform_services.cpp index 0b2c8cd3..fb155148 100644 --- a/src/platform_legacy/legacy_platform_services.cpp +++ b/src/platform_legacy/legacy_platform_services.cpp @@ -105,6 +105,21 @@ public: return types; } +[[nodiscard]] pp::platform::PlatformStoragePaths prepare_legacy_apple_storage_paths() +{ +#ifdef __IOS__ + [App::I->ios_view init_dirs]; +#elif defined(__OSX__) + [App::I->osx_app init_dirs]; +#endif + return { + App::I->data_path, + App::I->work_path, + App::I->rec_path, + App::I->tmp_path, + }; +} + [[nodiscard]] pp::platform::apple::AppleDocumentPlatformServices& active_apple_document_platform_services() { #ifdef __IOS__ @@ -211,21 +226,9 @@ public: [[nodiscard]] pp::platform::PlatformStoragePaths prepare_storage_paths() override { #if defined(__IOS__) - [App::I->ios_view init_dirs]; - return { - App::I->data_path, - App::I->work_path, - App::I->rec_path, - App::I->tmp_path, - }; + return prepare_legacy_apple_storage_paths(); #elif defined(__OSX__) - [App::I->osx_app init_dirs]; - return { - App::I->data_path, - App::I->work_path, - App::I->rec_path, - App::I->tmp_path, - }; + return prepare_legacy_apple_storage_paths(); #elif __LINUX__ const std::string data_path = linux_home_path() + "/PanoPainter"; mkpath(data_path + "/brushes");