diff --git a/README.md b/README.md index 12b013c..c3181b2 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,20 @@ it that reads as a hang. Each allocation also names itself on stderr (`[udpscope] buffer allocated: 4 channels, 839 s span, 74 ms`), so a run that prints a column of them is a re-plan that should have been coalesced. +**And a loading window covers the gap the status line cannot.** That notice only exists once there is +a window to put it in, and a run started from the **source chooser** has none: the chooser destroys +its own window the moment a row is clicked, and what follows --- the first allocation, the wait for +the stream's CONFIG or the read of a recording, and then the whole UI being built --- runs with no +main loop at all. The screen was simply empty for it, which at a multi-gigabyte buffer is the ten +seconds that read as "the dialog disappeared and nothing happened". A small window now stands in that +gap, raised before the buffer is planned and taken down only once the real window has been presented. +It names the wait in words --- `Connecting to 127.0.0.1:8000` over `planning the 4000 MB sample +buffer`, or `Opening rt.npy` over `reading the recording` --- because nothing can animate while the +work runs: its spinner is frozen for the duration, a progress bar would have nothing honest to show, +and it carries no close button for an application that could not yet service one. A source named with +`--server` or a file named with `--open` never sees it: those runs never had a window to lose, and the +notice after the present is the right place for them. + Every CONFIG is dumped to stderr once -- publish mode and, per signal, its name, element count, rate, time mode and time-signal index -- because a signal that refuses to plot is almost always a signal the stream described in an unexpected diff --git a/docs/claude/invariants-gtk-and-chrome.md b/docs/claude/invariants-gtk-and-chrome.md index 5a083b7..9f86eee 100644 --- a/docs/claude/invariants-gtk-and-chrome.md +++ b/docs/claude/invariants-gtk-and-chrome.md @@ -390,3 +390,25 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear worth knowing: the line composes on its own cadence, so a notice can appear a beat after it was set (do not read the label in the same tick a handler ran), and a refusal should say what to *do* about it -- "drop it on another plot, or add a row with the layout editor (L)" rather than the bare statement of the limit. + +- **The status line cannot cover the gap before the window exists, so a window does.** Between the chooser + closing and `gtk_window_present()` there is no main loop at all: the chooser destroyed its own window, + then `main()` allocates the sample buffer, waits for the first CONFIG or reads a recording, and + `on_activate()` builds every panel -- at a few gigabytes of buffer that is the ten seconds a user reported + as "the dialog disappeared and nothing happened". `loading_show()`/`loading_hide()` (in + `source_dialog.cpp`, beside the chooser they continue) keep an `AdwWindow` on screen across it, raised in + `main()` **before** the model is constructed and taken down in `on_activate()` **after** the present -- + either side is a frame of empty screen. Three rules it exists under: + - **It must be painted before the caller blocks.** `gtk_window_present()` only queues the frame, and + nothing after it runs a loop, so `loading_show()` ends in a nested `GMainLoop` quit by a timeout + (the chooser's own idiom). Iterating pending events is *not* enough: a window needs a frame-clock + cycle, and the frame clock only ticks while a loop runs. + - **It says what the wait is, in words.** The spinner is frozen for the whole of it -- no loop -- and a + progress bar would be a lie for the same reason, so the text carries the information: the address + being connected to or the file being opened, and under it the buffer size, which is the number that + explains the duration. It is `deletable=0` with no title buttons: a close button that cannot be + serviced until the work is over is worse than none. + - **Only the path that *asked* gets one.** `main()` raises it only when the chooser actually ran, which + is why every documented probe (all of them pass `--server`) is on exactly the path it was before -- + and a `--server` run never had a window to lose, so the post-present notice is the right place for it. + `loading_hide()` is a no-op when none is up, so `run_ui()` needs no notion of which path it is on. diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 691e847..0df3c22 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -910,6 +910,30 @@ sets them to **7.5 and 250** before saving, so a key that never reached the file | `known:` after a rename | one row, under the new name | a rename that leaves the old file, so the chooser lists the same session twice | | `known:` for a settings-only configuration | a row, though `signals=0` | the listing filter treating "no signals yet" as "nobody configured this", which hides every host the form set up | +## `[sources] loading` + +``` +[sources] picked 127.0.0.1:21077 multicast= interface= buffer=16 relative=0 capture=0 config=default note='' +[sources] loading: title='Connecting to 127.0.0.1:21077' detail='planning the 16 MB sample buffer' +[udpscope] CONFIG: 5 signals, publish=accumulate +[sources] loading: closed +``` + +The loading window covering the gap from the chooser to the first frame of the real window. It is +printed by any run that reaches the chooser, so `UDPSCOPE_PROBE_PICK_MS` reads the host leg and +`UDPSCOPE_PROBE_WAVE=` the file one (`title='Opening rt.npy' detail='reading the recording'`). + +| field | value it must read | the defect a wrong reading names | +| --- | --- | --- | +| `loading:` after `picked` | the address or basename just picked, and the buffer size | the window raised from the wrong place: before the chooser's answer it cannot name the wait, after the model it covers nothing | +| its position in the log | **before** the `CONFIG:` or `waveform loaded:` lines | a window shown after the work it is meant to cover — the ten seconds of empty screen, unchanged | +| `loading: closed` | **after** `CONFIG:`, i.e. once the real window is presented | a hide that runs earlier, which puts the screen back to empty for the frames the main window takes to map | +| no `loading:` line at all | correct for a `--server` or `--open` run | only the path that *asked* gets one; a line here means every probe just changed path | + +`loading ui: loading.ui is missing an id this code reads` is the file-and-code mismatch the chooser's +own check names, and like it the run continues: a loading window that cannot be built must not stop +the app from opening. + ## `[sources] prefs` ``` diff --git a/docs/claude/verification.md b/docs/claude/verification.md index a7c6e90..c8fed7f 100644 --- a/docs/claude/verification.md +++ b/docs/claude/verification.md @@ -307,6 +307,15 @@ as distinct from the fault injection below, which exercises ingest: configuration carried, into the `Config` the model is built from. A legacy file (`_.toml`) produces `layout loaded from …/_/default.toml`, an `active` holding `default`, and the old file renamed `.imported`. +- **The loading window covers the gap, and only on the path that asked.** The same + `UDPSCOPE_PROBE_PICK_MS` run must print + `[sources] loading: title='Connecting to 127.0.0.1:' detail='planning the N MB sample buffer'` + **between** the `picked` line and the stream's `CONFIG:`, and `[sources] loading: closed` **after** + it — the window is raised before the buffer is planned and taken down only once the real window has + been presented, which is the whole of what it exists for. `UDPSCOPE_PROBE_WAVE=` is the file + leg of it (`title='Opening ' detail='reading the recording'`, before `waveform loaded:`). + A run that names its source with `--server` prints **no** `loading:` line at all: every other probe + here is on that path and must stay on it. - **The event cache round-trips, and only where it is available.** `UDPSCOPE_PROBE_EVENTS_MS` reads the pane's availability first, in the state the app starts in -- `[events] live: button_visible=0 pane=0 recording=1`, **no pane and no button without trigger mode** -- then turns trigger mode on diff --git a/src/main.cpp b/src/main.cpp index 93079b7..13e5f27 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -30,13 +30,22 @@ int main(int argc, char** argv) { // declining must cost no buffer and no socket, and nothing below has to cope with "no source". // `--open` is the other answer to that question, and the chooser knows it: its Waveforms page // fills `open_waveform` in instead of the connection fields. - if (!cfg.server_specified && cfg.open_waveform.empty() && !udpscope::choose_source(cfg)) return 0; + const bool asked = !cfg.server_specified && cfg.open_waveform.empty(); + if (asked && !udpscope::choose_source(cfg)) return 0; // A source named on the command line skips the chooser, but its *stored* configuration still // applies: the buffer size, the clock and the capture ring are decided here, before the model -- // which is the only moment they can be -- and a saved host whose buffer was ignored because the // address came from the command line is an edit that looks like it never took effect. if (cfg.server_specified) udpscope::apply_stored_settings(cfg); + // Everything from here to `gtk_window_present()` inside `run_ui()` runs with no main loop: the + // buffer is allocated, a recording is read, then the whole UI is built. The chooser's window is + // already gone, so without this the screen is empty for all of it -- "the dialog disappeared and + // nothing happened", which at a few gigabytes of buffer is ten seconds long. Only on the path + // that *asked*: a source named with `--server` had no window to lose, and the status line after + // the present is the right place to say what such a run is waiting for. + if (asked) udpscope::loading_show(cfg); + udpscope::ScopeModel model(cfg); udpscope::RowSet rows; rows.set_row_count(cfg.plots); diff --git a/src/source_dialog.cpp b/src/source_dialog.cpp index 501ca43..6fba920 100644 --- a/src/source_dialog.cpp +++ b/src/source_dialog.cpp @@ -793,6 +793,86 @@ void apply_stored_settings(Config& cfg) { } } +namespace { + +/// The loading window and its builder. File scope because its life spans two calls made from two +/// translation units -- `main()` raises it, `run_ui()` takes it down once the real window is up -- +/// and there is exactly one of it: the gap it covers happens once per run. +GtkWidget* g_loading_window = nullptr; +GtkBuilder* g_loading_ui = nullptr; + +/// Runs the default main context for `ms`, the chooser's nested-loop idiom again. Pumping pending +/// events with `g_main_context_iteration()` is not enough: a window needs a frame-clock cycle to be +/// mapped and drawn, and the frame clock only ticks while a loop runs. +void pump_main_context(unsigned ms) { + GMainLoop* loop = g_main_loop_new(nullptr, FALSE); + g_timeout_add( + ms, + +[](gpointer user) { + g_main_loop_quit(static_cast(user)); + return G_SOURCE_REMOVE; + }, + loop); + g_main_loop_run(loop); + g_main_loop_unref(loop); +} + +} // namespace + +void loading_show(const Config& cfg) { + if (g_loading_window != nullptr) return; + g_loading_ui = gtk_builder_new_from_resource("/org/udpscope/ui/loading.ui"); + g_loading_window = GTK_WIDGET(gtk_builder_get_object(g_loading_ui, "loading_window")); + GtkWidget* title = GTK_WIDGET(gtk_builder_get_object(g_loading_ui, "loading_title")); + GtkWidget* detail = GTK_WIDGET(gtk_builder_get_object(g_loading_ui, "loading_detail")); + if (g_loading_window == nullptr || title == nullptr || detail == nullptr) { + std::fprintf(stderr, "[sources] loading ui: loading.ui is missing an id this code reads\n"); + std::fflush(stderr); + g_loading_window = nullptr; + g_clear_object(&g_loading_ui); + return; + } + + // The wording names the *wait*, not the destination: "connecting" with nothing else said is what + // this window was accused of being, and the buffer size is the number that explains the ten + // seconds. + std::string head; + std::string sub; + if (cfg.open_waveform.empty()) { + head = "Connecting to " + cfg.server + ":" + std::to_string(cfg.port); + if (!cfg.multicast.empty()) head += " (" + cfg.multicast + ")"; + char buf[128]; + std::snprintf(buf, sizeof(buf), "planning the %.0f MB sample buffer", cfg.buffer_mb); + sub = buf; + } else { + const std::size_t slash = cfg.open_waveform.rfind('/'); + head = "Opening " + (slash == std::string::npos ? cfg.open_waveform + : cfg.open_waveform.substr(slash + 1)); + sub = "reading the recording"; + } + gtk_label_set_text(GTK_LABEL(title), head.c_str()); + gtk_label_set_text(GTK_LABEL(detail), sub.c_str()); + gtk_window_present(GTK_WINDOW(g_loading_window)); + // Painted *here*, before the caller blocks. Everything this window covers -- the buffer + // allocation, the first CONFIG, a recording read -- runs on a thread that never returns to a main + // loop, so a window presented and not yet drawn would be an empty rectangle for the whole wait, + // which is the defect this is meant to fix rather than restate. + pump_main_context(180); + std::fprintf(stderr, "[sources] loading: title='%s' detail='%s'\n", head.c_str(), sub.c_str()); + std::fflush(stderr); +} + +void loading_hide() { + if (g_loading_window == nullptr) return; + GtkWidget* w = g_loading_window; + g_loading_window = nullptr; // before the destroy, as the chooser's own close does + gtk_window_destroy(GTK_WINDOW(w)); + // The window belonged to the builder's object list, so the builder goes after the destroy. + g_clear_object(&g_loading_ui); + std::fprintf(stderr, "[sources] loading: closed\n"); + std::fflush(stderr); +} + bool choose_source(Config& cfg) { const std::vector known = known_sources(); diff --git a/src/source_dialog.hpp b/src/source_dialog.hpp index 5ec9d50..901f50b 100644 --- a/src/source_dialog.hpp +++ b/src/source_dialog.hpp @@ -26,4 +26,17 @@ bool choose_source(Config& cfg); /// names signals, so it is loaded by the app once a CONFIG has described them. void apply_stored_settings(Config& cfg); +/// Puts a small window on screen saying what `cfg` is about to be opened as, and paints it before +/// returning. Call it after the chooser has answered and *before* the buffer is planned or a +/// recording is read: both of those run with no main loop, so nothing drawn later than this would +/// appear until they are over. +/// +/// Returns having pumped the main context long enough for the window to be mapped and drawn. The +/// caller owes a `loading_hide()`, which `run_ui()` does once the main window is presented. +void loading_show(const Config& cfg); + +/// Takes the loading window down. A no-op when none is up, so the paths that never show one -- a +/// source named with `--server` -- need no notion of it. +void loading_hide(); + } // namespace udpscope diff --git a/src/ui.cpp b/src/ui.cpp index 20613cc..a7b39b0 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -33,6 +33,7 @@ #include "model.hpp" #include "plot_host.hpp" #include "search.hpp" +#include "source_dialog.hpp" #include #include @@ -4838,6 +4839,12 @@ void on_activate(GtkApplication* application, gpointer user) { gtk_window_present(GTK_WINDOW(app->window)); + // The loading window covered the gap from the chooser to here -- the buffer, the file read and + // the whole of the construction above, none of which returns to a main loop. It goes down only + // now, *after* the present: destroying it before would leave the screen empty again for the + // frame or two the real window takes to map. A no-op when there was no chooser. + loading_hide(); + // What the window is waiting for, said *before* the first tick. The stream's first CONFIG plans // the buffer on the producer thread, which holds the model's mutex for the whole of it -- at a few // gigabytes that is around a second per gigabyte, and the frame tick cannot read the model while diff --git a/src/ui/loading.ui b/src/ui/loading.ui new file mode 100644 index 0000000..cb2b99c --- /dev/null +++ b/src/ui/loading.ui @@ -0,0 +1,69 @@ + + + + + + + udpscope + 420 + 220 + 0 + 0 + + + + + 0 + 0 + + + udpscope + + + + + + + vertical + 14 + center + center + 24 + 24 + 12 + 24 + + + 1 + 36 + 36 + + + + + 1 + center + title-4 + + + + + 1 + center + dim-label + + + + + + + + diff --git a/src/ui/udpscope-ui.gresource.xml b/src/ui/udpscope-ui.gresource.xml index 6dc9310..2c70961 100644 --- a/src/ui/udpscope-ui.gresource.xml +++ b/src/ui/udpscope-ui.gresource.xml @@ -14,6 +14,7 @@ help.ui source-form.ui source-chooser.ui + loading.ui main-window.ui cursors-panel.ui measures-panel.ui