From e808018e53ff692f035a889e7a30a5666a4af8df Mon Sep 17 00:00:00 2001 From: omigamedev Date: Wed, 17 Jun 2026 22:41:25 +0200 Subject: [PATCH] Stabilize startup and cloud dialog runtime --- docs/modernization/build-inventory.md | 5 + docs/modernization/debt.md | 12 + docs/modernization/roadmap.md | 6 + docs/modernization/tasks.md | 6 + scripts/automation/run-debugger.ps1 | 112 ++++++ src/legacy_file_menu_binding_services.cpp | 429 ++++++++++------------ src/legacy_ui_node_loader.cpp | 2 +- src/node_combobox.cpp | 87 ++++- src/node_dialog_cloud.cpp | 116 ++++-- src/node_dialog_cloud.h | 12 +- 10 files changed, 510 insertions(+), 277 deletions(-) create mode 100644 scripts/automation/run-debugger.ps1 diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index fd5bdee2..4ad3e10d 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -68,6 +68,7 @@ powershell -ExecutionPolicy Bypass -File scripts\automation\quiet-validate.ps1 - powershell -ExecutionPolicy Bypass -File scripts\automation\quiet-validate.ps1 -BuildTargets pp_app_core_app_dialog_tests,pp_ui_core_overlay_lifetime_tests -TestRegex "pp_(app_core_app_dialog|ui_core_(node_lifetime|overlay_lifetime))" powershell -ExecutionPolicy Bypass -File scripts\automation\quiet-validate.ps1 -BuildTargets PanoPainter,pano_cli -TestRegex "pp_app_core|pano_cli_plan" -IncludePlatformBuild powershell -ExecutionPolicy Bypass -File scripts\automation\quiet-validate.ps1 -BuildTargets PanoPainter,pano_cli -TestRegex "pp_app_core|pano_cli_plan" -IncludePlatformBuild -IncludeAppleRemote +powershell -ExecutionPolicy Bypass -File scripts\automation\run-debugger.ps1 -BreakOnFirstChanceAccessViolation ``` Use the standalone quiet helpers only when you need to isolate those gates from @@ -75,6 +76,10 @@ the bundled run. `platform-build.ps1 -Quiet` writes per-preset logs under `out/logs/platform-build`. `apple-remote-build.ps1 -Quiet` writes the local SSH session log under `out/logs/apple-remote-build` and reports the remote `out/logs/apple-platform-build-*.log` path in its JSON output. +`run-debugger.ps1` is the repeatable Windows startup-debug helper for local +`PanoPainter.exe` sessions; it resolves `cdb.exe`, writes a command file and +log under `out/logs/debugger`, and can break on first-chance access violations +without relying on fragile shell quoting. On Windows, the quiet wrapper is also the safest generator-compatibility path: it prefers the VS-bundled CMake that knows the `Visual Studio 18 2026` diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index ba0c5713..5bc5ee11 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -77,6 +77,18 @@ agent or engineer to remove them without reconstructing context from chat. through `AppRuntime::canvas_async_task` instead of a file-static worker singleton, while retained prompt/progress lifetime, OpenGL context guards, thumbnail loading, and transfer execution still remain in the cloud bridge. +- 2026-06-17: `DEBT-0038` was narrowed again. The retained cloud-browse dialog + in `src/node_dialog_cloud.cpp` no longer mutates the legacy UI tree directly + from its thumbnail loader worker; file-list population, error text updates, + and thumbnail attachment now queue onto `AppRuntime`'s UI task path and drop + safely when the dialog is already closed, reducing a cancel-time deadlock + risk while the broader retained cloud dialog and transfer flow still remain. +- 2026-06-17: `DEBT-0031`/`DEBT-0030` were narrowed again. + `src/legacy_file_menu_binding_services.cpp` no longer stores File-menu popup + callbacks that capture a stack-local binding service object through `this`; + retained File-menu and export-submenu actions now capture explicit `App&`, + popup root, and overlay handles, removing a startup/runtime lifetime hazard + while retained file/export execution still lives in the app shell. - 2026-06-17: `DEBT-0048` was narrowed again. The retained ABR/PPBR import path in `src/legacy_brush_package_import_services.cpp` now uses `AppRuntime::canvas_async_task` instead of a file-static worker singleton, diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 54a11b07..79883e29 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -100,6 +100,12 @@ Current conclusion: `src/app_layout_sidebar.cpp`, and `src/app_dialogs_info_openers.cpp` are thinner adapters even though broader retained dialog/sidebar execution still remains. +- Startup stability improved materially: the legacy UI loader now uses virtual + attribute parsing again, `NodeComboBox` no longer trusts invalid/empty item + state, the extracted File-menu binding no longer stores callbacks that capture + a dead stack service object, and the cloud-browse dialog now queues thumbnail + list/icon updates onto the UI thread instead of mutating the legacy UI tree + directly from its worker thread. - Platform extraction improved substantially and the root app source group no longer compiles Web platform sources directly, but broader CMake and entrypoint cleanup are not complete. diff --git a/docs/modernization/tasks.md b/docs/modernization/tasks.md index 69af6b8a..bd3f1470 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -138,6 +138,12 @@ Key facts: document export start/branching flows live in `src/legacy_document_export_services.*`, and the PPBR dialog opener now lives in `src/legacy_brush_package_export_services.*`. +- The startup/runtime stability slice narrowed several live risks at once: the + legacy UI loader again routes XML attributes through virtual node parsers, + `NodeComboBox` now guards empty and out-of-range item state, the extracted + File-menu binding no longer leaves click callbacks pointing at a dead stack + service object, and the cloud-browse dialog now queues file-list/thumbnail UI + updates onto the UI thread instead of mutating nodes directly from its worker. ## Parallel Assignment Rules diff --git a/scripts/automation/run-debugger.ps1 b/scripts/automation/run-debugger.ps1 new file mode 100644 index 00000000..f13504be --- /dev/null +++ b/scripts/automation/run-debugger.ps1 @@ -0,0 +1,112 @@ +[CmdletBinding()] +param( + [string]$BuildPreset = "windows-msvc-default", + [string]$Configuration = "Debug", + [string]$Executable = "", + [string]$DebuggerCommand = "", + [string]$LogDir = "out/logs/debugger", + [int]$StartupSmokeSeconds = 20, + [switch]$BreakOnFirstChanceAccessViolation, + [switch]$LeaveRunning +) + +$ErrorActionPreference = "Stop" + +function Resolve-CdbPath { + if ($DebuggerCommand.Length -gt 0) { + return $DebuggerCommand + } + + $candidates = @( + "C:\Program Files (x86)\Windows Kits\10\Debuggers\x64\cdb.exe", + "C:\Program Files\Windows Kits\10\Debuggers\x64\cdb.exe" + ) + + foreach ($candidate in $candidates) { + if (Test-Path -LiteralPath $candidate) { + return $candidate + } + } + + throw "Unable to find cdb.exe. Install the Windows Debugging Tools or pass -DebuggerCommand." +} + +function Resolve-ExecutablePath { + param( + [string]$Requested, + [string]$Preset, + [string]$Config + ) + + if ($Requested.Length -gt 0) { + return (Resolve-Path -LiteralPath $Requested).Path + } + + $candidate = Join-Path -Path "out/build/$Preset/$Config" -ChildPath "PanoPainter.exe" + if (Test-Path -LiteralPath $candidate) { + return (Resolve-Path -LiteralPath $candidate).Path + } + + throw "Unable to find PanoPainter.exe at '$candidate'. Pass -Executable to override." +} + +function New-DebuggerCommandFile { + param( + [string]$Path, + [bool]$BreakOnFirstChanceAccessViolation + ) + + $lines = @() + if ($BreakOnFirstChanceAccessViolation) { + $lines += 'sxe -c ".echo ==== FIRST CHANCE AV ====; .ecxr; kb; kv; q" av' + } + $lines += "g" + Set-Content -LiteralPath $Path -Value $lines +} + +$cdb = Resolve-CdbPath +$exe = Resolve-ExecutablePath -Requested $Executable -Preset $BuildPreset -Config $Configuration + +New-Item -ItemType Directory -Path $LogDir -Force | Out-Null + +$timestamp = Get-Date -Format "yyyyMMdd-HHmmss" +$commandFile = Join-Path -Path $LogDir -ChildPath "$timestamp-cdb.cmd" +$logPath = Join-Path -Path $LogDir -ChildPath "$timestamp-cdb.log" + +New-DebuggerCommandFile -Path $commandFile -BreakOnFirstChanceAccessViolation:$BreakOnFirstChanceAccessViolation + +$process = Start-Process -FilePath $cdb -ArgumentList @( + "-lines", + "-logo", $logPath, + "-cf", $commandFile, + $exe +) -PassThru + +Start-Sleep -Seconds $StartupSmokeSeconds + +$app = Get-Process PanoPainter -ErrorAction SilentlyContinue +$debugger = Get-Process cdb -ErrorAction SilentlyContinue | Where-Object { $_.Id -eq $process.Id } + +$summary = [ordered]@{ + debugger = $cdb + executable = $exe + commandFile = $commandFile + log = $logPath + smokeSeconds = $StartupSmokeSeconds + breakOnFirstChanceAccessViolation = [bool]$BreakOnFirstChanceAccessViolation + debuggerRunning = [bool]$debugger + appRunning = [bool]$app + appResponding = if ($app) { [bool]$app.Responding } else { $false } + mainWindowTitle = if ($app) { $app.MainWindowTitle } else { "" } +} + +$summary | ConvertTo-Json -Compress + +if (-not $LeaveRunning) { + if ($app) { + Stop-Process -Id $app.Id -Force + } + if ($debugger) { + Stop-Process -Id $debugger.Id -Force + } +} diff --git a/src/legacy_file_menu_binding_services.cpp b/src/legacy_file_menu_binding_services.cpp index 170c23a7..910a2cb2 100644 --- a/src/legacy_file_menu_binding_services.cpp +++ b/src/legacy_file_menu_binding_services.cpp @@ -38,249 +38,210 @@ void close_legacy_overlay_handle_ignoring_status( (void)pp::panopainter::close_legacy_overlay_node(anchor, overlay); } +void apply_file_menu_command(App& app, pp::app::FileMenuCommand command) +{ + pp::panopainter::apply_legacy_file_menu_command(app, command); +} + +void apply_document_export_menu(App& app, pp::app::DocumentExportMenuKind kind) +{ + (void)pp::panopainter::apply_legacy_document_export_menu_plan(app, kind); +} + +void close_popup(Node& popup_root, pp::ui::NodeHandle overlay) noexcept +{ + close_legacy_overlay_handle_ignoring_status(popup_root, overlay); +} + +void bind_export_submenu_button( + App& app, + Node& popup_root, + NodePopupMenu& subpopup, + const char* button_id, + pp::app::DocumentExportMenuKind kind, + pp::ui::NodeHandle popup_handle, + pp::ui::NodeHandle subpopup_handle) +{ + if (auto* b = subpopup.find(button_id)) { + b->on_click = [&app, &popup_root, kind, popup_handle, subpopup_handle](Node*) { + apply_document_export_menu(app, kind); + close_popup(popup_root, popup_handle); + close_popup(popup_root, subpopup_handle); + }; + } +} + +void bind_export_submenu_wiring( + App& app, + Node& popup_root, + NodePopupMenu& subpopup, + pp::ui::NodeHandle popup_handle, + pp::ui::NodeHandle subpopup_handle) +{ + bind_export_submenu_button( + app, + popup_root, + subpopup, + "file-submenu-export-png", + pp::app::DocumentExportMenuKind::png, + popup_handle, + subpopup_handle); + bind_export_submenu_button( + app, + popup_root, + subpopup, + "file-submenu-export-layers", + pp::app::DocumentExportMenuKind::layers, + popup_handle, + subpopup_handle); + bind_export_submenu_button( + app, + popup_root, + subpopup, + "file-submenu-export-cube", + pp::app::DocumentExportMenuKind::cube_faces, + popup_handle, + subpopup_handle); + bind_export_submenu_button( + app, + popup_root, + subpopup, + "file-submenu-export-depth", + pp::app::DocumentExportMenuKind::depth, + popup_handle, + subpopup_handle); + bind_export_submenu_button( + app, + popup_root, + subpopup, + "file-submenu-export-anim", + pp::app::DocumentExportMenuKind::animation_frames, + popup_handle, + subpopup_handle); + bind_export_submenu_button( + app, + popup_root, + subpopup, + "file-submenu-export-anim-mp4", + pp::app::DocumentExportMenuKind::animation_mp4, + popup_handle, + subpopup_handle); + bind_export_submenu_button( + app, + popup_root, + subpopup, + "file-submenu-export-timelapse", + pp::app::DocumentExportMenuKind::timelapse, + popup_handle, + subpopup_handle); +} + +void open_export_submenu( + App& app, + Node& popup_root, + NodeButtonCustom& export_button, + pp::ui::NodeHandle popup_handle) +{ + const glm::vec2 pos = export_button.m_pos + glm::vec2(export_button.m_size.x, 0); + const auto subpopup = add_menu_popup( + app, + "file-submenu-export", + pos, + export_button.m_size.x); + if (!subpopup) { + return; + } + + pp::panopainter::detach_legacy_node_from_parent(*subpopup); + const auto subpopup_overlay = pp::panopainter::open_legacy_overlay_node_with_handle(popup_root, subpopup); + if (!subpopup_overlay) { + pp::panopainter::destroy_legacy_node(*subpopup); + return; + } + + bind_export_submenu_wiring( + app, + popup_root, + *subpopup, + popup_handle, + subpopup_overlay.value()); +} + +void bind_popup_action( + App& app, + Node& popup_root, + NodePopupMenu& popup, + const char* button_id, + pp::app::FileMenuCommand command, + pp::ui::NodeHandle popup_handle) +{ + if (auto* b = popup.find(button_id)) { + b->on_click = [&app, &popup_root, command, popup_handle](Node*) { + apply_file_menu_command(app, command); + close_popup(popup_root, popup_handle); + }; + } +} + +void bind_popup_wiring( + App& app, + Node& popup_root, + NodePopupMenu& popup, + pp::ui::NodeHandle popup_handle) +{ + bind_popup_action(app, popup_root, popup, "file-newdoc", pp::app::FileMenuCommand::new_document, popup_handle); + bind_popup_action(app, popup_root, popup, "file-import", pp::app::FileMenuCommand::import_image, popup_handle); + bind_popup_action(app, popup_root, popup, "file-open", pp::app::FileMenuCommand::open_project, popup_handle); + bind_popup_action(app, popup_root, popup, "file-browse", pp::app::FileMenuCommand::browse_cloud, popup_handle); + bind_popup_action(app, popup_root, popup, "file-save", pp::app::FileMenuCommand::save, popup_handle); + bind_popup_action(app, popup_root, popup, "file-save-as", pp::app::FileMenuCommand::save_as, popup_handle); + bind_popup_action(app, popup_root, popup, "file-save-ver", pp::app::FileMenuCommand::save_version, popup_handle); + bind_popup_action(app, popup_root, popup, "file-export", pp::app::FileMenuCommand::export_jpeg, popup_handle); + bind_popup_action(app, popup_root, popup, "file-share", pp::app::FileMenuCommand::share, popup_handle); + bind_popup_action(app, popup_root, popup, "file-resize", pp::app::FileMenuCommand::resize, popup_handle); + bind_popup_action(app, popup_root, popup, "file-cloud-upload", pp::app::FileMenuCommand::cloud_upload, popup_handle); + bind_popup_action(app, popup_root, popup, "file-cloud-browse", pp::app::FileMenuCommand::cloud_browse, popup_handle); + + if (auto* b = popup.find("file-export-tick")) { + b->on_click = [&app, &popup_root, b, popup_handle](Node*) { + open_export_submenu(app, popup_root, *b, popup_handle); + }; + } +} + +void open_file_menu_popup( + App& app, + Node& popup_root, + NodeButtonCustom& menu_file) +{ + const glm::vec2 pos = menu_file.m_pos + glm::vec2(0, menu_file.m_size.y); + const auto popup = add_menu_popup(app, "file-menu", pos, menu_file.m_size.x); + if (!popup) { + return; + } + + pp::panopainter::detach_legacy_node_from_parent(*popup); + const auto popup_overlay = pp::panopainter::open_legacy_overlay_node_with_handle(popup_root, popup); + if (!popup_overlay) { + pp::panopainter::destroy_legacy_node(*popup); + return; + } + + bind_popup_wiring(app, popup_root, *popup, popup_overlay.value()); +} + } // namespace namespace pp::panopainter { -namespace { - -class LegacyFileMenuBindingServices final { -public: - LegacyFileMenuBindingServices(App& app, Node& popup_root) noexcept - : app_(app) - , popup_root_(popup_root) - { - } - - void bind_menu_button(NodeButtonCustom& menu_file) - { - menu_file.on_click = [this, &menu_file](Node*) { - open_file_menu_popup(menu_file); - }; - } - -private: - void open_file_menu_popup(NodeButtonCustom& menu_file) - { - const glm::vec2 pos = menu_file.m_pos + glm::vec2(0, menu_file.m_size.y); - const auto popup = add_menu_popup(app_, "file-menu", pos, menu_file.m_size.x); - if (!popup) { - return; - } - - pp::panopainter::detach_legacy_node_from_parent(*popup); - const auto popup_overlay = pp::panopainter::open_legacy_overlay_node_with_handle(popup_root_, popup); - if (!popup_overlay) { - pp::panopainter::destroy_legacy_node(*popup); - return; - } - - bind_popup_wiring(*popup, popup_overlay.value()); - } - - void bind_popup_wiring( - NodePopupMenu& popup, - pp::ui::NodeHandle popup_handle) - { - if (auto* b = popup.find("file-newdoc")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::new_document); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-import")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::import_image); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-open")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::open_project); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-browse")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::browse_cloud); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-save")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::save); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-save-as")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::save_as); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-save-ver")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::save_version); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-export")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::export_jpeg); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-export-tick")) { - b->on_click = [this, b, popup_handle](Node*) { - open_export_submenu(*b, popup_handle); - }; - } - if (auto* b = popup.find("file-share")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::share); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-resize")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::resize); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-cloud-upload")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::cloud_upload); - close_popup(popup_handle); - }; - } - if (auto* b = popup.find("file-cloud-browse")) { - b->on_click = [this, popup_handle](Node*) { - apply_file_menu_command(pp::app::FileMenuCommand::cloud_browse); - close_popup(popup_handle); - }; - } - } - - void open_export_submenu( - NodeButtonCustom& export_button, - pp::ui::NodeHandle popup_handle) - { - const glm::vec2 pos = export_button.m_pos + glm::vec2(export_button.m_size.x, 0); - const auto subpopup = add_menu_popup( - app_, - "file-submenu-export", - pos, - export_button.m_size.x); - if (!subpopup) { - return; - } - - pp::panopainter::detach_legacy_node_from_parent(*subpopup); - const auto subpopup_overlay = pp::panopainter::open_legacy_overlay_node_with_handle(popup_root_, subpopup); - if (!subpopup_overlay) { - pp::panopainter::destroy_legacy_node(*subpopup); - return; - } - - bind_export_submenu_wiring( - *subpopup, - popup_handle, - subpopup_overlay.value()); - } - - void bind_export_submenu_wiring( - NodePopupMenu& subpopup, - pp::ui::NodeHandle popup_handle, - pp::ui::NodeHandle subpopup_handle) - { - bind_export_submenu_button( - subpopup, - "file-submenu-export-png", - pp::app::DocumentExportMenuKind::png, - popup_handle, - subpopup_handle); - bind_export_submenu_button( - subpopup, - "file-submenu-export-layers", - pp::app::DocumentExportMenuKind::layers, - popup_handle, - subpopup_handle); - bind_export_submenu_button( - subpopup, - "file-submenu-export-cube", - pp::app::DocumentExportMenuKind::cube_faces, - popup_handle, - subpopup_handle); - bind_export_submenu_button( - subpopup, - "file-submenu-export-depth", - pp::app::DocumentExportMenuKind::depth, - popup_handle, - subpopup_handle); - bind_export_submenu_button( - subpopup, - "file-submenu-export-anim", - pp::app::DocumentExportMenuKind::animation_frames, - popup_handle, - subpopup_handle); - bind_export_submenu_button( - subpopup, - "file-submenu-export-anim-mp4", - pp::app::DocumentExportMenuKind::animation_mp4, - popup_handle, - subpopup_handle); - bind_export_submenu_button( - subpopup, - "file-submenu-export-timelapse", - pp::app::DocumentExportMenuKind::timelapse, - popup_handle, - subpopup_handle); - } - - void bind_export_submenu_button( - NodePopupMenu& subpopup, - const char* button_id, - pp::app::DocumentExportMenuKind kind, - pp::ui::NodeHandle popup_handle, - pp::ui::NodeHandle subpopup_handle) - { - if (auto* b = subpopup.find(button_id)) { - b->on_click = [this, kind, popup_handle, subpopup_handle](Node*) { - apply_document_export_menu(kind); - close_popup(popup_handle); - close_popup(subpopup_handle); - }; - } - } - - void apply_file_menu_command(pp::app::FileMenuCommand command) - { - pp::panopainter::apply_legacy_file_menu_command(app_, command); - } - - void apply_document_export_menu(pp::app::DocumentExportMenuKind kind) - { - (void)pp::panopainter::apply_legacy_document_export_menu_plan(app_, kind); - } - - void close_popup(pp::ui::NodeHandle overlay) noexcept - { - close_legacy_overlay_handle_ignoring_status(popup_root_, overlay); - } - - App& app_; - Node& popup_root_; -}; - -} // namespace - void bind_legacy_file_menu_popup( App& app, NodeButtonCustom& menu_file, Node& popup_root) { - LegacyFileMenuBindingServices services(app, popup_root); - services.bind_menu_button(menu_file); + menu_file.on_click = [&app, &popup_root, &menu_file](Node*) { + open_file_menu_popup(app, popup_root, menu_file); + }; } } // namespace pp::panopainter diff --git a/src/legacy_ui_node_loader.cpp b/src/legacy_ui_node_loader.cpp index d5836281..05438d4c 100644 --- a/src/legacy_ui_node_loader.cpp +++ b/src/legacy_ui_node_loader.cpp @@ -165,7 +165,7 @@ void load_legacy_ui_node(Node& node, const tinyxml2::XMLElement& x_node, bool sk auto attr = x_node.FirstAttribute(); while (attr) { - parse_legacy_ui_node_attribute(node, (kAttribute)const_hash(attr->Name()), attr); + node.parse_attributes((kAttribute)const_hash(attr->Name()), attr); attr = attr->Next(); } diff --git a/src/node_combobox.cpp b/src/node_combobox.cpp index 274c58bc..56b7f166 100644 --- a/src/node_combobox.cpp +++ b/src/node_combobox.cpp @@ -4,6 +4,38 @@ #include "legacy_ui_overlay_services.h" #include "node_popup_menu.h" +namespace { + +int clamp_combobox_item_index(const NodeComboBox& combo) noexcept +{ + if (combo.m_items.empty()) { + return 0; + } + + if (combo.m_current_index < 0) { + return 0; + } + + const auto max_index = static_cast(combo.m_items.size()) - 1; + return combo.m_current_index > max_index ? max_index : combo.m_current_index; +} + +const std::string* find_combobox_item(const NodeComboBox& combo, int index) noexcept +{ + if (index < 0) { + return nullptr; + } + + const auto item_index = static_cast(index); + if (item_index >= combo.m_items.size()) { + return nullptr; + } + + return &combo.m_items[item_index]; +} + +} // namespace + Node* NodeComboBox::clone_instantiate() const { return new NodeComboBox; @@ -22,8 +54,26 @@ void NodeComboBox::clone_copy(Node* dest) const void NodeComboBox::loaded() { NodeButton::loaded(); - m_text->set_text(m_data[m_current_index].c_str()); - m_selected_child_index = m_current_index; + if (m_items.empty()) { + LOG("NodeComboBox '%s' loaded with an empty item list", m_nodeID_s.c_str()); + m_current_index = 0; + m_selected_child_index = 0; + m_text->set_text(""); + } else { + const auto clamped_index = clamp_combobox_item_index(*this); + if (clamped_index != m_current_index) { + LOG( + "NodeComboBox '%s' default index %d out of range for %zu items; clamping to %d", + m_nodeID_s.c_str(), + m_current_index, + m_items.size(), + clamped_index); + m_current_index = clamped_index; + } + + m_text->set_text(m_items[static_cast(m_current_index)].c_str()); + m_selected_child_index = m_current_index; + } on_click = [this](Node* target) { auto popup = std::make_shared(); popup->set_manager(m_manager); @@ -111,23 +161,48 @@ void NodeComboBox::parse_attributes(kAttribute ka, const tinyxml2::XMLAttribute* void NodeComboBox::set_index(int index) { + if (m_items.empty()) { + LOG("NodeComboBox '%s' set_index(%d) ignored because the item list is empty", m_nodeID_s.c_str(), index); + m_current_index = 0; + m_selected_child_index = 0; + m_text->set_text(""); + return; + } + m_current_index = index; - m_text->set_text(m_items[index].c_str()); + const auto clamped_index = clamp_combobox_item_index(*this); + if (clamped_index != m_current_index) { + LOG( + "NodeComboBox '%s' set_index(%d) out of range for %zu items; clamping to %d", + m_nodeID_s.c_str(), + index, + m_items.size(), + clamped_index); + m_current_index = clamped_index; + } + + m_text->set_text(m_items[static_cast(m_current_index)].c_str()); //if (on_select) // on_select(this, index); } float NodeComboBox::get_float(int index) const noexcept { - return std::stof(m_data[index]); + if (const auto* item = find_combobox_item(*this, index)) { + return std::stof(*item); + } + return 0.f; } float NodeComboBox::get_float() const noexcept { - return std::stof(m_data[m_current_index]); + return get_float(m_current_index); } int NodeComboBox::get_int() const noexcept { - return std::stoi(m_data[m_current_index]); + if (const auto* item = find_combobox_item(*this, m_current_index)) { + return std::stoi(*item); + } + return 0; } diff --git a/src/node_dialog_cloud.cpp b/src/node_dialog_cloud.cpp index a2c711bd..2d6a8503 100644 --- a/src/node_dialog_cloud.cpp +++ b/src/node_dialog_cloud.cpp @@ -12,12 +12,12 @@ namespace { -bool load_cloud_thumb(CURL* curl, const std::string& name, NodeDialogCloudItem* node, std::string& response) +bool load_cloud_thumb(CURL* curl, const std::string& name, std::string& response, Image& thumb) { response.clear(); char* url_escaped = curl_easy_escape(curl, name.c_str(), (int)name.size()); std::string url = std::string("https://panopainter.com/cloud/cloud-info.php?file=") + url_escaped; - delete url_escaped; + curl_free(url_escaped); curl_easy_setopt(curl, CURLOPT_URL, url.c_str()); LOG("%s", url.c_str()); auto err = curl_easy_perform(curl); @@ -32,15 +32,8 @@ bool load_cloud_thumb(CURL* curl, const std::string& name, NodeDialogCloudItem* std::string rgb; rgb.resize(Base64::DecodedLength(info[3])); Base64::Decode(info[3], &rgb); - Image thumb; thumb.create(width, height); thumb.copy_from((uint8_t*)rgb.data()); - - auto image_tex = node->find("thumb-tex"); - image_tex->tex = std::make_shared(); - image_tex->tex->create(thumb); - - node->app_redraw(); return true; } } // namespace @@ -68,8 +61,10 @@ void NodeDialogCloud::init_controls() btn_cancel = find("btn-cancel"); pp::panopainter::bind_legacy_click_destroys_node(*btn_cancel, *this); container = find("files-list"); - load_thumbs_worker_ = std::jthread([this](std::stop_token stop) { - load_thumbs_thread(stop); + loading_status_container_ = create_loading_status_text()->m_parent; + auto self = std::static_pointer_cast(shared_from_this()); + load_thumbs_worker_ = std::jthread([self](std::stop_token stop) { + self->load_thumbs_thread(stop); }); } @@ -80,7 +75,8 @@ void NodeDialogCloud::loaded() void NodeDialogCloud::removed(Node* parent) { NodeBorder::removed(parent); - closed = true; + closed.store(true, std::memory_order_release); + items_by_name_.clear(); load_thumbs_worker_.request_stop(); } @@ -97,7 +93,7 @@ NodeText* NodeDialogCloud::create_loading_status_text() return text; } -bool NodeDialogCloud::load_cloud_file_list(CURL* curl, std::string& response, NodeText& status_text) +bool NodeDialogCloud::load_cloud_file_list(CURL* curl, std::string& response) { curl_easy_setopt(curl, CURLOPT_WRITEDATA, &response); curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, curl_data_handler); @@ -109,17 +105,39 @@ bool NodeDialogCloud::load_cloud_file_list(CURL* curl, std::string& response, No if (err != CURLE_OK) { LOG("connection error: %d", err); - status_text.set_text("Could not connect to the server"); return false; } return true; } -std::vector NodeDialogCloud::create_cloud_file_items(const std::vector& names) +void NodeDialogCloud::show_cloud_connection_error() { - std::vector nodes; - nodes.reserve(names.size()); + if (closed.load(std::memory_order_acquire)) { + return; + } + + if (loading_status_container_ == nullptr || loading_status_container_->m_children.empty()) { + return; + } + + if (auto* text = dynamic_cast(loading_status_container_->m_children[0].get())) { + text->set_text("Could not connect to the server"); + } +} + +void NodeDialogCloud::populate_cloud_file_items(const std::vector& names) +{ + if (closed.load(std::memory_order_acquire)) { + return; + } + + if (loading_status_container_ != nullptr && loading_status_container_->m_parent != nullptr) { + pp::panopainter::destroy_legacy_node(*loading_status_container_); + loading_status_container_ = nullptr; + } + + items_by_name_.clear(); for (const auto& name : names) { @@ -140,10 +158,35 @@ std::vector NodeDialogCloud::create_cloud_file_items(const current->m_selected = false; current = target; }; - nodes.push_back(node); + items_by_name_[name] = std::static_pointer_cast(node->shared_from_this()); + } +} + +void NodeDialogCloud::apply_cloud_thumb(const std::string& name, const Image& thumb) +{ + if (closed.load(std::memory_order_acquire)) { + return; } - return nodes; + const auto it = items_by_name_.find(name); + if (it == items_by_name_.end()) { + return; + } + + const auto node = it->second.lock(); + if (!node) { + items_by_name_.erase(it); + return; + } + + auto* image_tex = node->find("thumb-tex"); + if (!image_tex) { + return; + } + + image_tex->tex = std::make_shared(); + image_tex->tex->create(thumb); + node->app_redraw(); } void NodeDialogCloud::load_thumbs_thread(std::stop_token stop) @@ -154,37 +197,44 @@ void NodeDialogCloud::load_thumbs_thread(std::stop_token stop) std::string res; if (curl) { - if (stop.stop_requested() || closed) + if (stop.stop_requested() || closed.load(std::memory_order_acquire)) return; - auto* text = create_loading_status_text(); - auto* align = text->m_parent; - - if (!load_cloud_file_list(curl.get(), res, *text)) + if (!load_cloud_file_list(curl.get(), res)) { + auto self = std::static_pointer_cast(shared_from_this()); + App::I->runtime().ui_task_async([self] { + self->show_cloud_connection_error(); + }); return; } - - pp::panopainter::destroy_legacy_node(*align); - if (stop.stop_requested() || closed) + if (stop.stop_requested() || closed.load(std::memory_order_acquire)) return; LOG("CLOUD LIST: %s", res.c_str()); auto names = split(res, ','); - auto nodes = create_cloud_file_items(names); + auto self = std::static_pointer_cast(shared_from_this()); + App::I->runtime().ui_task_async([self, names] { + self->populate_cloud_file_items(names); + }); // load the icons - for (int i = 0; i < names.size(); i++) + for (const auto& n : names) { - const auto& n = names[i]; - auto* node = nodes[i]; - if (stop.stop_requested() || closed) + if (stop.stop_requested() || closed.load(std::memory_order_acquire)) break; - if (!load_cloud_thumb(curl.get(), n, node, res)) + Image thumb; + if (!load_cloud_thumb(curl.get(), n, res, thumb)) break; + + auto dialog = std::static_pointer_cast(shared_from_this()); + auto thumb_ptr = std::make_shared(std::move(thumb)); + App::I->runtime().ui_task_async([dialog, name = n, thumb_ptr] { + dialog->apply_cloud_thumb(name, *thumb_ptr); + }); } } #endif //CURL diff --git a/src/node_dialog_cloud.h b/src/node_dialog_cloud.h index a389a0c1..03e6b57c 100644 --- a/src/node_dialog_cloud.h +++ b/src/node_dialog_cloud.h @@ -5,6 +5,8 @@ #include "node_text.h" #include "node_text_input.h" +#include +#include #include #include #include @@ -33,7 +35,7 @@ public: class NodeDialogCloud : public NodeBorder { public: - bool closed = false; + std::atomic_bool closed = false; NodeButton* btn_cancel; NodeButton* btn_ok; NodeButton* btn_delete; @@ -50,9 +52,13 @@ public: virtual void removed(Node* parent) override; void load_thumbs_thread(std::stop_token stop); NodeText* create_loading_status_text(); - bool load_cloud_file_list(CURL* curl, std::string& response, NodeText& status_text); - std::vector create_cloud_file_items(const std::vector& names); + bool load_cloud_file_list(CURL* curl, std::string& response); + void show_cloud_connection_error(); + void populate_cloud_file_items(const std::vector& names); + void apply_cloud_thumb(const std::string& name, const Image& thumb); private: std::jthread load_thumbs_worker_; + Node* loading_status_container_ = nullptr; + std::unordered_map> items_by_name_; };