From ab36af0a8f19ae849e7ac09d17ce3cabb89f3d65 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Sat, 6 Jun 2026 10:46:29 +0200 Subject: [PATCH] Centralize retained panel popup attachment --- cmake/PanoPainterSources.cmake | 4 ++-- docs/modernization/build-inventory.md | 6 +++--- docs/modernization/debt.md | 8 +++++++- docs/modernization/roadmap.md | 7 ++++++- src/legacy_quick_ui_services.cpp | 5 +++-- src/legacy_ui_overlay_services.cpp | 17 +++++++++++++++++ src/legacy_ui_overlay_services.h | 4 ++++ src/node_panel_brush.cpp | 3 ++- src/node_panel_stroke.cpp | 9 +++++---- 9 files changed, 49 insertions(+), 14 deletions(-) diff --git a/cmake/PanoPainterSources.cmake b/cmake/PanoPainterSources.cmake index ba6bf6f..c5191a4 100644 --- a/cmake/PanoPainterSources.cmake +++ b/cmake/PanoPainterSources.cmake @@ -72,6 +72,8 @@ set(PP_LEGACY_APP_SOURCES src/legacy_history_services.h src/legacy_recording_services.cpp src/legacy_recording_services.h + src/legacy_ui_overlay_services.cpp + src/legacy_ui_overlay_services.h src/pch.cpp ) @@ -102,8 +104,6 @@ set(PP_PANOPAINTER_APP_SOURCES src/legacy_document_open_services.h src/legacy_document_session_services.cpp src/legacy_document_session_services.h - src/legacy_ui_overlay_services.cpp - src/legacy_ui_overlay_services.h src/platform_legacy/legacy_platform_services.cpp src/platform_legacy/legacy_platform_services.h src/version.cpp diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 2ca6ebb..faf0eff 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -752,14 +752,14 @@ powershell -ExecutionPolicy Bypass -File scripts\automation\apple-remote-build.p dialog, capture-restore, branch-close, and layout-reload coverage in `pp_ui_core_overlay_lifetime_tests`. Retained `Node`/popup/dialog code still needs to adopt those semantics under DEBT-0063. -- `panopainter_app` now owns `src/legacy_ui_overlay_services.*` as the retained +- `pp_legacy_app` now owns `src/legacy_ui_overlay_services.*` as the retained app-dialog overlay adapter. App-level progress/message/input dialogs and the retained about/manual/changelog, document open/save/new/browse/resize/ layer-rename, PPBR export, shortcuts, and what's-new overlays route their root attachment through that helper. Retained app-menu popup template cloning and root attachment for File/Export/Edit/Tools/Panels/Options/About/Layers menus - also route through the same helper while raw `Node` ownership and callbacks - remain legacy debt. + plus quick/stroke/brush panel popup root attachment also route through the same + helper while raw `Node` ownership and callbacks remain legacy debt. - `scripts/automation/analyze.*` runs shader validation plus a renderer-boundary guard that reports JSON and fails if active non-backend source code reintroduces raw `GL_*`/`WGL_*` constants outside the allowed diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index ecdd9b3..fe4b6ce 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -469,6 +469,12 @@ agent or engineer to remove them without reconstructing context from chat. status/logging instead of direct `m_children[0]` dereferences in `App::init_menu_*`. Raw popup callback captures and retained close semantics remain open. +- 2026-06-06: DEBT-0063 was narrowed again. `src/legacy_ui_overlay_services.*` + moved down to `pp_legacy_app` and now exposes retained root-popup attachment + for `pp_panopainter_ui`; quick-panel brush/color popups, stroke-panel preset/ + tip/dual/pattern popups, and brush-panel preset menu insertion no longer call + `root()->add_child(...)` directly. Base retained popup widgets such as + `NodeComboBox`, raw callback captures, and close/capture semantics remain open. - 2026-06-05: DEBT-0011 was narrowed. The Windows app package smoke target now passes the configure-time CMake executable into `package-smoke.ps1`, so VS 2026 generator validation does not depend on an older `cmake` on PATH, and @@ -662,7 +668,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0060 | Open | Modernization | Retained Android package CMake generates a patched `nanort.h` overlay in the build tree for `native-lib` instead of modifying the `libs/nanort` submodule | Current SDK Manager NDK/Clang rejects `TriangleSAHPred::operator=` assigning to a `const size_t` member, but the retained grid/lightmap path still includes `nanort` before that dependency is replaced or updated | `powershell -ExecutionPolicy Bypass -File scripts\automation\android-legacy-package-build.ps1 -Packages standard`; Quest/Focus retained package configure checks; Windows app build | Update/replace `nanort`, move grid/lightmap baking behind a component that owns its dependency, or retire the retained Android package CMake path so no generated vendor overlay is required | | DEBT-0061 | Open | Modernization | Desktop XR runtime selection now lives in tested `pp_platform_api` policy and prefers OpenXR, but `WindowsPlatformServices` still reports OpenXR unavailable and reaches the retained OpenVR SDK bridge as a legacy fallback; Windows runtime deployment copies `openvr_api.dll` beside `PanoPainter.exe` until that fallback is removed | Preserve current desktop VR behavior while replacing OpenVR with OpenXR behind the platform/renderer boundary | `pp_platform_api_tests`; `ctest --preset desktop-fast --build-config Debug -R pp_platform_api_tests --output-on-failure`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Add an OpenXR SDK/package target, implement desktop OpenXR startup/shutdown/pose/controller submission behind `pp_platform_vr` or `PlatformServices`, validate parity with mocked/runtime smoke coverage, and remove `libs/openvr` plus the OpenVR link/include paths from root CMake | | DEBT-0062 | Open | Modernization | VS 2026 builds generate a patched fmt `format.h` overlay in the build tree for `pp_legacy_vendor`, disabling the old `_SECURE_SCL` checked-array iterator branch while leaving the fmt submodule clean | VS 2026's STL no longer exposes the legacy checked-array iterator used by this old fmt release, but replacing fmt is part of the dependency migration rather than this platform unblock | `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter`; `cmake --build --preset windows-msvc-default --config Debug --target pp_platform_api_tests`; `ctest --preset desktop-fast --build-config Debug -R pp_platform_api_tests --output-on-failure` | Move fmt to a supported vcpkg/package version or update the vendored fmt release, then remove the generated fmt overlay from `pp_legacy_vendor` | -| DEBT-0063 | Open | Modernization | The retained UI tree still exposes `Node* m_parent`, public `std::vector> m_children`, raw `find()` lookup results, `add_child()` allocation through `new`, callbacks/observers that take or capture raw `Node*`, and manual `destroy()`/`m_destroyed` semantics. `pp_ui_core` now owns a tested `NodeLifetimeTree` target model with checked node handles, scoped callback connections, subtree destruction, pointer/keyboard capture release, whole-tree clear for layout reload, and mutation-safe dispatch, plus a tested `UiOverlayLifetime` popup/dialog stack model. Retained app-dialog root insertion and app-menu popup template cloning/root attachment are now centralized in `src/legacy_ui_overlay_services.*`, but retained `Node`/`NodePopupMenu`/`NodeDialog*` still have not adopted checked handles or scoped callback ownership | Preserve current UI behavior while making panel/dialog extraction safe instead of spreading lifetime hazards into the new architecture | `pp_ui_core_layout_xml_tests`; `pp_ui_core_node_lifetime_tests`; `pp_ui_core_overlay_lifetime_tests`; future `pp_panopainter_ui_dialog_lifetime_tests`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Retained `Node` and `pp_panopainter_ui` adopt checked node handles or equivalent non-owning references, scoped callback connection/disconnect semantics, mutation-safe event dispatch, parent/child invariants hidden behind APIs, and destroy-during-callback/capture-release/popup-close/layout-reload tests; retained `Node*` APIs are removed or isolated behind compatibility adapters | +| DEBT-0063 | Open | Modernization | The retained UI tree still exposes `Node* m_parent`, public `std::vector> m_children`, raw `find()` lookup results, `add_child()` allocation through `new`, callbacks/observers that take or capture raw `Node*`, and manual `destroy()`/`m_destroyed` semantics. `pp_ui_core` now owns a tested `NodeLifetimeTree` target model with checked node handles, scoped callback connections, subtree destruction, pointer/keyboard capture release, whole-tree clear for layout reload, and mutation-safe dispatch, plus a tested `UiOverlayLifetime` popup/dialog stack model. Retained app-dialog root insertion, app-menu popup template cloning/root attachment, and quick/stroke/brush panel popup root attachment are now centralized in `src/legacy_ui_overlay_services.*`, but retained `Node`/`NodePopupMenu`/`NodeDialog*` still have not adopted checked handles or scoped callback ownership | Preserve current UI behavior while making panel/dialog extraction safe instead of spreading lifetime hazards into the new architecture | `pp_ui_core_layout_xml_tests`; `pp_ui_core_node_lifetime_tests`; `pp_ui_core_overlay_lifetime_tests`; future `pp_panopainter_ui_dialog_lifetime_tests`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Retained `Node` and `pp_panopainter_ui` adopt checked node handles or equivalent non-owning references, scoped callback connection/disconnect semantics, mutation-safe event dispatch, parent/child invariants hidden behind APIs, and destroy-during-callback/capture-release/popup-close/layout-reload tests; retained `Node*` APIs are removed or isolated behind compatibility adapters | | DEBT-0057 | Open | Modernization | Default canvas allocation size now dispatches through `PlatformServices::default_canvas_resolution`, removing the `CANVAS_RES` platform macro from `src/canvas.h`; WebGL's retained 512 default now lives in tested `pp_platform_api::platform_policy`, but the Web shell still reaches it through the legacy platform fallback until injected Web services own the policy | Preserve WebGL memory behavior while moving canvas creation policy out of shared canvas headers and into the platform boundary | `pp_platform_api_tests`; `ctest --preset desktop-fast --build-config Debug -R pp_platform_api_tests`; Windows app build; WebGL package smoke once root Web build exists | Default canvas resolution is owned by injected `pp_platform_*` services for every supported platform, with no WebGL branch in the legacy fallback | | DEBT-0058 | Open | Modernization | App-level progress/message/input dialog metadata, including message-dialog OK/cancel captions, now consumes pure `pp_app_core` through `App::show_progress`, `App::message_box`, `App::input_box`, `pano_cli plan-app-dialog`, and `pp_app_core_app_dialog_tests`; live execution is centralized in `src/legacy_app_dialog_services.*`, and retained root insertion now routes through `src/legacy_ui_overlay_services.*`, but the bridge still creates retained `NodeProgressBar`, `NodeMessageBox`, and `NodeInputBox` instances with raw callback/lifetime ownership | Preserve current app-shell dialog behavior while moving shared dialog policy toward UI/app services | `pp_app_core_app_dialog_tests`; `pano_cli plan-app-dialog --kind progress --total -4`; `pano_cli plan-app-dialog --kind message --cancel`; `pano_cli plan-app-dialog --kind input --ok-caption Save`; `ctest --preset desktop-fast --build-config Debug`; Windows app build | Progress/message/input dialog creation, callback wiring, layout insertion, lifetime ownership, and headless automation are owned by injected app/UI services with `App` methods acting only as adapters | | DEBT-0059 | Open | Modernization | iOS root CMake headless builds assign generated bundle identifiers and disable code signing for executable test/tool targets | The current Apple gate is compile validation for shared component targets; signed iOS app/package validation is not migrated to root CMake yet | `powershell -ExecutionPolicy Bypass -File scripts\automation\apple-remote-build.ps1 -Presets macos,ios-simulator,ios-device`; `sh scripts/automation/platform-build.sh "ios-device"` on `panopainter-mac` | Root CMake owns the signed Apple app/package targets, package-smoke validates Apple bundles where signing material is available, and headless iOS test/tool targets are either excluded from signed package builds or use explicit test-runner signing policy | diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 1efa20a..8c5923a 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -483,7 +483,12 @@ browse/resize/layer-rename dialogs, PPBR export, shortcuts, and the what's-new remote page before those paths reach raw `Node` insertion. It also now owns retained app-menu popup template cloning and root attachment for File, Export, Edit, Tools, Panels, Options, About, and Layers menu popups, replacing the -previous direct template-child indexing in `App::init_menu_*`. +previous direct template-child indexing in `App::init_menu_*`. The helper now +also exposes retained root-popup attachment for `pp_panopainter_ui`, and the +quick-panel brush/color popups, stroke-panel preset/tip/dual/pattern popups, +and brush-panel preset menu root insertion route through it. Base retained +popup widgets such as `NodeComboBox`, raw popup callback captures, and close/ +capture semantics remain part of `DEBT-0063`. `pano_cli inspect-image` exposes PNG IHDR metadata as JSON, `pano_cli import-image` accepts a PNG path and imports decoded RGBA8 pixels into a new pure `pp_document` face payload, diff --git a/src/legacy_quick_ui_services.cpp b/src/legacy_quick_ui_services.cpp index af9597a..421e6a9 100644 --- a/src/legacy_quick_ui_services.cpp +++ b/src/legacy_quick_ui_services.cpp @@ -3,6 +3,7 @@ #include "legacy_quick_ui_services.h" #include "app.h" +#include "legacy_ui_overlay_services.h" #include "node_image.h" #include "node_stroke_preview.h" @@ -95,7 +96,7 @@ private: popup->SetHeight(glm::max(hh, 400.f)); popup->SetPositioning(YGPositionTypeAbsolute); popup->SetPosition(popup_pos); - panel_.root()->add_child(popup); + (void)attach_legacy_overlay_node_to_root(panel_, popup); panel_.root()->update(); popup->tick(0); @@ -153,7 +154,7 @@ private: popup->SetPositioning(YGPositionTypeAbsolute); popup->SetPosition(popup_pos); - panel_.root()->add_child(popup); + (void)attach_legacy_overlay_node_to_root(panel_, popup); panel_.root()->update(); popup->tick(0); diff --git a/src/legacy_ui_overlay_services.cpp b/src/legacy_ui_overlay_services.cpp index cd8c4aa..4973d69 100644 --- a/src/legacy_ui_overlay_services.cpp +++ b/src/legacy_ui_overlay_services.cpp @@ -32,6 +32,23 @@ pp::foundation::Status attach_legacy_overlay_node( return pp::foundation::Status::success(); } +pp::foundation::Status attach_legacy_overlay_node_to_root( + Node& anchor, + const std::shared_ptr& node) noexcept +{ + if (!node) { + return pp::foundation::Status::invalid_argument("legacy overlay node is null"); + } + + auto* root = anchor.root(); + if (!root) { + return pp::foundation::Status::invalid_argument("legacy overlay root is missing"); + } + + root->add_child(node); + return pp::foundation::Status::success(); +} + pp::foundation::Result> add_legacy_popup_menu( App& app, const char* template_id, diff --git a/src/legacy_ui_overlay_services.h b/src/legacy_ui_overlay_services.h index 549c3a3..3b49196 100644 --- a/src/legacy_ui_overlay_services.h +++ b/src/legacy_ui_overlay_services.h @@ -16,6 +16,10 @@ void initialize_legacy_overlay_node(App& app, Node& node); App& app, const std::shared_ptr& node) noexcept; +[[nodiscard]] pp::foundation::Status attach_legacy_overlay_node_to_root( + Node& anchor, + const std::shared_ptr& node) noexcept; + [[nodiscard]] pp::foundation::Result> add_legacy_popup_menu( App& app, const char* template_id, diff --git a/src/node_panel_brush.cpp b/src/node_panel_brush.cpp index c633810..32d51e5 100644 --- a/src/node_panel_brush.cpp +++ b/src/node_panel_brush.cpp @@ -4,6 +4,7 @@ #include "assets/brush_package.h" #include "app_core/brush_ui.h" #include "legacy_brush_ui_services.h" +#include "legacy_ui_overlay_services.h" #include "asset.h" #include "texture.h" @@ -579,7 +580,7 @@ void NodePanelBrushPreset::init() m_btn_menu->on_click = [this](Node* b) { auto popup = add_child_file("data/dialogs/panel-brushes.xml", "tpl-brush-popup"); popup->SetPosition(b->m_pos.x + b->m_size.x, b->m_pos.y); - root()->add_child(popup); + (void)pp::panopainter::attach_legacy_overlay_node_to_root(*this, popup); root()->update(); auto bounds = root()->GetSize() - zw(popup->get_children_rect()); popup->SetPosition(glm::clamp(popup->m_pos, { 0, 0 }, bounds)); diff --git a/src/node_panel_stroke.cpp b/src/node_panel_stroke.cpp index 4404988..9083db5 100644 --- a/src/node_panel_stroke.cpp +++ b/src/node_panel_stroke.cpp @@ -1,6 +1,7 @@ #include "pch.h" #include "app_core/brush_ui.h" #include "legacy_brush_ui_services.h" +#include "legacy_ui_overlay_services.h" #include "log.h" #include "node_panel_stroke.h" #include "canvas.h" @@ -316,7 +317,7 @@ void NodePanelStroke::init_controls() m_preset_button->on_click = [this](Node*) { auto screen = root()->m_size; glm::vec2 pos = m_preset_button->m_pos + glm::vec2(m_preset_button->m_size.x, 0); - root()->add_child(App::I->presets); + (void)pp::panopainter::attach_legacy_overlay_node_to_root(*this, App::I->presets); auto tick = root()->add_child(); tick->SetPositioning(YGPositionTypeAbsolute); tick->SetSize(16, 32); @@ -367,7 +368,7 @@ void NodePanelStroke::init_controls() m_brush_button->on_click = [this](Node*) { auto screen = root()->m_size; glm::vec2 pos = m_brush_button->m_pos + glm::vec2(m_brush_button->m_size.x, 0); - root()->add_child(m_brush_popup); + (void)pp::panopainter::attach_legacy_overlay_node_to_root(*this, m_brush_popup); auto tick = root()->add_child(); tick->SetPositioning(YGPositionTypeAbsolute); tick->SetSize(16, 32); @@ -403,7 +404,7 @@ void NodePanelStroke::init_controls() m_dual_brush_button->on_click = [this](Node*) { auto screen = root()->m_size; glm::vec2 pos = m_dual_brush_button->m_pos + glm::vec2(m_dual_brush_button->m_size.x, 0); - root()->add_child(m_brush_popup); + (void)pp::panopainter::attach_legacy_overlay_node_to_root(*this, m_brush_popup); auto tick = root()->add_child(); tick->SetPositioning(YGPositionTypeAbsolute); tick->SetSize(16, 32); @@ -439,7 +440,7 @@ void NodePanelStroke::init_controls() m_pattern_button->on_click = [this](Node*) { auto screen = root()->m_size; glm::vec2 pos = m_pattern_button->m_pos + glm::vec2(m_pattern_button->m_size.x, 0); - root()->add_child(m_pattern_popup); + (void)pp::panopainter::attach_legacy_overlay_node_to_root(*this, m_pattern_popup); auto tick = root()->add_child(); tick->SetPositioning(YGPositionTypeAbsolute); tick->SetSize(16, 32);