diff --git a/README.md b/README.md index 9284d98..b09c950 100644 --- a/README.md +++ b/README.md @@ -250,8 +250,9 @@ A sample exactly *at* a threshold is still on neither side, so a signal that mer does not count every sample as an event. Each measurement's row carries **two badges**: its own colour (the same one its marks use on -the plot) and the signal's chip, in the palette colour every other view names that signal -with. The dialog's fields and their wording follow the type — the split row is absent for +the plot) and the signal's chip, in the colour every other view names that signal with — +through `RowSet::signal_color()`, so a signal the user re-coloured is re-coloured here too and +not left at the palette entry. The dialog's fields and their wording follow the type — the split row is absent for `average`, `min` and `max`, the side row only for `pulse duration`, and the "show on plot" line says what would be drawn. @@ -1203,6 +1204,20 @@ shape and the column of chips reads as a legend. Because `plot::Plot` fixes a tr colour at `add_trace()`, the index is stored on the row (`Row::color_index`) and a colour change goes through a figure rebuild, which a drop already forces. +**Clicking the chip of a signal that is on a plot opens a colour chooser** (`GtkColorDialog`, +with the alpha slider off — a trace is stroked opaque, so a transparency setting would do +nothing), and the colour it returns *overrides* the palette entry for that signal. The chip of +a signal on no plot is not clickable: a grey chip names no trace, so there is nothing to +recolour. A chosen colour is held by signal **name** (`RowSet::set_signal_color()`), saved in +the session file's `[[color]]` tables and resolved — for the trace and for every chip — by the +one call `RowSet::signal_color()`, so a chip cannot claim a colour the trace is not drawn in. +The way back is **Default colour** in the signal's `⋯` menu, sensitive only while an override +exists; it is there rather than on the chip because a second gesture on a one-glyph target is +not something anybody would find. The chooser is opened by a `GtkGestureClick` on the existing +label rather than by wrapping it in a `GtkButton`, because a button's padding is exactly what +made this row twice as tall once (below), and the gesture claims its sequence so a click on the +chip cannot become the start of a row drag. + A `GtkSwitch` and two always-visible buttons made a row about twice as tall (46 px against 26 px now), which is what turned a four-signal stream into a panel you have to scroll; Adwaita's diff --git a/docs/claude/invariants-plot-and-layout.md b/docs/claude/invariants-plot-and-layout.md index 30ee674..50b2963 100644 --- a/docs/claude/invariants-plot-and-layout.md +++ b/docs/claude/invariants-plot-and-layout.md @@ -104,3 +104,14 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear figure rebuild — which `assign()` already forces. A signal on no plot gets `RowSet::unplotted_color()` (grey) rather than no chip, so the panel's rows stay the same height. +- **`RowSet::signal_color()` is the only route to a trace's colour.** The user can override the + palette entry per signal (clicking its chip; `set_signal_color()` / `clear_signal_color()`, + keyed by **name**, because a palette index is positional and a CONFIG renumbers), and that + override is exactly the state in which two routes to a colour disagree. So `install()` builds + the trace through this call and all three chip sites — the signal panel, the measures pane and + the cursors pane — read it too; none of them may index `palette_color()` itself. An override + marks every row holding the signal dirty (`mark_signal_dirty()`, not `assign()`: what the slot + *holds* did not change) and the rebuild happens on the frame tick via `relayout_pending`, + because `relayout_areas()` is the one place that builds a figure and re-points the area at it. + `UDPSCOPE_PROBE_COLOR_MS` reads the result back out of `plot::Plot::trace()` and out of the + chip's own markup, since asking the resolver twice would assert nothing. diff --git a/docs/claude/invariants-session-and-export.md b/docs/claude/invariants-session-and-export.md index 285cbf8..b25c629 100644 --- a/docs/claude/invariants-session-and-export.md +++ b/docs/claude/invariants-session-and-export.md @@ -6,7 +6,10 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear - **The session is the layout.** `LayoutState` persists what makes a reopened scope look like the one that was closed: the window, the rows and their slot signals, the calibration, **the enabled - signals**, **the math channels with their graphs** and **the measurements pane**. Signal identity + signals**, **the chosen trace colours** (`[[color]]`, `name` + `rgb = "#RRGGBB"` — only the + *overrides*, so a stream's new signals still take the positional palette and a saved session does + not freeze a colour that is meant to follow the signal list), **the math channels with their + graphs** and **the measurements pane**. Signal identity is the *name* throughout (a CONFIG renumbers channels), 'enabled' is read off the channels the model built -- an enabled signal is exactly one that produced a channel -- and `apply_layout()` installs the graphs **before** assigning the rows, since a row may hold a math channel's name and diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 4e2a1bc..c36fe86 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -571,5 +571,6 @@ Two lines of the self-check output are assertions in disguise: itself when it loses the grab, and `xvfb-run` has no window manager to hand it one — and restores it afterwards. - `[keys] a: armed 0->1 | space: paused 0->1 | t: trigger panel: 0->1 | … | editable focused: 'm' refused=1` (stderr, `UDPSCOPE_PROBE_KEYS_MS`) — **every `X->Y` pair must differ** and **`refused` must be 1**. The refusal is the whole reason the controller is in the bubble phase: every bare-letter accel is also a character, so an accel that fires while a `GtkEditable` has focus makes the window entry unusable. A pair reading `0->0` is an action whose dispatch does not reach the handler a click reaches. Note the format is `: : ->` — the name passed to `before_after()` includes the widget's description, so `t: trigger panel: 0->1` is the correct form (not `t: trigger panel 0->1` without the colon before the values). +- `[color] '' at row R slot S: trace=#RRGGBB chip_agrees=1 (want 1)`, then `[color] override: trace=#123456 chip_agrees=1`, `[color] session: entries=1 rgb=#123456` and `[color] default: trace=#RRGGBB chip_agrees=1` (stderr, `UDPSCOPE_PROBE_COLOR_MS`) — **all three `chip_agrees` must be 1**, the override line's `trace` must be `#123456`, the session line must read `entries=1 rgb=#123456`, and the default line's `trace` must be back to the first line's. Run it with `UDPSCOPE_ENABLE_ALL_MS` ahead of it, or there is no bound trace and the probe prints `no bound trace to recolour` instead of asserting anything; on the standard mock the first line reads `'sine' at row 0 slot 0: trace=#4FA3FF`. Two things make this an assertion rather than a tautology. The colours are read out of **`plot::Plot::trace().color`** — what the renderer will stroke, reached through `Row::plot` *after* `relayout_areas()`, since a new colour is a new figure — and out of the chip's own `` markup, not by asking `RowSet::signal_color()` twice, which would pass against a chip that never reached the trace. And the override is `#123456`, deliberately in no palette slot: an override equal to the entry it replaces passes against an implementation that ignores overrides entirely. The session leg round-trips `collect_layout()` → `save_layout()` → `load_layout()` on a temp path and deletes it, because a probe must not leave a colour in the real session file for a later run to inherit. - `[zoomkeys] ctrl+plus x=[..]->[..] y_held=1 | alt+plus y=[..]->[..] x_held=1` (stderr, `UDPSCOPE_PROBE_ZOOMKEYS_MS`) — **both `_held` must be 1 and both zooms must still happen.** Run it inside a pinned state, where a frame cannot re-frame the other axis for its own reasons: `UDPSCOPE_PROBE_PAUSE_MS=1500 UDPSCOPE_PROBE_ZOOMKEYS_MS=2200`. A `_held=0` is the modifier not reaching the plotter's controller; an unchanged range on the *zoomed* axis is the opposite failure, the key doing nothing. Ctrl restricts to x and Alt to y; Ctrl+Alt zooms neither (no special case needed — neither flag alone fires). - `status bar: glance=[● live · 1.09 MS/s · 2 s window] max_glance_chars=53 detail_fields=11 popover_rebuilds=0` (exit dump) — **`max_glance_chars` must stay small** (under about 60) and **`popover_rebuilds` must be 0**. The status row is directly above the plot, so this is the "layout must not depend on content" rule read from the bar's own side: the state field is one word from a fixed set (`connecting`, `live`, `paused`, `armed`, `capturing`, `event held`, `disarmed` — a word added to it must be **no wider than the widest already there**, which is what let `connecting` join at ten characters beside `event held`; the dead-port glance reads `max_glance_chars=51` against a live run's 54), the rate field is `%.3g` (so `10 kS/s`, never `10.0 kS/s`), and the two fields a *state* adds are both elapsed times — trigger mode's `last event … ago` and a pause's `for ` — which step units rather than growing digits (`format_elapsed()`). A **paused** glance is not in the dump at all: it is read by `[pause-probe]`'s `glance=` field, since the dump's own line is taken live. `detail_fields` is the popover's label count, a literal, so adding a label without thinking about layout shows up here. The glance rate format matters: `%.3g` drops the trailing zero that a `%.4g` or a fixed-precision format would keep — a rate that reads differently in the dump and in the help page is a format disagreement, and `14-status.txt`'s example must match it. diff --git a/docs/claude/verification.md b/docs/claude/verification.md index af8a9b5..bbf77b9 100644 --- a/docs/claude/verification.md +++ b/docs/claude/verification.md @@ -161,6 +161,11 @@ precedent, not an exception. stops while paused and "how long have I been looking at this" is a question about the user, not about the stream; it is written through `format_elapsed()` so the field steps to a coarser unit instead of growing digits (see the layout rule). +- `UDPSCOPE_PROBE_COLOR_MS` — recolour one plotted signal and read the colour back out of the + trace and the chip, round-trip it through the session writer, then clear it. Schedule + `UDPSCOPE_ENABLE_ALL_MS` before it: with nothing bound it has no trace to recolour and asserts + nothing. Its four lines are in + [probe-readings.md](probe-readings.md). - `UDPSCOPE_EXPORT_MS` (+ optional `UDPSCOPE_EXPORT_PATH`, default `/tmp/udpscope-export`) — collect the export table and write it both ways, skipping only the file chooser. diff --git a/src/help/03-signals.txt b/src/help/03-signals.txt index 01a4ca7..a5ad3d7 100644 --- a/src/help/03-signals.txt +++ b/src/help/03-signals.txt @@ -22,6 +22,9 @@ the overflow menu opens the editor where the unit and the calibration live. • A signal's colour follows the signal, not the slot, so the same signal on two rows is one colour and the side panel's chip identifies it unambiguously. A signal on no row gets a grey chip rather than none, so the panel's rows keep their height. +• Click a coloured chip to change that signal's colour. A grey chip names no trace, so + it is not clickable. The colour is saved with the session; Default colour in the + signal's ⋯ menu gives the palette entry back. Example diff --git a/src/layout_store.cpp b/src/layout_store.cpp index 23b9869..edfdcb0 100644 --- a/src/layout_store.cpp +++ b/src/layout_store.cpp @@ -239,13 +239,14 @@ bool load_layout(const std::string& path, LayoutState& out) { st.window_s = 0.0; st.rows.clear(); st.calibration.clear(); + st.colors.clear(); st.enabled.clear(); st.math.clear(); st.measures.clear(); std::string line; // What the last `[[...]]` header opened, so keys land in the right record. - enum class Section { Top, Plot, Calibration, Math, Measure } section = Section::Top; + enum class Section { Top, Plot, Calibration, Color, Math, Measure } section = Section::Top; int plot_count = 0; bool plot_section_seen = false; while (std::getline(in, line)) { @@ -259,6 +260,9 @@ bool load_layout(const std::string& path, LayoutState& out) { } else if (t == "[[calibration]]") { section = Section::Calibration; st.calibration.emplace_back(); + } else if (t == "[[color]]") { + section = Section::Color; + st.colors.emplace_back(); } else if (t == "[[math]]") { section = Section::Math; st.math.emplace_back(); @@ -378,6 +382,23 @@ bool load_layout(const std::string& path, LayoutState& out) { } catch (...) { } } + } else if (section == Section::Color && !st.colors.empty()) { + if (key == "name") { + st.colors.back().name = strip_quote(value); + } else if (key == "rgb") { + // `"#RRGGBB"`, so the file says what it means to a reader hand-editing it -- a bare decimal + // would be the one number in this file nobody could check by eye. Anything unparseable + // leaves the entry at 0, and `apply_layout()` drops an entry with no name anyway. + const std::string hex = strip_quote(value); + const std::size_t at = hex.find('#'); + try { + st.colors.back().rgb = + static_cast(std::stoul( + at == std::string::npos ? hex : hex.substr(at + 1), nullptr, 16)) & + 0x00FFFFFFu; + } catch (...) { + } + } } } if (!plot_section_seen && plot_count > 0) { @@ -422,6 +443,11 @@ bool save_layout(const std::string& path, const LayoutState& state) { o << "\n[[calibration]]\nname = " << toml_escape(cal.name) << "\ngain = " << cal.gain << "\noffset = " << cal.offset << "\n"; } + for (const auto& color : state.colors) { + char rgb[16]; + std::snprintf(rgb, sizeof(rgb), "\"#%06X\"", color.rgb & 0x00FFFFFFu); + o << "\n[[color]]\nname = " << toml_escape(color.name) << "\nrgb = " << rgb << "\n"; + } for (const MathChannel& m : state.math) { o << "\n[[math]]\nname = " << toml_escape(m.name) << "\nunit = " << toml_escape(m.unit) << "\nenabled = " << (m.enabled ? "true" : "false") << "\noutput = " << m.output diff --git a/src/layout_store.hpp b/src/layout_store.hpp index 7d7692c..35963d0 100644 --- a/src/layout_store.hpp +++ b/src/layout_store.hpp @@ -32,6 +32,15 @@ struct LayoutState { }; std::vector calibration; + /// The trace colours the user chose, `0xRRGGBB`. Only the *overrides*: a signal with no entry + /// takes the positional palette entry, which is what keeps a stream's new signals coloured + /// without a session having to name them. Saved by name like every identity here. + struct Color { + std::string name; + std::uint32_t rgb = 0; + }; + std::vector colors; + /// Signals the session had enabled. Enabling is what starts acquisition, so this is what makes a /// reopened scope look like the one that was closed -- and it is saved by *name*, like every other /// identity here, because a CONFIG renumbers the channels. diff --git a/src/plot_host.cpp b/src/plot_host.cpp index 46c05eb..37c2142 100644 --- a/src/plot_host.cpp +++ b/src/plot_host.cpp @@ -69,6 +69,40 @@ plot::Color RowSet::palette_color(int index) { plot::Color RowSet::unplotted_color() { return plot::Color::hex(kUnplottedColor); } +plot::Color RowSet::signal_color(const std::string& signal, int palette_index) const { + const auto it = signal_colors_.find(signal); + if (it != signal_colors_.end()) return plot::Color::hex(it->second); + return palette_color(palette_index); +} + +void RowSet::mark_signal_dirty(const std::string& signal) { + for (int i = 0; i < kMaxRows; ++i) { + const Row& r = row(i); + for (int s = 0; s < kSlotsPerRow; ++s) { + if (r.signal_name[static_cast(s)] != signal) continue; + row_dirty_[static_cast(i)] = true; + break; + } + } +} + +void RowSet::set_signal_color(const std::string& signal, plot::u32 rgb) { + if (signal.empty()) return; + // Masked to 24 bits: the palette is `0xRRGGBB` and `Color::hex()` reads it as such, so an alpha + // byte left in the high octet by a caller converting from a `GdkRGBA` would be silently ignored + // by the plot and silently written to the session file. + const plot::u32 want = rgb & 0x00FFFFFFu; + const auto it = signal_colors_.find(signal); + if (it != signal_colors_.end() && it->second == want) return; + signal_colors_[signal] = want; + mark_signal_dirty(signal); +} + +void RowSet::clear_signal_color(const std::string& signal) { + if (signal_colors_.erase(signal) == 0) return; + mark_signal_dirty(signal); +} + RowSet::RowSet() { rebuild_all(); } void RowSet::set_row_count(int rows) { @@ -130,8 +164,8 @@ void RowSet::install(Row& r, int /*row_index*/, plot::Plot& p) { {"", {-1.0, 1.0}, true, plot::AxisPlace::Left, false, plot::TickFormat::Si}); for (int s = 0; s < kSlotsPerRow; ++s) { const std::size_t i = static_cast(s); - r.plot->add_trace( - {&r.slots[i], palette_color(r.color_index[i]), 1.2f, r.axis_x, r.axis_y, r.name[i]}); + r.plot->add_trace({&r.slots[i], signal_color(r.signal_name[i], r.color_index[i]), 1.2f, + r.axis_x, r.axis_y, r.name[i]}); } r.cursor_a = r.plot->add_cursor(0.0, plot::Color::hex(kCursorAColor)); r.cursor_b = r.plot->add_cursor(0.0, plot::Color::hex(kCursorBColor)); diff --git a/src/plot_host.hpp b/src/plot_host.hpp index ebff018..03cd247 100644 --- a/src/plot_host.hpp +++ b/src/plot_host.hpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -399,7 +400,30 @@ class RowSet { /// names beside it shifted sideways as signals were placed. static plot::Color unplotted_color(); + /// The colour a signal is drawn in: the one the user chose for it if they chose one, the + /// positional palette entry otherwise. + /// + /// **The one resolver**, used by `install()` for the trace and by the panels for their chips. + /// A chip exists to say which trace on screen a panel row is about, so a chip that resolved + /// a colour by a different route than the trace is the single failure the chip cannot + /// survive -- and an override is exactly the state in which two routes would disagree. + plot::Color signal_color(const std::string& signal, int palette_index) const; + /// Gives `signal` the colour `rgb` (0xRRGGBB), by *name*: a palette entry is positional, so an + /// override held by index would move to another signal on the next CONFIG. Marks every row that + /// plots it dirty, because a trace's colour is fixed at `add_trace()` -- a new colour is a new + /// figure, which is the same path `assign()` already takes. + void set_signal_color(const std::string& signal, plot::u32 rgb); + /// Back to the palette entry. A no-op, and no rebuild, when there was no override. + void clear_signal_color(const std::string& signal); + /// Every override, for the session file. Ordered by name, so a saved session's colour + /// tables do not reshuffle between writes. + const std::map& signal_colors() const noexcept { return signal_colors_; } + private: + /// Marks every row with `signal` in a slot for rebuild. Not `assign()`: nothing about what + /// the slot *holds* changed, only the colour its trace has to be built with. + void mark_signal_dirty(const std::string& signal); + /// Moves one row's *persistent* state -- what it plots, in which colours, where its /// cursors are -- onto another row. Deliberately not a `Row` assignment: a `Row` also /// carries the figure handles (`plot`, the axis and cursor ids) and the `SlotSeries` @@ -440,6 +464,10 @@ class RowSet { double right_gutter_px_ = 0.0; bool y_gutter_reserved_ = false; std::array(kMaxRows)> storage_{}; + /// The colours the user chose, by signal name (see `set_signal_color()`). Sparse on purpose: + /// an absent entry means "the palette decides", which is what keeps a new CONFIG's signals + /// coloured without anything having to seed them. + std::map signal_colors_; CursorMode cursor_mode_ = CursorMode::Vertical; bool cursors_on_ = false; bool relative_x_ = false; diff --git a/src/ui.cpp b/src/ui.cpp index 3cd4b9e..7989b2c 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -719,6 +719,11 @@ LayoutState collect_layout(App& app) { if (model.signals()[si].math || !on[si]) continue; st.enabled.push_back(model.signals()[si].name); } + // Only the signals the user actually re-coloured: the rest take the positional palette, and + // writing those out would freeze a colour that is meant to follow the signal list. + for (const auto& [name, rgb] : app.rows->signal_colors()) { + st.colors.push_back({name, rgb}); + } // The app's own lists are the pane's source of truth for both of these. st.math = app.math; st.measures = app.measures; @@ -822,6 +827,16 @@ void apply_layout(App& app, const LayoutState& st) { } } + // Skipped for a name the stream no longer carries, like the calibration above: an override the + // signal list cannot show is one `collect_layout()` would write back every session for ever. + for (const auto& color : st.colors) { + for (std::size_t si = 0; si < signals.size(); ++si) { + if (signals[si].name != color.name) continue; + app.rows->set_signal_color(color.name, color.rgb); + break; + } + } + if (st.window_s > 0.0) apply_time_window(app, st.window_s); // set_row_count() rebuilt the figure (its row count is fixed at construction): @@ -1735,14 +1750,33 @@ int signal_color_index(const std::vector& signals, std::size_t signa return index; } -std::string chip_markup(plot::Color c) { - char buf[64]; - std::snprintf(buf, sizeof(buf), "\u25CF", - unsigned(c.r * 255.0f + 0.5f), unsigned(c.g * 255.0f + 0.5f), - unsigned(c.b * 255.0f + 0.5f)); +/// A trace colour as `#RRGGBB`: what a chip is drawn with and what the session file stores. One +/// formatter for both, so a chip and the colour it claims to name cannot be written differently -- +/// and so the probe can read a chip's own markup rather than recomputing what it should say. +std::string color_hex(plot::Color c) { + char buf[16]; + std::snprintf(buf, sizeof(buf), "#%02X%02X%02X", unsigned(c.r * 255.0f + 0.5f), + unsigned(c.g * 255.0f + 0.5f), unsigned(c.b * 255.0f + 0.5f)); return buf; } +std::string chip_markup(plot::Color c) { + return "\u25CF"; +} + +/// Whether any row's slot holds `name` -- i.e. whether the signal has a trace on screen, which is +/// what decides between its own colour and the grey of an unplotted one, and what decides whether +/// there is a colour for the user to change. +bool signal_plotted(App& app, const std::string& name) { + for (int r = 0; r < app.rows->row_count(); ++r) { + const RowSet::Row& row = app.rows->row(r); + for (int s = 0; s < RowSet::kSlotsPerRow; ++s) { + if (row.signal_name[static_cast(s)] == name) return true; + } + } + return false; +} + /// The chip for a row of the signal list, as Pango markup: the signal's own colour when /// it is on a plot, grey when it is not (whether that is because it is not acquired or /// because it is acquired and not placed). Always a filled disc, never a blank: a blank @@ -1751,14 +1785,78 @@ std::string chip_markup(plot::Color c) { std::string signal_chip_markup(App& app, const std::vector& signals, std::size_t signal) { const std::string& name = signals[signal].name; - for (int r = 0; r < app.rows->row_count(); ++r) { - const RowSet::Row& row = app.rows->row(r); - for (int s = 0; s < RowSet::kSlotsPerRow; ++s) { - if (row.signal_name[static_cast(s)] != name) continue; - return chip_markup(RowSet::palette_color(signal_color_index(signals, signal))); - } + if (!signal_plotted(app, name)) return chip_markup(RowSet::unplotted_color()); + // Through `RowSet::signal_color()`, which is also what `install()` builds the trace with: the + // chip's whole job is to say which trace this row is about, so it may not resolve a colour by + // its own route. A user's override is exactly the state in which two routes would disagree. + return chip_markup(app.rows->signal_color(name, signal_color_index(signals, signal))); +} + +/// What an open colour chooser is choosing for. The signal's **name**, not its index: the chooser +/// is asynchronous and a CONFIG can renumber the signal list while it is open, which is the same +/// reason every persisted identity in this app is a name. +struct ColorPick { + App* app; + std::string signal; +}; + +/// Applies a chosen colour, or leaves everything alone if the chooser was dismissed. +void on_signal_color_chosen(GObject* source, GAsyncResult* res, gpointer user) { + std::unique_ptr pick(static_cast(user)); + GError* error = nullptr; + GdkRGBA* picked = gtk_color_dialog_choose_rgba_finish(GTK_COLOR_DIALOG(source), res, &error); + if (picked == nullptr) { + // A dismissed chooser arrives here as an error (`GTK_DIALOG_ERROR_DISMISSED`, or a cancelled + // `GError` if the window went away), and a cancel must leave the colour as it was. There is + // nothing to report either way: the user closed a chooser. + if (error != nullptr) g_error_free(error); + return; } - return chip_markup(RowSet::unplotted_color()); + const auto octet = [](double v) { + return static_cast(std::clamp(v, 0.0, 1.0) * 255.0 + 0.5); + }; + const plot::u32 rgb = (octet(picked->red) << 16) | (octet(picked->green) << 8) | + octet(picked->blue); + gdk_rgba_free(picked); + + App& app = *pick->app; + app.rows->set_signal_color(pick->signal, rgb); + // The rebuild is deferred to the frame tick, like a drop's: a trace's colour is fixed at + // `add_trace()`, so a new colour is a new figure, and `relayout_areas()` is the one place that + // builds one and re-points the area at it. + app.relayout_pending = true; + rebuild_signal_panels(app); + schedule_layout_save(app); +} + +/// Opens the colour chooser for one signal's trace. +/// +/// `GtkColorDialog`, not the `GtkColorChooserDialog` deprecated in GTK 4.10: libadwaita 1.8 already +/// puts this build's floor at GTK 4.18, so the modern API is simply available. Alpha is off -- +/// `plot::Color` carries one but a trace is stroked opaque, so a transparency slider would offer a +/// setting that does nothing. +void open_signal_color(App& app, std::size_t signal) { + std::string name; + int palette_index = 0; + { + std::lock_guard lock(app.model->mutex()); + const std::vector& signals = app.model->signals(); + if (signal >= signals.size()) return; + name = signals[signal].name; + palette_index = signal_color_index(signals, signal); + } + if (name.empty()) return; + const plot::Color current = app.rows->signal_color(name, palette_index); + const GdkRGBA start{current.r, current.g, current.b, 1.0f}; + + GtkColorDialog* dialog = gtk_color_dialog_new(); + gtk_color_dialog_set_title(dialog, ("Colour for " + name).c_str()); + gtk_color_dialog_set_with_alpha(dialog, FALSE); + gtk_color_dialog_choose_rgba(dialog, GTK_WINDOW(app.window), &start, nullptr, + on_signal_color_chosen, new ColorPick{&app, name}); + // The async call holds its own reference for the operation's life, which is the documented + // idiom for every `Gtk*Dialog` in GTK 4.10's async family. + g_object_unref(dialog); } /// One line per signal: colour chip, checkbox, name, unit, and a menu for the things @@ -1786,11 +1884,34 @@ void rebuild_signal_list(App& app, const std::vector& signals) { gtk_widget_set_margin_start(box, 6); gtk_widget_set_margin_end(box, 2); + const bool plotted = signal_plotted(app, sig.name); GtkWidget* chip = gtk_label_new(nullptr); gtk_label_set_markup(GTK_LABEL(chip), signal_chip_markup(app, signals, i).c_str()); - gtk_widget_set_tooltip_text(chip, app.model->signal_enabled(i) - ? "the colour this signal is plotted in" - : "not acquired"); + gtk_widget_set_tooltip_text( + chip, !app.model->signal_enabled(i) ? "not acquired" + : plotted ? "the colour this signal is plotted in -- click to change it" + : "on no plot, so there is no trace to colour"); + if (plotted) { + // A gesture on the label rather than a `GtkButton` around it: a button brings its own + // padding, and padding on this row is what made the list twice as tall once already (see + // this function's note). The chip also has to stay exactly one disc wide, or the names + // beside it shift as signals are placed. + gtk_widget_set_cursor_from_name(chip, "pointer"); + GtkGesture* pick = gtk_gesture_click_new(); + g_object_set_data(G_OBJECT(pick), "udpscope-signal", GSIZE_TO_POINTER(i)); + g_signal_connect(pick, "pressed", + G_CALLBACK(+[](GtkGestureClick* g, int, double, double, gpointer user) { + App* a = static_cast(user); + // Claimed, so the row's `GtkDragSource` cannot take the same sequence + // and turn a click on the chip into the start of a drag. + gtk_gesture_set_state(GTK_GESTURE(g), GTK_EVENT_SEQUENCE_CLAIMED); + open_signal_color(*a, static_cast(GPOINTER_TO_SIZE( + g_object_get_data(G_OBJECT(g), + "udpscope-signal")))); + }), + &app); + gtk_widget_add_controller(chip, GTK_EVENT_CONTROLLER(pick)); + } gtk_box_append(GTK_BOX(box), chip); GtkWidget* enable = gtk_check_button_new(); @@ -1825,6 +1946,33 @@ void rebuild_signal_list(App& app, const std::vector& signals) { g_signal_connect(info, "clicked", G_CALLBACK(on_info_clicked), &app); gtk_box_append(GTK_BOX(menu_box), info); + // The way back from a chosen colour. The chooser itself cannot carry a "default" button, and a + // setting with no undo is a trap; it lives here rather than on the chip because a second + // gesture on a one-glyph target is not something a user would find. + GtkWidget* recolor = menu_button("Default colour", nullptr); + gtk_widget_set_sensitive(recolor, app.rows->signal_colors().count(sig.name) != 0); + gtk_widget_set_tooltip_text(recolor, "back to the palette colour for this signal's position"); + g_object_set_data(G_OBJECT(recolor), "udpscope-signal", GSIZE_TO_POINTER(i)); + g_signal_connect(recolor, "clicked", + G_CALLBACK(+[](GtkButton* b, gpointer user) { + App* a = static_cast(user); + const std::size_t idx = static_cast( + GPOINTER_TO_SIZE(g_object_get_data(G_OBJECT(b), "udpscope-signal"))); + std::string name; + { + std::lock_guard lock(a->model->mutex()); + const std::vector& sigs = a->model->signals(); + if (idx >= sigs.size()) return; + name = sigs[idx].name; + } + a->rows->clear_signal_color(name); + a->relayout_pending = true; + rebuild_signal_panels(*a); + schedule_layout_save(*a); + }), + &app); + gtk_box_append(GTK_BOX(menu_box), recolor); + GtkWidget* cal = menu_button("Calibrate...", nullptr); gtk_widget_set_tooltip_text(cal, "gain and offset, applied at ingest"); g_object_set_data(G_OBJECT(cal), "udpscope-signal", GSIZE_TO_POINTER(i)); @@ -2365,8 +2513,9 @@ GtkWidget* measures_row(App& app, std::size_t index, const Measure& m) { } } gtk_label_set_markup(GTK_LABEL(sig_chip), - chip_markup(color_index >= 0 ? RowSet::palette_color(color_index) - : RowSet::unplotted_color()) + chip_markup(color_index >= 0 + ? app.rows->signal_color(m.signal, color_index) + : RowSet::unplotted_color()) .c_str()); } gtk_widget_set_tooltip_text(sig_chip, m.signal.c_str()); @@ -2597,7 +2746,8 @@ void rebuild_cursors_pane(App& app) { // what "the pane shows a lot of empty signals that do not exist" was. if (row.signal_name[si].empty() || row.channel[si] < 0) continue; app.cursors_lines.push_back(cursors_line( - app, row.name[si].c_str(), RowSet::palette_color(row.color_index[si]), true)); + app, row.name[si].c_str(), + app.rows->signal_color(row.signal_name[si], row.color_index[si]), true)); app.cursors_lines.back().row = r; app.cursors_lines.back().slot = s; } @@ -9185,6 +9335,100 @@ gboolean menus_probe_step(gpointer user) { return G_SOURCE_REMOVE; } +/// Checks that a chosen trace colour reaches the **trace** and not merely the resolver. +/// +/// It reads the colour back out of `plot::Plot::trace()` -- what the renderer will stroke -- and out +/// of the signal list's chip markup, because the chip's only job is to name a trace and the two +/// resolving a colour by separate routes is the one failure a chip cannot survive. The override is +/// `#123456`, deliberately in no palette slot: an override that happened to equal the entry it +/// replaced would pass against an implementation that ignored it entirely. +gboolean color_probe(gpointer user) { + App* a = static_cast(user); + + // Any slot with a *bound* trace: an unbound one has no colour on screen to check. + int probe_row = -1; + int probe_slot = -1; + std::string name; + for (int r = 0; r < a->rows->row_count() && probe_row < 0; ++r) { + const RowSet::Row& row = a->rows->row(r); + for (int s = 0; s < RowSet::kSlotsPerRow; ++s) { + const std::size_t si = static_cast(s); + if (row.signal_name[si].empty() || row.channel[si] < 0) continue; + probe_row = r; + probe_slot = s; + name = row.signal_name[si]; + break; + } + } + if (probe_row < 0) { + std::fprintf(stderr, "[color] no bound trace to recolour\n"); + std::fflush(stderr); + return G_SOURCE_REMOVE; + } + + // The chip as the signal list actually writes it, and the trace as the renderer will read it. + const auto chip_of = [&]() { + std::lock_guard lock(a->model->mutex()); + const std::vector& signals = a->model->signals(); + for (std::size_t i = 0; i < signals.size(); ++i) { + if (signals[i].name == name) return signal_chip_markup(*a, signals, i); + } + return std::string(); + }; + const auto trace_of = [&]() { + const plot::Plot* p = a->rows->row(probe_row).plot; + if (p == nullptr) return std::string("#??????"); + return color_hex(p->trace(static_cast(probe_slot)).color); + }; + // A chip carries its colour inside ``, so "the chip says X" is the + // substring test and not a recomputation of what it ought to say. + const auto chip_says = [](const std::string& markup, const std::string& hex) { + return markup.find("\"" + hex + "\"") != std::string::npos ? 1 : 0; + }; + + const std::string before = trace_of(); + std::fprintf(stderr, "[color] '%s' at row %d slot %d: trace=%s chip_agrees=%d (want 1)\n", + name.c_str(), probe_row, probe_slot, before.c_str(), + chip_says(chip_of(), before)); + + constexpr plot::u32 kPick = 0x123456u; + a->rows->set_signal_color(name, kPick); + // The same path the frame tick takes: a trace's colour is fixed at `add_trace()`, so this is a + // new figure, and reading the old `Row::plot` after it would read the figure that was replaced. + relayout_areas(*a); + const std::string after = trace_of(); + std::fprintf(stderr, "[color] override: trace=%s chip_agrees=%d (want #123456 1)\n", + after.c_str(), chip_says(chip_of(), after)); + + // The session file, which is where a colour has to survive to be worth choosing. Through the real + // writer and reader on a temp path, not the live session's -- a probe must not leave a colour in + // the file a later run would inherit. + const std::string tmp = std::string(g_get_tmp_dir()) + "/udpscope-color-probe.toml"; + LayoutState saved = collect_layout(*a); + int round_trip = -1; + std::size_t entries = 0; + if (save_layout(tmp, saved)) { + LayoutState back; + if (load_layout(tmp, back)) { + entries = back.colors.size(); + for (const auto& c : back.colors) { + if (c.name == name) round_trip = static_cast(c.rgb); + } + } + } + std::remove(tmp.c_str()); + std::fprintf(stderr, "[color] session: entries=%zu rgb=#%06X (want 1 #123456)\n", entries, + round_trip < 0 ? 0u : static_cast(round_trip)); + + a->rows->clear_signal_color(name); + relayout_areas(*a); + const std::string restored = trace_of(); + std::fprintf(stderr, "[color] default: trace=%s chip_agrees=%d (want %s 1)\n", restored.c_str(), + chip_says(chip_of(), restored), before.c_str()); + std::fflush(stderr); + return G_SOURCE_REMOVE; +} + /// Fires a representative accel from each group and reports the state it changed, then /// checks the GtkEditable refusal. It goes through run_app_action() for the actions and /// through the real handler for the refusal, because the refusal lives in the handler. @@ -9391,6 +9635,9 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit if (const char* env = g_getenv("UDPSCOPE_PROBE_KEYS_MS")) { g_timeout_add(static_cast(std::atoi(env)), keys_probe, &app); } + if (const char* env = g_getenv("UDPSCOPE_PROBE_COLOR_MS")) { + g_timeout_add(static_cast(std::atoi(env)), color_probe, &app); + } // Our own options were consumed by parse_args(); GTK would reject them, so only // the program name is handed to g_application_run().