diff --git a/README.md b/README.md index 3e64450..09cbea9 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,21 @@ window the toolbar offers, default 600), `--holdoff` (default trigger hold-off), added and removed from the **Layout editor** afterwards), `--relative-time` (see below), `--capture-packets N` (see "Packet capture" below). +**What a large `--buffer-mb` costs at startup, and what the window says while it happens.** The +buffer's rings are value-initialised, so allocating one touches every byte of `--buffer-mb` --- about +a second per gigabyte, mostly first-touch page faults --- and it happens on the producer thread +holding the lock the frame tick also takes. It is therefore a real freeze and not a slow paint. Two +things follow. First, a rebuild whose plan describes the storage already allocated **clears** the +store instead of allocating a new one, which is what most structural changes are: startup used to ask +for fifteen allocations and now asks for two or three (3.36 s → 1.18 s at `--buffer-mb 1000`), and +every enable of a signal is applied in one batch rather than re-planning per signal. Second, the +window is **presented before** the buffer is planned and carries a notice naming what it is waiting +for --- `connecting to 127.0.0.1:8000 — planning the 4000 MB buffer` --- because the remaining second +or so at a multi-gigabyte buffer is unavoidable, and a blank window that says nothing is the part of +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. + 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 @@ -109,6 +124,14 @@ export's second column reads — and it fixed a latent bug on the way, the expor having added `trigger_time()` to a *paused* view's stamps: an unrelated edge, or none at all before the first latch. +**An axis with nothing bound to it is still `[-window, 0]`.** "No samples yet" is the state every +structural change leaves behind — opening an event or a waveform re-plans the rings, which drops +their history, and the axis is then framed before the first sample of the new view lands. Framing it +at `[0, window]` there (which is what it did) offsets the view by a whole window under the x link +group's follow rule, so the two or three frames after every open drew the time base one screen to the +left of where the following frames drew it: a trace that jumps sideways and comes back, on every open. +The empty axis is now drawn in the same place as the full one. + Two things move in **opposite** directions under a rolling axis, and the asymmetry is deliberate. A **cursor** stays on screen: placed at -1.5 s it keeps meaning "1.5 s ago" and the data flows under it, which is a bench scope's roll behaviour and is free here, the cursors already being @@ -523,9 +546,15 @@ empty column beside the plot is what *off* must not look like. Recording is swit panel**, with the trigger's other options rather than in the pane whose contents it decides, and it is **off by default** -- writing every latch to disk is the user's decision about their disk. -Each row carries **its own picture** beside the numbers -- time, duration, signal count, rate -- in a -fixed 126x42 cell (the preview's own 3:1), so the picture is the same size in every row whatever the -pane's width. Clicking a row **replays** it: the same loader `--open` uses, on the event's file. **The layout does +Each row carries **its own picture** beside the numbers -- time, duration, signal count, rate -- 138 +pixels wide whatever the pane's width, and as tall as that event's own proportions make it: the +preview is one 44-pixel strip per signal, so a one-signal event is a short row and a six-signal one a +tall one (capped, so that a row stays a row). **Hovering it shows the whole preview at full size**, +through a tooltip -- the small picture says *which* event this is, the zoom says what is in it. +Clicking a row **replays** it: the same loader `--open` uses, on the event's file. The click **leaves +the list alone** -- your scroll position and the pair you had selected for a comparison both survive +it, because marking which event is on screen rewrites the rows' titles in place instead of rebuilding +the list. **The layout does not change** -- the rows, which signal sits in which slot, the colours, the row count, the time window, the cursors and the measurements all stay exactly where they are, and now read the stored event instead of the stream; that is what "show me that event" asks for, and rebinding the rows to the file would @@ -562,6 +591,8 @@ Three things worth knowing: trigger edge as a brighter line -- not captured from the window: the plot is a `GtkGLArea`, and a GL area's ink is not in the widget's paint tree, so a snapshot of it is empty. A data-drawn preview is also the honest one: it shows the samples, not the zoom the window happened to be at. + It is a plain 24-bit BMP, written bottom-up with **blue first** — both are BMP's own order, and + writing RGB there is what made every external viewer show the traces in the wrong colour. - **The picture is decoded synchronously** (`gdk_texture_new_from_filename()` into `gtk_picture_set_paintable()`), not through `gtk_picture_set_filename()`: that path uses the image loader's sandboxed, asynchronous decoder, which here fails silently -- the picture keeps a @@ -569,9 +600,19 @@ Three things worth knowing: It is then **scaled once to the row's size** and cached by file: a `GtkPicture`'s natural size is its content, so the stored 480-pixel preview given straight to a row claims the row's whole width and squeezes the text out of it -- a size request on the picture is a floor, not a cap. +- **A picture that is missing or will not decode is drawn again from the samples.** The `.npy` is the + event; the `.bmp` is only a picture of it, so a write that failed, a truncated file or a cache from + an older build is repaired rather than shown as a blank row. Once per row: a second failure is a + real one. - **A replay stops the acquisition** (a live producer would overwrite the loaded samples within a frame), so the status bar names the file until *Live* starts a fresh client and thread. The replayed samples age out of the ring as the stream refills it. +- **Opening an event, a comparison or going back to *Live* is one re-plan of the buffer**, not one + per channel. Each of those switches rebuilds the signal set, and a rebuild that moves the plan + allocates --- and so touches --- the whole of `--buffer-mb`; enabling a comparison's eight channels + one at a time was eight of those, which is most of what made the switch feel slow. They are now + enabled in a single call, and a rebuild whose plan is unchanged clears the buffer instead of + allocating a new one. - The switch in the pane's header stops the recording; the cache is only useful if it was already on when the event you wanted happened, which is why it defaults to on. diff --git a/docs/claude/invariants-data-path.md b/docs/claude/invariants-data-path.md index 7ce8daa..430e936 100644 --- a/docs/claude/invariants-data-path.md +++ b/docs/claude/invariants-data-path.md @@ -12,6 +12,33 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear `plan_partition()` rather than recomputing the factor, because two expressions for that decision is how the buffer and the view end up disagreeing about the stored rate. It takes `mutex()` itself — do not call it holding the lock. +- **A rebuild that would plan the *same* storage clears the store instead of allocating one.** + `rebuild_locked()` compares the new `Partition` against the current one with + `Partition::same_storage_as()` — every field that decides an allocation: `time_capacity`, + `time_stride`, `abs_time`, `base_dt` and the per-channel `stride`/`value_capacity` vectors — and + on a match calls `BufferStore::reset()` rather than constructing a new one. `base_dt` is in the + comparison on purpose: it is not a size, but it is what the slots *mean*, and keeping samples + under a changed `base_dt` would re-date the whole ring. + **Construction is the cost.** A `RingBuffer` holds a `std::vector>`, which is + value-initialised, so making a store touches every byte of `--buffer-mb` — about a second per + gigabyte, mostly first-touch page faults — while `reset()` is one relaxed store per ring plus an + `invalidate()` and an epoch back to zero. It must leave nothing of the old history behind, which + is why it clears the time ring, the absolute-time ring, every value ring, and both the published + window and the pyramid of every channel: a reused store that kept a stale published window would + hand the renderer samples the new plan does not describe. + `reset()` runs on the consumer thread under `ScopeModel::mutex()`, which is what makes a + `RingBuffer::clear()` safe here at all — the producer holds that same mutex for the whole of + `on_data()`. + Every allocation that *is* made says so: `[udpscope] buffer allocated: N channels, S s span, + T ms` on stderr. A start used to emit fifteen of those; two is a real start, and a count that + climbs again is this rule having stopped applying. +- **Enables are applied in one batch, because each one re-plans.** `set_signals_enabled()` takes + the mutex once, moves every signal in the list, and re-plans and bumps the generation **once** — + and not at all when nothing actually moved, which is the common case for a restored session + re-enabling what is already on. Calling `set_signal_enabled()` in a loop is a buffer rebuild per + signal; `apply_layout()` and the waveform loader collect their wanted signals into one vector for + that reason, and `apply_layout()` records the session's span **first**, with no channels bound, so + the `apply_time_window()` that follows re-plans nothing. - **The shared clock is decimated to fit the requested window; channels never are.** A window too long to store at the wire rate used to be refused (the control was capped to `Partition::span_s`), which made a 600 s view of a 1 MS/s stream unreachable. diff --git a/docs/claude/invariants-gtk-and-chrome.md b/docs/claude/invariants-gtk-and-chrome.md index 1cbeaaa..5a083b7 100644 --- a/docs/claude/invariants-gtk-and-chrome.md +++ b/docs/claude/invariants-gtk-and-chrome.md @@ -207,6 +207,23 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear thread and `gtk_picture_set_paintable()` hands over the result -- and a NULL texture at least says so. A picture that must be on screen is also a picture with a **height request**: `can_shrink` alone asks for no height at all in a box. +- **A cache that is the *only* source of a widget's content must be created by the thing that reads + it.** The event pane's thumbnails come from an `EventThumbCache` hung off the list box with + `g_object_set_data_full`, and `event_preview_texture()` answers out of that cache or not at all. + Nothing ever created it — so for as long as the pane existed, **every** row came up without a + picture, which is what "event screenshot are unusable and sometime missing" was. A missing cache + there is not a slow path, it is a blank one; `event_thumb_cache()` therefore creates one on first + use. Owned by the **list**, because nothing else has the pane's lifetime: textures of a list that + no longer exists are so much held memory, and the destroy notify is where they are unref'd. +- **Hover magnification is a tooltip, not a popover of ours.** A row's thumbnail is 138 px wide and + the stored preview is 480: the small one says *which* event this is, and the zoom says what is in + it. GTK's own tooltip (`gtk_widget_set_has_tooltip` + `query-tooltip` + `gtk_tooltip_set_custom`) + is shown, placed, dismissed **and destroyed** by GTK, and takes no pointer grab. A popover per row + would be a parented widget to unparent on every rebuild of the pane — one defect below — and a + grab to lose, for a thing with no interaction in it. Two details: `can_shrink` must be FALSE on the + tooltip's picture or the tooltip is given the smallest size its content allows (the opposite of the + point), and the path handed to the handler has to be **owned by the closure** + (`g_signal_connect_data` with a destroy func), since the record it came from is a loop variable. - **Unparenting a popover inside its own `closed` emission is a use-after-free.** Dismissals go through `gtk_popover_popdown()`, which emits `closed`; a handler that unparents the popover there drops the reference the parent held, the widget dies, and GTK's own popdown path then runs on freed diff --git a/docs/claude/invariants-time-axis-and-window.md b/docs/claude/invariants-time-axis-and-window.md index fc1d03f..ec681cf 100644 --- a/docs/claude/invariants-time-axis-and-window.md +++ b/docs/claude/invariants-time-axis-and-window.md @@ -31,6 +31,21 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear reasons (an event pin) call `sync_window_display()` alone, which only restates the control and never moves the plot — calling `apply_time_window()` there would clamp a legitimately short capture up to the requested window. +- **With nothing bound, `set_time_window()` frames `{-window, 0}` and never `{0, window}`.** Under + a following x group a range states two things — its span, and *through its maximum* where the + window sits behind the live edge — so a maximum **at** the edge is what makes the offset zero. + When there is a live edge to read that is `{newest - window, newest}`; when there is not, the + edge the live view would put at x = 0 is 0, so the equivalent statement is `{-window, 0}`. + Asking for `{0, window}` there is an offset of a *whole window*. This is not a rare fallback: + "no samples bound" is the state every structural change leaves behind, and + `apply_time_window()` re-plans the rings and *then* calls this, so it is the axis a mode change + is drawn with until the first sample lands. With `{0, window}` the two or three frames after + every event or waveform open drew the time base one screen to the left of where the following + frames drew it — a trace that jumps a screen sideways and comes back, which is what "opening + events or even waveform sometime result in data flickering (like time base) and is unusable" + was. The reading is the dump's traceless paint counts: `mode=live` 38 → 0 and `mode=waiting` + 2 → 40, the latter being the point — a waiting view now paints its empty axis in the *same* + place every frame instead of twitching into a framed one. - **The x labels go relative on the tick *step against the label resolution at the axis' magnitude*, never on the span.** `RowSet::update_time_labels()`'s question is only ever "can this step be written at this magnitude": `TickFormat::Time` carries three significant figures diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index d254593..f3f279e 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -987,9 +987,10 @@ configuration that carries settings, is the defect this exists for — a saved h [events] live, recording off: button_visible=0 pane=0 pane_w=0 tick=1 recording=0 [events] recording on, no trigger: button_visible=1 pane=0 pane_w=0 tick=1 recording=1 [events] after trigger on: button_visible=1 pane=0 pane_w=0 tick=1 recording=1 -[events] dir=/tmp/k5/cache/udpscope/events/127.0.0.1-21023 n=1 newest: dur=2.000000 pre=1.000000 post=1.000000 edge=1.000100 signals=4 samples=20001 rate=10000 seq=1 thumb_bytes=230454 recorded=1 pane=1 -[events] activated row0: replaying=1 pane=1 pane_w=240 tick=0 window=2.000 thumbnail=1 thumb_size=126x42 thumb_visible=1 -[events] replayed: channels=4 samples=20001 replaying=1 waveform=1 +[events] preview redrawn: signals=3 stored=480x132 row_drawn=1 +[events] dir=/home/u/.cache/udpscope/events/127.0.0.1-21023 n=3 newest: stem=20261013-184140-0003 dur=2.000000 pre=1.600000 post=0.400000 edge=5.660100 signals=3 samples=20001 rate=10000 seq=3 thumb_bytes=190134 recorded=1 pane=1 +[events] activated row0: replaying=1 pane=1 pane_w=240 pane_box=228 tick=0 window=2.000 axis=[-1.600,0.400] marker=0.000 thumbnail=1 thumb_size=138x46 thumb_visible=1 rows_kept=1 selected=2/2 +[events] replayed: channels=3 samples=20001 replaying=1 waveform=1 [events] live again: replaying=0 waveform=0 [events] recording off, trigger off: button_visible=1 pane=1 pane_w=240 tick=1 recording=0 [events] recording off, trigger on: button_visible=1 pane=1 pane_w=240 tick=1 recording=0 @@ -1001,21 +1002,35 @@ configuration that carries settings, is the defect this exists for — a saved h [events] group left: replaying=0 kind=0 waveform=0 ``` -`thumb_size` is the built size of the row's picture (96x32 logical, plus the CSS border), so it moves with -the font and the display's scale factor; what it must never be is `0x0`. +`thumb_size` is the built size of the row's picture: `kEventThumbW` wide, and as tall as the *stored* +preview's own aspect makes it at that width, clamped to `[kEventThumbH, kEventThumbMaxH]`. So it moves +with the event's **signal count** (a three-signal event sits on the floor at `138x46`, a six-signal one +reads `138x75`) as well as with the CSS border and the display's scale factor. What it must never be is +`0x0`, and a height pinned to the floor for *every* event says the aspect fit was dropped and tall +previews are being squashed again. + +`[events] preview redrawn:` is the repair path, and the probe forces it: step 1 deletes the newest +`.bmp`, drops the thumbnail cache and refreshes the pane, so this line is `EventStore::redraw_preview()` +drawing the picture again from the `.npy`. `stored` is the file it wrote, read back out of the BMP +header — `480` wide and `44 * signals` tall — and `row_drawn=1` says the row got a paintable from it. +The probe therefore needs a cache a **previous** run populated: it fires ~4.5 s in, before this run's +first latch. | field | value it must read | the defect a wrong reading names | | --- | --- | --- | | `n`, `stem`, `seq` | one entry per latch, named by its time and the run's counter | two events in one second overwriting each other, or a cache that is never written | | `dur` | `pre + post` for the event's window | the snapshot's own window not recorded, so the list disagrees with the picture | | `signals`, `samples` | the event's channels and rows | a table built from the *rows* instead of the snapshot: only what happened to be plotted | -| `thumb_bytes` | a real 24-bit BMP (`54 + 480*160*3`) | a preview that is empty or missing -- the widget snapshot of a `GtkGLArea`, which is NULL | +| `thumb_bytes` | a real 24-bit BMP, `54 + 480 * (44 * signals) * 3` rounded up per row to a multiple of four -- so it *grows with the signal count* (190134 at three signals, 380214 at six) | a preview that is empty or missing -- the widget snapshot of a `GtkGLArea`, which is NULL -- or a picture back on a fixed total height, which gave a six-signal event 26 px a signal and an eight-signal one 20: a smear, not a waveform | | `recorded` | 1 while the trigger panel's switch is on | the switch not wired, or recording defaulting off (it must be on: the cache is for the event you did not expect) | | `pane` | 1 once the history button has been clicked | the button's handler not revealing the pane | | `live` | `button_visible=0 pane=0** | a pane or a button on screen with no trigger mode: a column of nothing beside a live plot | | `after trigger on` | `button_visible=1 pane=0` | the pane revealed by trigger mode alone (it must be on demand from the toolbar button), or the button missing in trigger mode | | `recording off, trigger off` and `recording off, trigger on` | `button_visible=1 pane=1 recording=0` in both **once the cache holds anything** (the probe's own run does by this point) | the button going away with the switch when events are already written -- an event on disk outlives the switch that wrote it -- or `pane_w` not being 240 with the pane open. With an *empty* cache the same two lines read `button_visible=0 pane=0`, which is the empty column the item exists to prevent | -| `thumbnail`, `thumb_size`, `thumb_visible` | 1, a non-zero size, and 1 | the picture loaded through `gtk_picture_set_filename()`, which fails silently (the paint-able is there, the allocation is there, nothing is drawn), or a `GtkPicture` given a size request and no cap -- its natural size is its content, so the row's text is squeezed off. `thumb_size` is the reading that catches both | +| `thumbnail`, `thumb_size`, `thumb_visible` | 1, a non-zero size, and 1 | the picture loaded through `gtk_picture_set_filename()`, which fails silently (the paint-able is there, the allocation is there, nothing is drawn), or a `GtkPicture` given a size request and no cap -- its natural size is its content, so the row's text is squeezed off. `thumb_size` is the reading that catches both. It also catches the cache never being *created*: `event_preview_texture()` answers from the cache or not at all, so a reader that does not create one leaves **every** row blank, which is what `thumbnail=0` meant for as long as it read 0 | +| `preview redrawn` | `stored=480x<44*signals>` and `row_drawn=1` | a preview that cannot be repaired. The `.npy` is the event and the `.bmp` is only a picture of it, so a write that failed, a truncated file or a cache from an older build must be redrawn rather than shown as a blank row -- `row_drawn=0` is a row the user reported as "sometimes missing" | +| `axis`, `marker`, `pane_box` (in `activated row0`) | `[-pre, +post]` of the event just opened, `marker=0.000`, and a `pane_box` close to `pane_w` | an opened event framed on something other than its own pin; a trigger line left **parked** (a large negative) rather than on the edge, which is a file opened without its marker; or a pane whose *rendered* width is 0 while the divider says 240 -- the position is what was asked for, `pane_box` is what the pane actually got, and a divider at 240 with a zero-width pane is still no pane | +| `rows_kept`, `selected` (in `activated row0`) | `rows_kept=1` and `selected=N/N` -- the counts either side of the activation equal | the list **rebuilt** by the activation. It is the row *object* that is compared, not its index: a rebuilt list has a row 0 too, and only identity tells a list that was left alone from one that was torn down under the user's scroll position and their comparison selection. `mark_events_on_screen()` must rewrite the titles in place | | `replaying` (last line) | 0 | a replay with no way back: *Live* must start a fresh client and thread | | `pane_w` | the divider's position: 0 while the pane is closed, its remembered width (240 by default) while it is open | the pane reserving a column while closed (a paned hands its start child its minimum unless the position says otherwise), or a width that is not remembered | | `tick` | 1 in live mode, **0** once a file is on screen | the signal list still offering "acquire this signal" for samples that are already stored | @@ -1252,10 +1267,20 @@ refusal it may still hold the previous text. ## `[enable-all-probe]` and switching a signal off ``` -[enable-all-probe] 4 channel(s) enabled and assigned to plots +[enable-all-probe] 4 channel(s) enabled and assigned to plots, notice='connecting to 127.0.0.1:21023 -- planning the 200 MB buffer' [enable-all-probe] switched 'sine' off: traces_left=0 (want 0) notice=''sine' is off: 1 trace left the plots' ``` +The first line's `notice` is the **startup** notice, and it is read here because this probe is the +earliest thing that reads the status bar. It is set as the window is presented -- before the first +CONFIG can take the model's mutex, so before anything can block -- and it names the host and the +buffer being planned, because that is the window's only answer to "why is nothing happening yet". +An empty `notice` here is the window that used to come up silent while a large buffer was allocated: +at `--buffer-mb 4000` that is the dialog disappearing and then over a second of a frozen, blank +window. (The buffer is allocated on the producer thread under `ScopeModel::mutex()`, which the frame +tick also takes, so the freeze is real and cannot be papered over in the UI -- see the +`[udpscope] buffer allocated:` line below.) + `UDPSCOPE_ENABLE_ALL_MS` enables every signal and assigns the acquired channels across the rows, then switches one off **through its own acquire tick in the signal list** -- the tick is a box inside the row, which is the shape `signal_tick_visible()` walks -- and reads the plots back. `traces_left=0` is the rule: a disabled @@ -1270,6 +1295,30 @@ which must be untouched: [enable-all-probe] switched 'sine' off: traces_left=0 (want 0) math_off=[math x2,math3] math_on=[math lp] notice=''sine' is off: 1 trace left the plots; math off: math x2 and 1 more' ``` +## `[udpscope] buffer allocated:` + +``` +[udpscope] buffer allocated: 4 channels, 839 s span, 74 ms +``` + +Not a probe — it is printed by `ScopeModel::rebuild_locked()` in every build, because this is the one +operation in the application whose duration a user can see with no instrument: the store's rings are +`std::vector>` and so **value-initialised**, which touches the whole `--buffer-mb` +(roughly a second per GB, mostly first-touch page faults), and it runs on the producer thread holding +`ScopeModel::mutex()` — which the frame tick also takes. So the line's `ms` is a frozen window, not +bookkeeping. + +**It is the count that is the assertion, not the duration.** One line per *real* change of plan. A +rebuild whose plan describes the storage already allocated clears the store instead +(`Partition::same_storage_as()` → `BufferStore::reset()`) and prints nothing, so a startup or a mode +change that reads more than one or two of these is a re-plan that should have been coalesced. Startup +asked for **fifteen**; it now asks for two or three. A run at `--buffer-mb 4000` measured those +coalesced allocations at 1148 ms each and a max frame tick of 1186 ms — the same number, which is the +evidence that the freeze *is* this call and not something downstream of it. + +The companion reading is the startup `notice` on the `[enable-all-probe]` line above: the allocation +cannot be made free, so the window must at least say what it is doing while it happens. + ## `[math-probe] graph edit:` ``` diff --git a/docs/claude/verification.md b/docs/claude/verification.md index a0f6e3d..a7c6e90 100644 --- a/docs/claude/verification.md +++ b/docs/claude/verification.md @@ -26,6 +26,12 @@ all. A fifth warning, or any critical, is a finding. An *incremental* build reads **two** of those four: the generated GResource `.c` files are not recompiled when only `src/` changed, so their `-Woverlength-strings` do not reappear. +A third reading every run offers for free: **how many `[udpscope] buffer allocated:` lines it +printed.** The line is not behind a probe — it is in every build — and one line is one *real* change +of buffer plan, each of which blocks the main thread for the time it takes to touch the whole +`--buffer-mb`. A normal run prints two or three. Fifteen at startup is the defect it was added for; +see [probe-readings.md](probe-readings.md). + That second baseline used to be "exactly two, `gtk_widget_get_height` and `gtk_widget_get_width`, from the dump asking a destroyed widget for its size", and tolerating them was a mistake: the dump runs from the destroy handler, so the widget pointer is **freed memory**, and a critical is only diff --git a/src/event_store.cpp b/src/event_store.cpp index 956cfb2..bef25c6 100644 --- a/src/event_store.cpp +++ b/src/event_store.cpp @@ -13,6 +13,7 @@ #include #include "layout_store.hpp" +#include "waveform.hpp" namespace udpscope { namespace { @@ -52,25 +53,30 @@ std::string stem_of(const std::string& file, const char* ext) { /// The palette is fixed rather than the plots': this is navigation, and what has to be told apart at /// a glance is one *event* from another, not one signal from another (for which the names are on the /// row above). -bool write_preview(const ExportTable& table, const std::string& path, int width = 480, int height = 160) { +/// +/// The height is **per strip**, not for the whole picture: with a fixed total, a six-signal event got +/// 26 pixels a signal and an eight-signal one got 20, which is a smear and not a waveform -- the +/// picture has to grow with the signal count, and it is the *viewer* that scales it down to a row. +bool write_preview(const ExportTable& table, const std::string& path, int width = 480, + int strip_h = 44) { if (table.empty() || table.columns.empty()) return false; const std::size_t rows = table.rows(); const std::size_t strips = table.columns.size(); - const int strip_h = std::max(6, height / static_cast(strips)); const int total_h = strip_h * static_cast(strips); static const std::uint32_t kPalette[] = {0x4FA3FFu, 0xFF7A4Fu, 0x6EE07Au, 0xE06ED0u, 0xFFD24Fu, 0x6FD8E0u, 0xB9A0FFu, 0xE0E0E0u}; - // One byte per pixel per channel, bottom-up (BMP's own order), padded to a 4-byte row. + // One byte per pixel per channel, bottom-up and **blue first** -- both are BMP's own order, and + // writing RGB here is what made every external viewer show the traces in the wrong colour. const int row_bytes = ((width * 3) + 3) & ~3; std::vector pixels(std::size_t(row_bytes) * total_h, 0); const auto put = [&](int x, int y, std::uint32_t rgb, double alpha) { if (x < 0 || y < 0 || x >= width || y >= total_h) return; unsigned char* p = &pixels[std::size_t(total_h - 1 - y) * row_bytes + std::size_t(x) * 3]; const double a = std::clamp(alpha, 0.0, 1.0); - p[0] = static_cast(p[0] * (1.0 - a) + ((rgb >> 16) & 0xFF) * a); + p[0] = static_cast(p[0] * (1.0 - a) + (rgb & 0xFF) * a); p[1] = static_cast(p[1] * (1.0 - a) + ((rgb >> 8) & 0xFF) * a); - p[2] = static_cast(p[2] * (1.0 - a) + (rgb & 0xFF) * a); + p[2] = static_cast(p[2] * (1.0 - a) + ((rgb >> 16) & 0xFF) * a); }; // The background: a dark field, so the traces read the way they do on screen. for (int y = 0; y < total_h; ++y) { @@ -268,6 +274,34 @@ std::string EventStore::save(const ExportTable& table, const EventRecord& meta, return samples; } +std::string EventStore::redraw_preview(const EventRecord& record) const { + if (dir_.empty() || record.stem.empty() || record.file.empty()) return {}; + WaveformReader reader; + std::string error; + if (!reader.open(record.file, &error)) return {}; + // The whole event in one table, which an event can afford where an export cannot: it is bounded by + // the trigger's own pre/post window, so this is seconds of samples and not a session's worth. + ExportTable table; + table.columns.resize(reader.names().size()); + for (std::size_t c = 0; c < table.columns.size(); ++c) { + table.columns[c].name = reader.names()[c]; + table.columns[c].values.reserve(reader.rows()); + } + table.time.reserve(reader.rows()); + std::vector values(reader.names().size(), 0.0f); + double t = 0.0; + double abs_t = 0.0; + while (reader.next(&t, &abs_t, values.data())) { + table.time.push_back(t); + for (std::size_t c = 0; c < table.columns.size(); ++c) { + table.columns[c].values.push_back(values[c]); + } + } + const std::string path = dir_ + "/" + record.stem + ".bmp"; + if (!write_preview(table, path)) return {}; + return path; +} + std::vector EventStore::list() const { std::vector out; if (dir_.empty()) return out; diff --git a/src/event_store.hpp b/src/event_store.hpp index d4052aa..f70b9e5 100644 --- a/src/event_store.hpp +++ b/src/event_store.hpp @@ -73,6 +73,15 @@ class EventStore { /// however large the captures are. std::vector list() const; + /// Draws `record`'s preview again from its samples and returns the thumbnail's path, or an empty + /// string when the samples cannot be read. + /// + /// This is the repair for a preview that is missing or will not decode -- a write that failed, a + /// truncated file, a cache written by an older build. The `.npy` is the event, the `.bmp` is only a + /// picture of it, so a lost picture is recoverable and must not be shown as a blank row. It reads + /// the samples, so unlike `list()` it is **not** cheap: call it for the one event that needs it. + std::string redraw_preview(const EventRecord& record) const; + /// Deletes one event's samples, sidecar and thumbnail. bool forget(const std::string& stem); diff --git a/src/model.cpp b/src/model.cpp index 7104fcc..a0383de 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -163,6 +163,16 @@ BufferStore::BufferStore(const Partition& part, f64 span_s) } } +void BufferStore::reset() noexcept { + time_->clear(); + if (abs_time_) abs_time_->clear(); + for (auto& v : values_) v->clear(); + for (auto& c : channels_) { + c->invalidate(); + c->set_epoch(0.0); + } +} + void BufferStore::acquire_all() { for (auto& c : channels_) c->acquire(); } @@ -536,9 +546,32 @@ void ScopeModel::rebuild_locked() { return; } - part_ = plan_partition(dts, cfg_.buffer_mb * 1024.0 * 1024.0, - target_span_.load(std::memory_order_relaxed), cfg_.relative_time); - store_ = std::make_unique(part_, part_.span_s); + const Partition want = plan_partition(dts, cfg_.buffer_mb * 1024.0 * 1024.0, + target_span_.load(std::memory_order_relaxed), + cfg_.relative_time); + // A plan that describes the storage already allocated is cleared, not rebuilt. Allocating + // a store is what costs -- its rings are value-initialised, so it touches the whole + // `--buffer-mb` -- and most structural changes do not move the plan at all: a calibration + // keeps the channel set, and so does leaving an event for the live view. Startup asked for + // sixteen of these and eight of them planned the same buffer twice. Clearing has the same + // meaning (`BufferStore::reset()`): the history is gone either way, which is why + // `plan_generation_` bumps above regardless. + if (store_ && part_.same_storage_as(want)) { + part_ = want; + store_->reset(); + } else { + part_ = want; + const std::uint64_t t0 = now_ns(); + store_ = std::make_unique(part_, part_.span_s); + // Named, with what it cost: this is the one operation in the application whose duration a + // user can see with no instrument, because it blocks the main thread for the time it takes + // to touch the whole buffer -- about 300 ms per GB. A startup or a mode change that reads + // more than one of these lines is a re-plan that should have been coalesced, and that is + // the defect the line exists to name (it was fifteen of them at startup). + std::fprintf(stderr, "[udpscope] buffer allocated: %zu channels, %.3g s span, %.0f ms\n", + channels_.size(), part_.span_s, double(now_ns() - t0) / 1.0e6); + std::fflush(stderr); + } if (trigger_.channel >= static_cast(channels_.size())) trigger_.channel = -1; prepare_math_locked(); @@ -590,9 +623,8 @@ bool ScopeModel::signal_enabled(std::size_t signal) const { return signal < enabled_.size() && enabled_[signal]; } -void ScopeModel::set_signal_enabled(std::size_t signal, bool on) { - std::lock_guard lock(mutex_); - if (signal >= enabled_.size() || enabled_[signal] == on) return; +bool ScopeModel::enable_locked(std::size_t signal, bool on) { + if (signal >= enabled_.size() || enabled_[signal] == on) return false; enabled_[signal] = on; // A math channel's synthetic signal is enabled *here* -- it is the panel's switch -- while // `append_math_signals_locked()` re-states `enabled_` from `math_` on the next edit. The two have @@ -603,9 +635,14 @@ void ScopeModel::set_signal_enabled(std::size_t signal, bool on) { if (m.name == signals_[signal].name) m.enabled = on; } } + return true; +} + +void ScopeModel::rebuild_enabled_locked() { // The trigger names a channel by its display name, which is stable across // renumbering: when its own signal is disabled the channel vanishes (the trigger - // then waits, and comes back when the signal is re-enabled). + // then waits, and comes back when the signal is re-enabled). Read before + // `rebuild_channels()`, which is what renumbers. const int old_trigger = trigger_.channel; const std::string trigger_name = (old_trigger >= 0 && old_trigger < static_cast(channels_.size())) @@ -622,6 +659,24 @@ void ScopeModel::set_signal_enabled(std::size_t signal, bool on) { generation_.fetch_add(1, std::memory_order_release); } +void ScopeModel::set_signal_enabled(std::size_t signal, bool on) { + std::lock_guard lock(mutex_); + if (!enable_locked(signal, on)) return; + rebuild_enabled_locked(); +} + +void ScopeModel::set_signals_enabled(const std::vector& signals, bool on) { + std::lock_guard lock(mutex_); + bool any = false; + for (const std::size_t s : signals) { + if (enable_locked(s, on)) any = true; + } + // Nothing moved: no re-plan, and in particular no `generation_` bump -- a restored session + // re-enables what is already on, and a bump there would make the UI rebind for nothing. + if (!any) return; + rebuild_enabled_locked(); +} + std::pair ScopeModel::calibration(std::size_t signal) { std::lock_guard lock(mutex_); if (signal >= signals_.size()) return {1.0, 0.0}; diff --git a/src/model.hpp b/src/model.hpp index 48b4d19..cd995fe 100644 --- a/src/model.hpp +++ b/src/model.hpp @@ -109,6 +109,23 @@ struct Partition { /// Nyquist. The boxcar the ingest path averages with has its first null at the stored /// rate itself, so everything that would fold back is attenuated. double antialias_hz() const noexcept { return 0.5 * stored_rate(); } + + /// Two plans that describe the **same storage**: the same rings, of the same lengths, at + /// the same decimation, over the same clock. This is the test `rebuild_locked()` uses to + /// decide whether it can clear the buffer it has instead of allocating another one. + /// + /// `base_dt` is compared although it buys no byte, because a reused store keeps the + /// `Partition` *it* was built with and hands `slot_dt()` to every `ChannelBuffer` as the + /// nominal grid `dx()` reports -- and `dx()` is what picks the LOD level. Two channel sets + /// can want the same capacities off a different base tick, and reusing across that would + /// leave every channel describing a grid the samples are not on. It is an exact compare of + /// a value that is copied, never recomputed, from the fastest channel's `dt`. + /// `span_s` needs no compare of its own: it is the product of the three fields above. + bool same_storage_as(const Partition& o) const noexcept { + return time_capacity == o.time_capacity && time_stride == o.time_stride && + abs_time == o.abs_time && base_dt == o.base_dt && stride == o.stride && + value_capacity == o.value_capacity; + } }; /// Plans a partition for `channel_dt` cadences within `buffer_bytes`, covering at least @@ -156,6 +173,27 @@ class BufferStore { } const Partition& partition() const noexcept { return part_; } + /// Drops every sample and leaves a store indistinguishable from a freshly built one with + /// the same `Partition` -- the state `rebuild_locked()` wants when the plan did not change. + /// + /// Why it exists: constructing a store *is* its cost. A ring is a + /// `vector>`, which is value-initialised, so building one touches every + /// byte of the user's `--buffer-mb` -- about 300 ms per GB, nearly all of it first-touch + /// page faults. Clearing is a count store per ring. Most structural changes (a + /// calibration, a re-enable of what was just disabled, going back to live) re-plan to the + /// *same* partition, and paying a second full allocation for them is what made a large + /// buffer take ten seconds to start and an event take seconds to leave. + /// + /// It must leave **nothing** of the old history behind, in either copy: the rings are + /// emptied and every `ChannelBuffer` is `invalidate()`d, which drops the published window + /// and the pyramid built over it. The epoch goes back to 0 for the same reason -- a fresh + /// store has none, and a reused one keeping the old event's epoch would re-base the next + /// axis against a trigger that no longer exists. + /// + /// Caller's thread: the consumer, under `ScopeModel::mutex()` -- the same lock the + /// producer's pushes hold. `RingBuffer::clear()` is not safe against a concurrent push. + void reset() noexcept; + /// Consumer: publishes every channel's new samples. void acquire_all(); @@ -356,6 +394,17 @@ class ScopeModel { static bool is_time_signal(const std::vector& signals, std::size_t signal); void set_signal_enabled(std::size_t signal, bool on); + /// The same switch thrown for several signals as **one** structural change. + /// + /// Enabling a signal changes the channel count, so it re-plans the partition and the new + /// plan really is a different buffer -- there is no reusing the old store for it (see + /// `BufferStore::reset()`). A caller that enables n signals one at a time therefore pays n + /// full allocations to reach a state one allocation describes, and every one of them drops + /// the history the one before it started collecting. That is what a restored session, a + /// loaded waveform and an event group all do, and at a 1 GB buffer it is seconds of a + /// blank window: the plots are bound to a store that is replaced again before it holds a + /// screenful. Indices the model has no signal for are ignored. + void set_signals_enabled(const std::vector& signals, bool on); /// Stores the calibration for a signal's *name* and applies it to that signal's /// live channels. Keyed by name, not index: a CONFIG renumbers and an /// enable/disable rebuilds the channel list, and `rebuild_channels()` re-applies @@ -587,6 +636,15 @@ class ScopeModel { private: void rebuild_channels(); + /// One signal's enable bookkeeping -- `enabled_` and the math channel's own flag -- with no + /// re-plan. Returns whether the state actually moved. The two halves of + /// `set_signal_enabled()` are split so that a batch can do n of these and one of the + /// rebuild below. + bool enable_locked(std::size_t signal, bool on); + /// The re-plan an enable needs: carry the trigger's channel across the renumbering, + /// rebuild the channels and the buffers, and publish the new generation. Caller holds + /// `mutex()` and has already moved `enabled_`. + void rebuild_enabled_locked(); /// Re-appends one synthetic signal per math channel, after the stream's own. Called under the /// lock by both `set_signals()` (a CONFIG) and `set_math_channels()` (an edit). void append_math_signals_locked(); diff --git a/src/plot_host.cpp b/src/plot_host.cpp index 4d64416..1bdc4b3 100644 --- a/src/plot_host.cpp +++ b/src/plot_host.cpp @@ -452,7 +452,19 @@ void RowSet::set_time_window(double seconds) { // `{0, window}` here instead would state an offset of a whole epoch. figure_->set_x_link_range(x_group_, {newest - window_s_, newest}); } else { - figure_->set_x_link_range(x_group_, {0.0, window_s_}); + // No edge to read: there are no samples bound yet, which is the state every structural + // change leaves behind -- a re-plan drops the rings' history, and this function is called + // from `apply_time_window()`, which is what re-plans. So this is not a rare fallback; it + // is the axis a mode change is drawn with until the first sample lands. + // + // Which is why it states the window the same way the branch above does, with its maximum + // *at* the edge the live view puts at x = 0. `{0, window}` -- what it asked for before -- + // is an offset of a whole window by the rule the comment above states, so the frames + // between the re-plan and the first sample drew the time base one screen to the left of + // where the following frames drew it. Two or three frames, every time an event or a + // waveform was opened: a trace that jumps a screen sideways and comes back is the + // "the time base flickers and it is unusable" report. + figure_->set_x_link_range(x_group_, {-window_s_, 0.0}); } } diff --git a/src/ui.cpp b/src/ui.cpp index 330e0ee..8f4bf82 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -4827,6 +4827,20 @@ void on_activate(GtkApplication* application, gpointer user) { gtk_widget_add_controller(app->window, GTK_EVENT_CONTROLLER(press)); gtk_window_present(GTK_WINDOW(app->window)); + + // 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 + // it runs, so the window does not change. Set here it is on screen already: the present paints a + // frame before any timeout fires, which is the difference between waiting and "the dialog + // disappeared and nothing happened". + if (app->waveform_name.empty()) { + const Config& cfg = app->model->config(); + char buf[192]; + std::snprintf(buf, sizeof(buf), "connecting to %s:%u -- planning the %.0f MB buffer", + cfg.server.c_str(), unsigned(cfg.port), cfg.buffer_mb); + set_notice(*app, buf, 6); + } } /// Headless self-check, step 2: read the panel geometry while the widgets still diff --git a/src/ui_app.hpp b/src/ui_app.hpp index 7159f6f..6661386 100644 --- a/src/ui_app.hpp +++ b/src/ui_app.hpp @@ -743,6 +743,12 @@ struct App { /// it keeps for a second latch (it is counting retries, not time). int dbg_events_step = 0; int dbg_events_wait = 0; + /// The event row the probe selected before it activated one, and how many rows were selected. + /// Both are read back *after* the activation: a row pointer that is no longer the list's row 0, + /// or a smaller count, says the activation rebuilt the list and so threw away the user's scroll + /// position and their comparison selection. + GtkListBoxRow* dbg_events_row0 = nullptr; + int dbg_events_selected = 0; /// Which step of the plot-context-menu probe runs next (`UDPSCOPE_PROBE_ROWMENU_MS`), and the /// popover it is measuring. int dbg_rowmenu_step = 0; @@ -1097,7 +1103,13 @@ constexpr double kWirePick = 6.0; constexpr int kZoomGestureSteps = 8; constexpr int kZoomWindowSteps = 8; constexpr int kEventThumbW = 138; +/// The row thumbnail's height floor and ceiling. A floor and not a fixed size: the stored preview is +/// `44 px` per signal, so its proportions depend on how many signals the event has, and a row shows +/// it at *its* aspect -- a one-signal event is a short row and a six-signal one a tall one. The +/// ceiling is what keeps a wide event from making a row taller than the pane; the full picture is a +/// hover away. constexpr int kEventThumbH = 46; +constexpr int kEventThumbMaxH = 104; inline int g_stats_phase = 0; inline std::vector g_stats_n_before; inline double g_trigrenum_edge = 0.0; @@ -1333,6 +1345,15 @@ GdkTexture* scale_texture(GdkTexture* source, int width, int height); EventThumbCache* event_thumb_cache(App& app); GdkTexture* event_preview_texture(App& app, const std::string& path); void refresh_events_pane(App& app); +/// Re-marks which row is the event currently on screen, **without rebuilding the list**. +/// +/// The mark is the only thing about a row that a replay changes -- the picture and the numbers +/// are the file's -- and rebuilding the list to restate it threw away the scroll position and +/// the selection with it, so reaching the thirtieth event meant scrolling to it again after +/// every click, and a selection made for a comparison was lost by the act of looking at one of +/// its members. Call this for a change of what is on screen, and `refresh_events_pane()` only +/// for a change of what is *cached*. +void mark_events_on_screen(App& app); void on_event_row_activated(GtkListBox*, GtkListBoxRow* row, gpointer user); GtkWidget* build_events_pane(App& app); gboolean tick(gpointer user); diff --git a/src/ui_events.cpp b/src/ui_events.cpp index a693bde..ef7102d 100644 --- a/src/ui_events.cpp +++ b/src/ui_events.cpp @@ -215,6 +215,10 @@ bool go_live_again(App& app) { app.sync_last_b.clear(); app.rows->set_trace_alpha(1.0f); rebuild_signal_panels(app); + // Nothing is on screen from the cache any more, so no row may claim to be. In place, for the + // reason `on_event_row_activated()` gives -- and this is why the mark went stale here before: + // rebuilding the list was too expensive to do for it. + mark_events_on_screen(app); set_notice(app, "live", 3); return true; } @@ -361,7 +365,14 @@ bool build_event_group(App& app, const std::vector& records, EventG app.model->set_target_span(span); app.model->set_signals(std::move(signals)); - for (std::size_t i = 0; i < cols * layers; ++i) app.model->set_signal_enabled(i, true); + // One call, so the group's eight channels are planned once: a loop of single enables + // re-planned -- and reallocated -- the whole buffer per channel (see + // `ScopeModel::set_signals_enabled`), which is most of what made opening a comparison slow. + { + std::vector all(cols * layers); + for (std::size_t i = 0; i < all.size(); ++i) all[i] = i; + app.model->set_signals_enabled(all, true); + } const f32 gap = std::numeric_limits::quiet_NaN(); std::vector values(cols * layers, 0.0f); @@ -556,8 +567,24 @@ void on_group_clicked(GtkButton* button, gpointer user) { EventThumbCache* event_thumb_cache(App& app) { if (app.events_list == nullptr) return nullptr; - return static_cast( + auto* cache = static_cast( g_object_get_data(G_OBJECT(app.events_list), "udpscope-event-thumbs")); + if (cache != nullptr) return cache; + // Created on first use and owned by the list. Nothing used to create it, and a missing cache is + // not a slow path here but a *blank* one: `event_preview_texture()` answers from the cache or not + // at all, so every row came up without a picture -- which is what the previews were reported for. + // Owned by the list because nothing else has the pane's lifetime: textures of a list that no + // longer exists are so much held memory. + cache = new EventThumbCache(); + g_object_set_data_full(G_OBJECT(app.events_list), "udpscope-event-thumbs", cache, + [](gpointer p) { + auto* dead = static_cast(p); + for (auto& entry : *dead) { + if (entry.second != nullptr) g_object_unref(entry.second); + } + delete dead; + }); + return cache; } /// The stored preview scaled to a row's size. Scaled here, once, for two reasons: a `GtkPicture`'s @@ -609,13 +636,84 @@ GdkTexture* event_preview_texture(App& app, const std::string& path) { const auto found = cache->find(path); if (found != cache->end()) return found->second; GdkTexture* stored = gdk_texture_new_from_filename(path.c_str(), nullptr); - GdkTexture* texture = - stored != nullptr ? scale_texture(stored, kEventThumbW, kEventThumbH) : nullptr; - if (stored != nullptr) g_object_unref(stored); + GdkTexture* texture = nullptr; + if (stored != nullptr) { + // The stored picture's *own* proportions, fitted to the row's width: the preview is now as tall + // as its signal count makes it, and squeezing an eight-strip picture into the height a one-strip + // picture gets is what made a row unreadable. Capped, because the row still has to be a row. + const int sw = gdk_texture_get_width(stored); + const int sh = gdk_texture_get_height(stored); + const int height = + sw > 0 ? std::clamp(kEventThumbW * sh / sw, kEventThumbH, kEventThumbMaxH) : kEventThumbH; + texture = scale_texture(stored, kEventThumbW, height); + g_object_unref(stored); + } cache->emplace(path, texture); return texture; } +/// Hovering a row's thumbnail shows the stored preview at full size. The row's picture is 138 pixels +/// wide and the stored one is 480: the small one says *which* event this is, and this says what is in +/// it, which is the whole reason a preview is worth keeping. +/// +/// Through GTK's own tooltip rather than a popover of ours, for lifetime: a tooltip is shown, placed, +/// dismissed and destroyed by GTK, and it takes no pointer grab -- a popover per row would be a +/// parented widget to unparent on every rebuild of the pane, and a grab to lose (see the GTK +/// invariants on both). The texture is decoded in the handler and dropped with the tooltip's picture, +/// so a hover costs nothing once it is over and the row cache keeps holding only the small one. +gboolean on_preview_query_tooltip(GtkWidget*, int, int, gboolean, GtkTooltip* tooltip, gpointer user) { + const char* path = static_cast(user); + if (path == nullptr || *path == '\0' || tooltip == nullptr) return FALSE; + GdkTexture* full = gdk_texture_new_from_filename(path, nullptr); + if (full == nullptr) return FALSE; + GtkWidget* picture = gtk_picture_new_for_paintable(GDK_PAINTABLE(full)); + g_object_unref(full); + // `can_shrink` FALSE: the point is the full size, and a tooltip would otherwise be given the + // smallest size its content allows. + gtk_picture_set_can_shrink(GTK_PICTURE(picture), FALSE); + gtk_tooltip_set_custom(tooltip, picture); + return TRUE; +} + +void attach_preview_zoom(GtkWidget* frame, const std::string& path) { + if (frame == nullptr || path.empty()) return; + gtk_widget_set_has_tooltip(frame, TRUE); + // The path outlives the signal: owned by the closure and freed with the widget, because the record + // it came from is a loop variable. + g_signal_connect_data(frame, "query-tooltip", G_CALLBACK(on_preview_query_tooltip), + g_strdup(path.c_str()), [](gpointer p, GClosure*) { g_free(p); }, + GConnectFlags(0)); +} + +/// A row's first line: when the event was latched, how long it is, and whether it is the one on +/// screen. One function because two callers write it -- the row builder and the in-place +/// re-mark -- and a mark that reads differently depending on which of them last ran would be +/// worse than no mark. +std::string event_row_title(App& app, const EventRecord& r) { + const std::time_t t = static_cast(r.wall_time); + std::tm tm{}; + localtime_r(&t, &tm); + char clock[32]; + std::strftime(clock, sizeof(clock), "%H:%M:%S", &tm); + char title[160]; + std::snprintf(title, sizeof(title), "%s %.3f s%s", clock, r.duration_s, + std::string(app.waveform_name) == r.file ? " (on screen)" : ""); + return title; +} + +void mark_events_on_screen(App& app) { + if (app.events_list == nullptr) return; + for (GtkWidget* row = gtk_widget_get_first_child(app.events_list); row != nullptr; + row = gtk_widget_get_next_sibling(row)) { + const auto* record = + static_cast(g_object_get_data(G_OBJECT(row), "udpscope-event")); + GtkWidget* name = GTK_WIDGET(g_object_get_data(G_OBJECT(row), "udpscope-event-title")); + // The "no events yet" placeholder is a row with neither: skipped, not asserted about. + if (record == nullptr || name == nullptr) continue; + gtk_label_set_text(GTK_LABEL(name), event_row_title(app, *record).c_str()); + } +} + /// The events pane's rows, newest first: each row is the picture of *that* event beside the /// sidecar's numbers, because what tells one event from another is its shape, and one picture for /// the whole pane describes whichever event happens to be newest rather than the one being read. The @@ -631,7 +729,8 @@ void refresh_events_pane(App& app) { // the one that finds out. app.events_thumb = nullptr; if (app.event_store == nullptr) return; - const std::vector records = app.event_store->list(); + // Not const: a record whose preview had to be redrawn learns its thumbnail's path here. + std::vector records = app.event_store->list(); if (records.empty()) { GtkWidget* empty = gtk_label_new("no events yet -- arm the trigger"); gtk_widget_add_css_class(empty, "dim-label"); @@ -657,10 +756,8 @@ void refresh_events_pane(App& app) { } } std::size_t unreadable = 0; - for (const EventRecord& r : records) { + for (EventRecord& r : records) { GtkWidget* row = gtk_list_box_row_new(); - g_object_set_data_full(G_OBJECT(row), "udpscope-event", - new EventRecord(r), [](gpointer p) { delete static_cast(p); }); GtkWidget* line = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 8); gtk_widget_set_margin_start(line, 6); @@ -688,8 +785,24 @@ void refresh_events_pane(App& app) { gtk_widget_add_css_class(frame, "udpscope-event-thumb"); gtk_box_append(GTK_BOX(frame), shot); GdkTexture* texture = event_preview_texture(app, r.thumbnail); + if (texture == nullptr && app.event_store != nullptr) { + // Drawn again from the samples, which are still there: a row with no picture is the state the + // user reported as "sometimes missing", and the `.npy` is the event -- the `.bmp` is a picture + // of it. Once per row and never in a loop: a second failure is a real one, and the cache keeps + // the null so the next refresh does not read the file again. + const std::string redrawn = app.event_store->redraw_preview(r); + if (!redrawn.empty()) { + if (EventThumbCache* cache = event_thumb_cache(app)) cache->erase(redrawn); + r.thumbnail = redrawn; + texture = event_preview_texture(app, redrawn); + } + } if (texture != nullptr) { gtk_picture_set_paintable(GTK_PICTURE(shot), GDK_PAINTABLE(texture)); + // The frame takes the *texture's* height, so a tall preview gets a tall row rather than being + // squashed into the floor. Its width is fixed: the rest of the row belongs to the text. + gtk_widget_set_size_request(frame, kEventThumbW, gdk_texture_get_height(texture)); + attach_preview_zoom(frame, r.thumbnail); } else { ++unreadable; } @@ -702,15 +815,11 @@ void refresh_events_pane(App& app) { gtk_widget_set_valign(box, GTK_ALIGN_CENTER); gtk_widget_set_hexpand(box, TRUE); - const std::time_t t = static_cast(r.wall_time); - std::tm tm{}; - localtime_r(&t, &tm); - char clock[32]; - std::strftime(clock, sizeof(clock), "%H:%M:%S", &tm); - char title[160]; - std::snprintf(title, sizeof(title), "%s %.3f s%s", clock, r.duration_s, - std::string(app.waveform_name) == r.file ? " (on screen)" : ""); - GtkWidget* name = gtk_label_new(title); + GtkWidget* name = gtk_label_new(event_row_title(app, r).c_str()); + // Kept on the row: the title is the one part of a row that changes without the cache + // changing -- the "(on screen)" mark follows a replay -- and `mark_events_on_screen()` + // rewrites it through this instead of rebuilding the list. + g_object_set_data(G_OBJECT(row), "udpscope-event-title", name); gtk_widget_set_halign(name, GTK_ALIGN_START); gtk_label_set_ellipsize(GTK_LABEL(name), PANGO_ELLIPSIZE_END); // A cap, not only an ellipsis: `max-width-chars` bounds a label's *natural* width, and the row's @@ -741,6 +850,10 @@ void refresh_events_pane(App& app) { gtk_box_append(GTK_BOX(box), extra); gtk_box_append(GTK_BOX(line), box); + // Attached *after* the thumbnail was resolved: a preview redrawn a moment ago is part of this + // record now, and the copy the row carries is what every later reader of the row sees. + g_object_set_data_full(G_OBJECT(row), "udpscope-event", new EventRecord(r), + [](gpointer p) { delete static_cast(p); }); gtk_list_box_row_set_child(GTK_LIST_BOX_ROW(row), line); gtk_list_box_append(GTK_LIST_BOX(app.events_list), row); } @@ -759,9 +872,11 @@ void on_event_row_activated(GtkListBox*, GtkListBoxRow* row, gpointer user) { auto* record = static_cast(g_object_get_data(G_OBJECT(row), "udpscope-event")); if (record == nullptr) return; replay_event(app, *record); - // Rebuilt, not for the picture -- the row already has it -- but for the "on screen" mark the - // replay just changed. - refresh_events_pane(app); + // The mark the replay just moved, rewritten on the rows that are already there. Rebuilding + // the list -- which is what this did -- discarded the scroll position and the selection to + // restate one word, so a click on an event scrolled the list back to the top and dropped the + // pair a comparison was being assembled from. + mark_events_on_screen(app); } /// The pane itself: a collapsible column on the left, a header and the list of events -- each row diff --git a/src/ui_layout.cpp b/src/ui_layout.cpp index 2f705ab..c056574 100644 --- a/src/ui_layout.cpp +++ b/src/ui_layout.cpp @@ -284,18 +284,38 @@ void apply_layout(App& app, const LayoutState& st) { ScopeModel& model = *app.model; const auto& signals = model.signals(); - // The session's own state first: enabling is what starts acquisition, and a math channel's - // *name* is only a signal once its graph is installed -- a row that holds one would otherwise be - // skipped as a name the stream no longer has. + // The window before anything structural. Nothing is enabled yet, so there are no channels + // to plan for and this only *records* the span -- but it is the span every plan below is + // then made with, so the `apply_time_window()` at the end of this function finds the + // decimation already right and re-plans nothing. Stated last, as it used to be, it moved + // the stride and bought a second full allocation of the buffer after the data was already + // flowing into the first. + if (st.window_s > 0.0) model.set_target_span(st.window_s); + + // The math graph before the enables, because a math channel's *name* is only a signal once + // its graph is installed -- a row that holds one would otherwise be skipped as a name the + // stream no longer has -- and because installing it while nothing is enabled is the one + // ordering in which it does not pay for a buffer of its own. `signals` is a reference to + // the model's own vector, so the synthetic entries this appends are visible below. + app.math = st.math; + apply_math_channels(app); + + // What the session had on, collected and enabled **once**, at the end of this function. + // Each enable re-plans the partition, and a re-plan allocates -- and so touches -- the whole + // of `--buffer-mb`: seven signals one at a time is seven allocations before a single sample + // is kept, which is the ten seconds of blank window a large buffer used to start with. The + // math channels are not collected here: their enable state is the session's `math` and + // `set_math_channels()` has already restored it, so listing them again could turn on one + // the user saved off. + std::vector wanted; + wanted.reserve(st.enabled.size()); for (const std::string& name : st.enabled) { for (std::size_t si = 0; si < signals.size(); ++si) { if (signals[si].name != name || signals[si].math) continue; - model.set_signal_enabled(si, true); + wanted.push_back(si); break; } } - app.math = st.math; - apply_math_channels(app); app.measures = st.measures; app.measure_stats_on = st.measure_stats; app.updating_panels = true; @@ -341,12 +361,18 @@ void apply_layout(App& app, const LayoutState& st) { // Enabling is what starts acquisition (a loaded plot nobody acquires would // come back blank), and it is what a drop does for the signal it places. app.rows->assign(i, slot, name, name, signal_color_index(signals, si)); - model.set_signal_enabled(si, true); + wanted.push_back(si); ++slot; break; } } } + // The one structural change of a restored session: everything the file had on, and + // everything its rows placed, in a single re-plan. Enabling is what starts acquisition (a + // loaded plot nobody acquires would come back blank), and it is what a drop does for the + // signal it places -- so a row holding a signal the `enabled` list did not name is enabled + // here, by having been collected above. + model.set_signals_enabled(wanted, true); for (const auto& cal : st.calibration) { for (std::size_t si = 0; si < signals.size(); ++si) { diff --git a/src/ui_probe.cpp b/src/ui_probe.cpp index 8602d46..9c886aa 100644 --- a/src/ui_probe.cpp +++ b/src/ui_probe.cpp @@ -1892,8 +1892,13 @@ gboolean layout_probe_step(gpointer user) { gboolean enable_all_probe(gpointer user) { App* a = static_cast(user); - for (std::size_t i = 0; i < a->model->signals().size(); ++i) { - a->model->set_signal_enabled(i, true); + // In one call, the way a restored session does it: enabling them one at a time re-plans + // the buffer per signal, so a probe that did that would not be measuring the path a real + // start takes (and `[gen]` would read a burst that the application no longer produces). + { + std::vector all(a->model->signals().size()); + for (std::size_t i = 0; i < all.size(); ++i) all[i] = i; + a->model->set_signals_enabled(all, true); } int row = 0; int slot = 0; @@ -1908,8 +1913,12 @@ gboolean enable_all_probe(gpointer user) { } } a->relayout_pending = true; - std::fprintf(stderr, "[enable-all-probe] %zu channel(s) enabled and assigned to plots\n", - a->model->channel_count()); + // The startup notice comes with the reading, because it is the window's only answer to "why is + // nothing happening yet": it is set as the window is presented, before the first CONFIG can take + // the model's mutex, and it names the buffer being planned. An empty `notice` here is the window + // that used to come up silent while a large buffer was allocated. + std::fprintf(stderr, "[enable-all-probe] %zu channel(s) enabled and assigned to plots, notice='%s'\n", + a->model->channel_count(), a->notice.c_str()); std::fflush(stderr); // Then switch one off through its own switch in the signal list: a disabled signal must leave the plots, // not sit there drawing nothing in a slot a live one could take. Late enough that a math probe armed behind @@ -3158,6 +3167,40 @@ gboolean events_probe_step(gpointer user) { } if (a->dbg_events_step == 1) { + // The preview's repair, measured before anything else reads the pane: the newest event's picture + // is deleted and the pane refreshed, so the row that comes back proves the pane draws it again + // from the samples rather than showing a blank. The stored geometry is read back too, because the + // defect this fixes was a readable *file* that was too short to read -- `strip_h * signals` says + // the picture grew with the signal count, a fixed 160 says it did not. + const std::vector before = a->event_store->list(); + if (!before.empty()) { + const std::string bmp = a->event_store->dir() + "/" + before.front().stem + ".bmp"; + ::remove(bmp.c_str()); + // The decoded picture has to go with the file: the cache answers by path and would hand back + // the texture of a file that is no longer there, so the refresh would never reach the repair. + if (EventThumbCache* cache = event_thumb_cache(*a)) { + for (auto& entry : *cache) { + if (entry.second != nullptr) g_object_unref(entry.second); + } + cache->clear(); + } + refresh_events_pane(*a); + int w = -1; + int h = -1; + if (std::FILE* f = std::fopen(bmp.c_str(), "rb"); f != nullptr) { + unsigned char head[26] = {}; + if (std::fread(head, 1, sizeof(head), f) == sizeof(head)) { + w = int(head[18]) | int(head[19]) << 8 | int(head[20]) << 16; + h = int(head[22]) | int(head[23]) << 8 | int(head[24]) << 16; + } + std::fclose(f); + } + const bool drawn = a->events_thumb != nullptr && + gtk_picture_get_paintable(GTK_PICTURE(a->events_thumb)) != nullptr; + std::fprintf(stderr, "[events] preview redrawn: signals=%zu stored=%dx%d row_drawn=%d\n", + before.front().signals, w, h, drawn ? 1 : 0); + std::fflush(stderr); + } const std::vector records = a->event_store->list(); std::fprintf(stderr, "[events] dir=%s n=%zu", a->event_store->dir().c_str(), records.size()); if (!records.empty()) { @@ -3185,6 +3228,18 @@ gboolean events_probe_step(gpointer user) { // "framed" from "untouched". apply_time_window(*a, 0.5); GtkListBoxRow* row = gtk_list_box_get_row_at_index(GTK_LIST_BOX(a->events_list), 0); + // Selected *and* remembered, because what the activation must not do is rebuild the list: + // a selection the user made for a comparison, and the row object itself, are what a + // rebuild silently discarded. Two rows, so the reading after the click can say the + // selection survived rather than merely that something is selected. + if (row != nullptr) gtk_list_box_select_row(GTK_LIST_BOX(a->events_list), row); + if (GtkListBoxRow* second = gtk_list_box_get_row_at_index(GTK_LIST_BOX(a->events_list), 1)) { + gtk_list_box_select_row(GTK_LIST_BOX(a->events_list), second); + } + a->dbg_events_row0 = row; + GList* chosen = gtk_list_box_get_selected_rows(GTK_LIST_BOX(a->events_list)); + a->dbg_events_selected = int(g_list_length(chosen)); + g_list_free(chosen); if (row != nullptr) g_signal_emit_by_name(a->events_list, "row-activated", row); } // One layout pass before the picture is measured: a widget created a moment ago has no @@ -3198,10 +3253,18 @@ gboolean events_probe_step(gpointer user) { // is set, so a non-null one says nothing about whether anything is on screen. const bool picture = a->events_thumb != nullptr && gtk_picture_get_paintable(GTK_PICTURE(a->events_thumb)) != nullptr; + // The row *object*, not its index: a rebuilt list has a row 0 too, so only identity tells a + // list that was left alone from one that was torn down and refilled under the user's scroll. + const bool rows_kept = + a->dbg_events_row0 != nullptr && + gtk_list_box_get_row_at_index(GTK_LIST_BOX(a->events_list), 0) == a->dbg_events_row0; + GList* still = gtk_list_box_get_selected_rows(GTK_LIST_BOX(a->events_list)); + const int selected_now = int(g_list_length(still)); + g_list_free(still); std::fprintf(stderr, "[events] activated row0: replaying=%d pane=%d pane_w=%d pane_box=%d tick=%d " "window=%.3f axis=[%.3f,%.3f] marker=%.3f " - "thumbnail=%d thumb_size=%dx%d thumb_visible=%d\n", + "thumbnail=%d thumb_size=%dx%d thumb_visible=%d rows_kept=%d selected=%d/%d\n", a->replaying ? 1 : 0, a->events_revealer != nullptr && gtk_widget_get_visible(a->events_revealer) ? 1 : 0, a->events_divider != nullptr ? gtk_paned_get_position(GTK_PANED(a->events_divider)) @@ -3226,7 +3289,8 @@ gboolean events_probe_step(gpointer user) { picture ? 1 : 0, a->events_thumb != nullptr ? gtk_widget_get_width(a->events_thumb) : -1, a->events_thumb != nullptr ? gtk_widget_get_height(a->events_thumb) : -1, - a->events_thumb != nullptr && gtk_widget_get_mapped(a->events_thumb) ? 1 : 0); + a->events_thumb != nullptr && gtk_widget_get_mapped(a->events_thumb) ? 1 : 0, + rows_kept ? 1 : 0, selected_now, a->dbg_events_selected); std::fflush(stderr); events_probe_next(*a, 2, 1200); return G_SOURCE_REMOVE; diff --git a/src/waveform.cpp b/src/waveform.cpp index 0f7ab4c..66f762e 100644 --- a/src/waveform.cpp +++ b/src/waveform.cpp @@ -272,7 +272,11 @@ bool load_waveform_into_model(ScopeModel& model, const std::string& path, } model.set_target_span(reader.rows() * (dt > 0.0 ? dt : 0.001)); model.set_signals(std::move(signals)); - for (std::size_t i = 0; i < reader.names().size(); ++i) model.set_signal_enabled(i, true); + // All of them in one call: one re-plan, hence one allocation of the buffer, where a loop + // of single enables planned it once per column (see `set_signals_enabled`). + std::vector all(reader.names().size()); + for (std::size_t i = 0; i < all.size(); ++i) all[i] = i; + model.set_signals_enabled(all, true); const std::size_t channels = model.channels().size(); if (channels == 0) {