From 1135289309555a5b3f6a5108f2b109450530d3ba Mon Sep 17 00:00:00 2001 From: Manupa Wickramasinghe <73810867+manupawickramasinghe@users.noreply.github.com> Date: Fri, 31 Jul 2026 19:14:50 +0530 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=93=9D=20Record=20what=20running=20th?= =?UTF-8?q?e=20release=20build=20on=20Windows=20found?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three "untested on Windows" entries existed because nobody had the machine. Running the release binaries for the first time retired one of them and turned up three reproducible defects, none of them fixed here: - every release build demands elevation, including the headless one meant for servers and containers, because the manifest is gated on the profile alone rather than on the gui feature. A non-elevated parent gets ERROR_ELEVATION_REQUIRED with no UAC prompt available - --help, --version and every clap error print nothing, because clap handles them inside Cli::parse() and attach_console() is not called until cli::run(). A mistyped subcommand fails in silence - kill without --force can never succeed, since sysinfo supports no signal but Kill on Windows. This also breaks the Task Manager's default "End process" button And the reason CI stayed green through all three: the headless smoke test runs a debug build, and nothing on any platform ever executes a release binary. What was verified working is listed too, so it is not re-investigated: AttachConsole itself, the windows_subsystem gating, exit codes, stdout/stderr separation, redirection, and the TUI's panic-hook terminal restoration under panic = "abort". The ring/C-compiler entry is settled rather than removed: it builds fine with MSVC, so the requirement moves to the README as a prerequisite. Co-Authored-By: Claude Opus 5 --- README.md | 8 +++- TODO.md | 130 ++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 117 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 4e7cdd92..4e2e63cb 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,6 @@ controller — the field renders `—` rather than a plausible-looking guess. - **Hex Viewer** for raw firmware blobs (ACPI/SMBIOS on Windows and Linux) - Configurable poll interval, min/max reset, light/dark/grey themes -### Task Manager - **Processes** — PID, name, owner, CPU %, memory, virtual size, disk read/write and uptime; sortable columns, a filter box, and end/force-kill behind a confirmation. Rows are virtualised, so a 700-process list scrolls @@ -263,7 +262,12 @@ cargo run --release Platform prerequisites: ```bash -# Windows — also build the sensor sidecar (needs .NET 8 SDK) +# Windows — MSVC Build Tools are required, not optional: the default feature set +# includes `push`, which pulls in rustls → ring, and ring compiles C and assembly. +# Build from a developer prompt (vcvars64.bat) so cl.exe and the Windows SDK are +# on PATH. Without a C toolchain, use --no-default-features --features gui,web,tui. +# +# Also build the sensor sidecar (needs .NET 8 SDK): dotnet publish sidecar -c Release -o sidecar/publish # Linux diff --git a/TODO.md b/TODO.md index 315e6014..3e046d47 100644 --- a/TODO.md +++ b/TODO.md @@ -6,25 +6,116 @@ that should eventually change. ## Build -### `ring` needs a C compiler (debug on Windows) +### `ring` needs a C compiler Enabling `--features push` pulls in `rustls` → `ring`, which compiles C and -assembly rather than being pure Rust. GitHub's runners all ship a toolchain and -CI is green on all three platforms, but this is a **new build requirement** for -anyone compiling that feature locally — notably MSVC on Windows. +assembly rather than being pure Rust. This is a **build requirement** for anyone +compiling the default feature set, notably MSVC on Windows. -Two known consequences: +Verified on Windows 11 with VS 2022 (MSVC 14.36.32532) + Windows SDK +10.0.18362: `cargo build --release` compiles `ring` 0.17.14 and `rustls` 0.23.43 +with no workaround beyond running inside the MSVC environment. So the +requirement is real but ordinary — it is now stated in the README's "Build from +source" section rather than left implicit here. -- Cross-compiling to `x86_64-pc-windows-msvc` from macOS fails at `ring`'s build - script for exactly this reason. Windows-target checks have to skip `push`: +What remains open: + +- Cross-compiling to `x86_64-pc-windows-msvc` from macOS still fails at `ring`'s + build script. Windows-target checks have to skip `push`: `cargo clippy --target x86_64-pc-windows-msvc --no-default-features --features gui,web,tui` -- A contributor on Windows without Build Tools installed will hit this on a - default `cargo build`, since `push` is in the default feature set. +- Worth investigating: whether `rustls` can be pointed at a pure-Rust crypto + provider (`aws-lc-rs` has the same problem; `rustls` supports pluggable + providers) so `push` needs no C toolchain at all. + +## Confirmed Windows bugs + +Found by running the **release** binaries on Windows 11 for the first time +(2026-07-31), which is also what retired the old "Windows CLI console handling +is untested" entry. Three reproducible defects, plus the reason CI missed all of +them. None are fixed yet. + +What *does* work, so it is not re-investigated: `AttachConsole` itself (a +GUI-subsystem binary's subcommand output does reach a real console), the +feature-gated `windows_subsystem` attribute (GUI subsystem only with `gui` + +release, console otherwise), exit codes (0/1/2 as documented), stdout/stderr +separation, redirection and piping, and the TUI's panic-hook terminal +restoration under `panic = "abort"`. + +### `kill` without `--force` can never succeed on Windows + +`procs::kill` maps `force = false` to `Signal::Term`, and `sysinfo` supports no +signal but `Kill` on Windows, so the call always returns: + +``` +sensorview: Term is not supported on this platform +``` + +Confirmed against a disposable child process: it survived the plain `kill` and +died on `kill --force`. + +This also breaks the GUI. The Task Manager's confirmation modal passes +`force = false` for its default **"End process"** button — described in the +dialog as *"SIGTERM — asks the process to exit"* — so on Windows that button can +only ever report an error. Only "Force kill" works. The error is surfaced rather +than swallowed, so this is a capability gap, not a reporting bug. + +Fix: on Windows either fall back to `Signal::Kill` with the wording changed to +match, or disable the non-force action rather than offering something the +platform cannot do. + +### Every release build demands elevation, including the headless one + +`build.rs` gates the `requireAdministrator` manifest on `PROFILE == "release"` +alone, not on the `gui` feature. So the headless binary — the one the README +offers "for servers and containers" — also requires admin. A non-elevated +parent cannot start it at all: + +``` +CreateProcess FAILED: The requested operation requires elevation (error 740) +``` + +With `UseShellExecute = false` there is no UAC prompt to accept: the process +simply never starts, so a service, scheduled task, container entrypoint or CI +step running as a normal user fails outright — and with it every documented +scripting use of the CLI, since nothing can be piped or redirected from a +process that never ran. + +Scope of the claim, measured: this is specific to **non-interactive parents**. +An interactive `ShellExecute` launch (double-click, a shortcut, `Start-Process` +without `-NoNewWindow`) would raise a UAC prompt and succeed if the user +consents — that path was not tested here. The failure case is the +server/container/CI one, which is exactly what the headless build exists for. + +Fix: gate the manifest on the `gui` feature as well as the profile. An +unelevated CLI should degrade to reading fewer sensors — which is what +`lhm_bridge.rs` already documents — not refuse to launch. + +### `--help`, `--version` and every clap error print nothing + +In the shipped GUI-subsystem build these produce **zero** console output, while +the same commands on a console-subsystem build print normally. A mistyped +subcommand fails in complete silence — clap's "a similar subcommand exists" +hint is never seen. + +Cause: `main()` calls `Cli::parse()`, and clap handles `--help`, `--version` and +all parse errors *inside* that call, printing and exiting there. But +`attach_console()` is not called until `cli::run()`, which is only reached after +parsing succeeds. Everything clap emits goes to a process with no console yet. + +Fix: call `attach_console()` at the top of `main()`, before `Cli::parse()`. It +is already a no-op when there is no parent console and on non-Windows builds, so +moving it earlier costs nothing. + +### CI cannot catch any of the above — it never runs a release binary + +The headless smoke test builds and runs `target/debug/sensorview`, which is +`asInvoker` and console-subsystem, so it passes while telling us nothing about +the artifact that ships. The `Build (release)` step only compiles; nothing ever +executes a release binary on any platform. That is precisely why all three bugs +above survived a green CI. -Worth investigating: whether `rustls` can be pointed at a pure-Rust crypto -provider (`aws-lc-rs` has the same problem; `rustls` supports pluggable -providers) so `push` needs no C toolchain at all. Until then, document it in the -build instructions. +Worth adding: a step that runs the *release* binary's `--help` and asserts it +produces output and exits 0. ## Task Manager @@ -51,12 +142,13 @@ missed: - **macOS fan sensors are not implemented.** Fans would come from the same HID sensor plane as temperatures, on a different usage page, but development happened on a fanless MacBook Air with nothing to read or verify against. -- **Windows CLI console handling is untested.** The `AttachConsole` path and the - feature-gated `windows_subsystem` attribute compile and are exercised by CI, - but nobody has run `sensorview.exe get ...` from a real `cmd.exe`. -- **The TUI has never run on a real terminal.** `sensorview top` is covered by - ratatui's `TestBackend` (layout, values, filtering), but the keystroke handling - and the panic-hook terminal restoration have only been reasoned about. +- **The TUI is only partly verified.** `sensorview top` has now been run on a + real Windows console — rendering, `q`/`Esc`/`Ctrl-C`, and panic-hook terminal + restoration under `panic = "abort"` all check out (see below). Two things + still have not been observed: whether `r` actually resets min/max (no visible + sensor had a spread to reset at the time), and behaviour under legacy + `conhost` rather than the default terminal. It has also never run on a Linux + or macOS terminal. - **The Linux GUI has never been rendered.** It compiles and its tests pass in CI, but the build server is headless, so every window has only ever been *looked at* on macOS. Fonts, DPI scaling and the Task Manager's table layout From 16962c7e5384620519507c5f3a1367975835b3fe Mon Sep 17 00:00:00 2001 From: Manupa Wickramasinghe <73810867+manupawickramasinghe@users.noreply.github.com> Date: Fri, 31 Jul 2026 19:15:23 +0530 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=92=84=20Rebuild=20the=20Task=20Manag?= =?UTF-8?q?er=20after=20the=20Windows=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit More colour, and the layout people already know. Processes now leads with Name and follows Windows' column order, with the measured columns heat-shaded — the tint strengthens and shifts amber to red with the value, so the processes worth looking at are found by glancing rather than by reading. Each header carries its machine-wide total. The Disk column needed real data, not just a cell: sysinfo only exposes cumulative counters, so the collector keeps the previous tick's values and differences them against the wall clock actually elapsed rather than the nominal interval, which would overstate every rate on a slow machine. The rate is None until a baseline exists, exactly as cpu_pct is, and pid reuse is handled with saturating_sub so a recycled pid cannot report an absurd rate. CPU % in the table is now a share of the whole machine, which is what the column means to anyone reading it — 4 saturated threads of 16 reads 25 %, not 400 %. The per-thread-summed figure is in the tooltip and is still what `sensorview ps` prints; the filter bar was showing the summed number beside the header's share, which read as a contradiction, and now agrees. Performance gives each device class its own hue — CPU blue, memory purple, disk green, network rose, GPU teal — used for its card, thumbnail and graph, so colour identifies the device rather than decorating it. Graphs fill to the baseline over a fixed grid, and utilisation is pinned to 0-100 % so an idle machine reads as idle instead of auto-scaling noise to full height. Sidebar titles are disambiguated in a post-pass, because the rule depends on a tally no single node knows while the tree is walked. This machine has two RAM-class nodes and six NICs, which rendered as two cards labelled "Memory" and six labelled "Network" — the one thing the sidebar must never do. Distinct device names win over an index ("Wi-Fi 2" beats "Network 5", and is what Windows shows); numbering stays the fallback for genuinely identical devices such as two disks reporting the same model string. format_rate now prints <0.1 MB/s rather than 0 MB/s for small but real I/O: rounding a measured value to zero contradicts this module's rule that 0 means idle and — means unknown. Verified on screen in both the Black and Light palettes. Co-Authored-By: Claude Opus 5 --- README.md | 29 +- app/src/cli/procs.rs | 1 + app/src/procs/mod.rs | 116 +++++- app/src/ui/taskmgr_window.rs | 752 +++++++++++++++++++++++++++-------- 4 files changed, 724 insertions(+), 174 deletions(-) diff --git a/README.md b/README.md index 4e2e63cb..abc66f81 100644 --- a/README.md +++ b/README.md @@ -73,14 +73,27 @@ controller — the field renders `—` rather than a plausible-looking guess. - **Hex Viewer** for raw firmware blobs (ACPI/SMBIOS on Windows and Linux) - Configurable poll interval, min/max reset, light/dark/grey themes -- **Processes** — PID, name, owner, CPU %, memory, virtual size, disk - read/write and uptime; sortable columns, a filter box, and end/force-kill - behind a confirmation. Rows are virtualised, so a 700-process list scrolls - without cost -- **Performance** — a sidebar of category rows (CPU, GPU, Memory, Disk *n*, - Network *n*), each with a mini sparkline and its current value, and the - selected category filling the pane with a large graph and a stats grid. It - reads the telemetry that is already being collected, so it adds no polling +Laid out after the Windows Task Manager, because that is the layout people +already know. + +- **Processes** — name, CPU, memory, disk rate, PID and owner. The measured + columns are **heat-shaded**: the tint strengthens and shifts from amber to red + with the value, so the processes worth looking at are found by glancing rather + than reading. Each column header carries its machine-wide total. Sortable + columns, a filter box, and end/force-kill behind a confirmation. Rows are + virtualised, so a 700-process list scrolls without cost +- **Performance** — a sidebar of device cards (CPU, GPU, Memory, Disk *n*, + Network *n*), each with a live filled thumbnail and its current value, and the + selected device filling the pane with a large filled area graph over a fixed + grid. Every device class has its own colour — CPU blue, memory purple, disk + green, network rose, GPU teal — used for its card, its thumbnail and its + graph, so colour identifies the device rather than just decorating. Utilisation + charts are pinned to 0–100 % so an idle machine reads as idle. It reads the + telemetry that is already being collected, so it adds no polling + +CPU percentages in the **Processes** table are a share of the whole machine, as +Windows reports them: a process saturating 4 of 16 threads reads 25 %. Hovering +gives the per-thread-summed figure (400 %), which is what `sensorview ps` prints. - **The same data from the terminal** — `sensorview ps` and `sensorview kill`, which is what you want over SSH on a box with no display - The process collector runs **only while the window is open** — enumerating diff --git a/app/src/cli/procs.rs b/app/src/cli/procs.rs index fa378863..c18f49d1 100644 --- a/app/src/cli/procs.rs +++ b/app/src/cli/procs.rs @@ -140,6 +140,7 @@ mod tests { virt_bytes: 20 * 1024 * 1024, disk_read_bytes: 0, disk_write_bytes: 0, + disk_bps: None, run_time_s: 1, } } diff --git a/app/src/procs/mod.rs b/app/src/procs/mod.rs index 5b00188a..916a8015 100644 --- a/app/src/procs/mod.rs +++ b/app/src/procs/mod.rs @@ -20,10 +20,11 @@ //! `crate::sysinfo` module (static machine facts). Inside this module the crate //! is always spelled `::sysinfo` so which one is meant is never ambiguous. +use std::collections::HashMap; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::thread::JoinHandle; -use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use arc_swap::ArcSwap; @@ -57,6 +58,10 @@ pub struct ProcessRow { /// "not permitted to look" — see the note in `build_snapshot`. pub disk_read_bytes: u64, pub disk_write_bytes: u64, + /// Combined read+write bytes per second. `None` until a baseline exists: + /// the underlying counters are cumulative, so a rate needs two samples — + /// exactly the constraint `cpu_pct` has, and reported the same way. + pub disk_bps: Option, pub run_time_s: u64, } @@ -187,33 +192,55 @@ fn collect_loop(sink: &ArcSwap, running: &AtomicBool) { let mut seq: u64 = 0; + // Last tick's cumulative I/O counters, for turning them into a rate. + // Measured against the wall clock actually elapsed rather than + // REFRESH_INTERVAL, because a refresh over several hundred processes can + // take a sizeable fraction of the interval — assuming the nominal period + // would overstate every rate on a slow machine. + let mut prev_io: HashMap = HashMap::new(); + let mut last_tick = Instant::now(); + while running.load(Ordering::Relaxed) { // `true` removes processes that have exited since the last refresh. sys.refresh_processes_specifics(ProcessesToUpdate::All, true, what); seq += 1; + let now = Instant::now(); + let dt_s = now.duration_since(last_tick).as_secs_f64(); + last_tick = now; + // Occasionally, in case a user appeared mid-session. if seq.is_multiple_of(60) { users = Users::new_with_refreshed_list(); } - sink.store(Arc::new(build_snapshot(&sys, &users, seq, cpu_count))); + let (snapshot, next_io) = + build_snapshot(&sys, &users, seq, cpu_count, &prev_io, dt_s); + prev_io = next_io; + sink.store(Arc::new(snapshot)); // Sliced so stopping doesn't wait out a full interval. sleep_interruptible(running, REFRESH_INTERVAL); } } +/// Build a snapshot, and return the I/O counters to difference against next +/// tick. fn build_snapshot( sys: &::sysinfo::System, users: &::sysinfo::Users, seq: u64, cpu_count: usize, -) -> ProcessSnapshot { + prev_io: &HashMap, + dt_s: f64, +) -> (ProcessSnapshot, HashMap) { // CPU percentages are meaningless until sysinfo has two samples to // difference, so the first snapshot reports them absent rather than 0. let cpu_ready = seq >= 2; + // Same rule for I/O rates, plus a positive interval to divide by. + let io_ready = cpu_ready && dt_s > 0.0; let mut total_cpu_pct = 0.0f32; + let mut next_io = HashMap::with_capacity(sys.processes().len()); let rows: Vec = sys .processes() @@ -229,8 +256,23 @@ fn build_snapshot( // the more common case, so 0 is reported as 0 and the caveat is // documented rather than guessed at. let disk = p.disk_usage(); + let pid_u32 = pid.as_u32(); + let cumulative = (disk.total_read_bytes, disk.total_written_bytes); + // `saturating_sub` because pids are recycled: a new process + // inheriting a dead one's pid starts its counters at zero, and an + // unsigned wrap would report an absurd rate rather than none. + let disk_bps = io_ready.then(|| { + prev_io.get(&pid_u32).map(|(pr, pw)| { + let moved = cumulative.0.saturating_sub(*pr) + + cumulative.1.saturating_sub(*pw); + moved as f64 / dt_s + }) + }) + .flatten(); + next_io.insert(pid_u32, cumulative); + ProcessRow { - pid: pid.as_u32(), + pid: pid_u32, parent: p.parent().map(|p| p.as_u32()), name: p.name().to_string_lossy().into_owned(), cmd: p @@ -246,14 +288,15 @@ fn build_snapshot( cpu_pct: cpu_ready.then_some(cpu), mem_bytes: p.memory(), virt_bytes: p.virtual_memory(), - disk_read_bytes: disk.total_read_bytes, - disk_write_bytes: disk.total_written_bytes, + disk_read_bytes: cumulative.0, + disk_write_bytes: cumulative.1, + disk_bps, run_time_s: p.run_time(), } }) .collect(); - ProcessSnapshot { + let snapshot = ProcessSnapshot { seq, unix_ms: SystemTime::now() .duration_since(UNIX_EPOCH) @@ -262,7 +305,8 @@ fn build_snapshot( rows, total_cpu_pct, cpu_count, - } + }; + (snapshot, next_io) } fn sleep_interruptible(running: &AtomicBool, total: Duration) { @@ -281,6 +325,7 @@ pub enum Sort { #[default] Cpu, Memory, + Disk, Pid, Name, } @@ -317,6 +362,11 @@ pub fn arrange( .unwrap_or(f32::NEG_INFINITY) .total_cmp(&b.cpu_pct.unwrap_or(f32::NEG_INFINITY)), Sort::Memory => a.mem_bytes.cmp(&b.mem_bytes), + // Unread I/O sorts below idle, for the same reason as CPU above. + Sort::Disk => a + .disk_bps + .unwrap_or(f64::NEG_INFINITY) + .total_cmp(&b.disk_bps.unwrap_or(f64::NEG_INFINITY)), Sort::Pid => a.pid.cmp(&b.pid), Sort::Name => a.name.to_lowercase().cmp(&b.name.to_lowercase()), }; @@ -356,6 +406,30 @@ pub fn kill(pid: u32, force: bool) -> Result<(), String> { } } +/// Human-readable transfer rate, the way a task manager shows a Disk column. +/// +/// `None` is "no reading yet" and prints as `—`, keeping the distinction the +/// rest of this module makes between *unknown* and *idle*. +/// +/// A known rate below 0.1 MB/s prints as `<0.1 MB/s`, not as `0 MB/s`: at one +/// decimal place a process doing steady small writes would otherwise be +/// indistinguishable from one doing nothing at all, and this column exists to +/// tell those apart. Exactly zero still prints `0 MB/s`. +pub fn format_rate(bps: Option) -> String { + match bps { + None => "—".to_string(), + Some(v) if v <= 0.0 => "0 MB/s".to_string(), + Some(v) => { + let mb = v / (1024.0 * 1024.0); + if mb >= 0.1 { + format!("{mb:.1} MB/s") + } else { + "<0.1 MB/s".to_string() + } + } + } +} + /// Human-readable byte size. pub fn format_bytes(bytes: u64) -> String { const KB: f64 = 1024.0; @@ -387,6 +461,7 @@ mod tests { virt_bytes: mem * 4, disk_read_bytes: 0, disk_write_bytes: 0, + disk_bps: None, run_time_s: 10, } } @@ -440,6 +515,31 @@ mod tests { assert!(arrange(&sample(), Some("nothing-here"), Sort::Pid, false).is_empty()); } + /// A disk rate that has no baseline yet must read as unknown, not as 0 — + /// the same distinction `cpu_pct` makes, and for the same reason. + #[test] + fn unread_disk_rate_sorts_last_and_prints_as_unknown() { + assert_eq!(format_rate(None), "—"); + // Measured as genuinely idle. + assert_eq!(format_rate(Some(0.0)), "0 MB/s"); + // Small but real: must not be flattened into "0 MB/s", which is the + // whole point of having the column. + assert_eq!(format_rate(Some(1024.0)), "<0.1 MB/s"); + assert_eq!(format_rate(Some(5.0 * 1024.0 * 1024.0)), "5.0 MB/s"); + + let mut rows = sample(); + rows[0].disk_bps = Some(4.0 * 1024.0 * 1024.0); + rows[1].disk_bps = Some(0.0); + // rows[2] keeps `None`. + let sorted = arrange(&rows, None, Sort::Disk, true); + assert_eq!(sorted[0].pid, 3, "busiest disk user must sort first"); + assert_eq!( + sorted.last().unwrap().pid, + 2, + "an unread rate must sort below a known-idle one" + ); + } + #[test] fn byte_formatting_picks_sensible_units() { assert_eq!(format_bytes(512), "512 B"); diff --git a/app/src/ui/taskmgr_window.rs b/app/src/ui/taskmgr_window.rs index cee05984..1d813a36 100644 --- a/app/src/ui/taskmgr_window.rs +++ b/app/src/ui/taskmgr_window.rs @@ -178,16 +178,20 @@ fn processes_tab(ui: &mut egui::Ui, s: &Shared, pal: &Palette, state: &mut State state.filter.clear(); } ui.separator(); + // Shown as a share of the machine, matching the CPU column + // header — the same number twice in two different units read + // as a contradiction. let cpu = if snapshot.has_cpu() { - format!("{:.0} %", snapshot.total_cpu_pct) + format!("{:.0} %", snapshot.total_cpu_pct / snapshot.cpu_count.max(1) as f32) } else { // Not zero: the baseline simply doesn't exist yet. "—".to_string() }; ui.label( RichText::new(format!( - "{} processes · total CPU {cpu}", - snapshot.rows.len() + "{} processes · CPU {cpu} of {} logical CPUs", + snapshot.rows.len(), + snapshot.cpu_count )) .size(11.0) .color(pal.text_dim), @@ -211,53 +215,67 @@ fn processes_tab(ui: &mut egui::Ui, s: &Shared, pal: &Palette, state: &mut State return; } - let row_h = 18.0; + // Every heat column is scaled to the busiest process on show, which + // makes the shading a *ranking* rather than a measurement — the + // exact figure is in the cell, and the tint is only there to draw + // the eye to it. + // + // CPU gets a floor as well. Scaling it against a full machine + // instead looks principled but renders the column colourless in + // practice: on 16 threads a busy process is ~6 % of the whole, so + // every cell would be the palest tint. The floor stops the other + // extreme — on a genuinely idle machine the top process is not + // painted scarlet for using 0.4 %. + let cpus = snapshot.cpu_count.max(1) as f32; + let peak_cpu = rows + .iter() + .filter_map(|r| r.cpu_pct) + .fold(0.0f32, f32::max) + .max(2.0 * cpus); + let peak_mem = rows.iter().map(|r| r.mem_bytes).max().unwrap_or(0) as f32; + let peak_disk = rows + .iter() + .filter_map(|r| r.disk_bps) + .fold(0.0f64, f64::max) as f32; + + let row_h = 20.0; // Virtualised: `body.rows` only builds what is on screen, which // matters at ~700 processes. TableBuilder::new(ui) .striped(true) .cell_layout(egui::Layout::left_to_right(egui::Align::Center)) - .column(Column::exact(64.0)) // pid - .column(Column::exact(64.0)) // cpu - .column(Column::exact(78.0)) // memory - .column(Column::exact(96.0)) // user .column(Column::remainder()) // name - .header(20.0, |mut header| { - for (label, sort) in [ - ("PID", Some(Sort::Pid)), - ("CPU %", Some(Sort::Cpu)), - ("Memory", Some(Sort::Memory)), - ("User", None), - ("Name", Some(Sort::Name)), + .column(Column::exact(72.0)) // cpu + .column(Column::exact(84.0)) // memory + .column(Column::exact(84.0)) // disk + .column(Column::exact(58.0)) // pid + .column(Column::exact(90.0)) // user + .header(34.0, |mut header| { + // Windows heads each measured column with the machine-wide + // total, so the columns double as a system summary. + let total_cpu = snapshot + .has_cpu() + .then(|| format!("{:.0}%", snapshot.total_cpu_pct / cpus)); + let total_disk = (peak_disk > 0.0).then(|| { + let sum: f64 = rows.iter().filter_map(|r| r.disk_bps).sum(); + procs::format_rate(Some(sum)) + }); + for (label, total, sort) in [ + ("Name", None, Some(Sort::Name)), + ("CPU", total_cpu, Some(Sort::Cpu)), + ("Memory", None, Some(Sort::Memory)), + ("Disk", total_disk, Some(Sort::Disk)), + ("PID", None, Some(Sort::Pid)), + ("User", None, None), ] { header.col(|ui| { - sort_header(ui, pal, state, label, sort); + sort_header(ui, pal, state, label, total.as_deref(), sort); }); } }) .body(|body| { body.rows(row_h, rows.len(), |mut row| { let r = &rows[row.index()]; - row.col(|ui| mono(ui, pal, &r.pid.to_string())); - row.col(|ui| { - // An unknown reading is an em dash, never 0.0 — - // "we can't see" and "idle" are different facts. - let (text, color) = match r.cpu_pct { - None => ("—".to_string(), pal.text_dim), - Some(v) => (format!("{v:.1}"), load_color(v, pal)), - }; - ui.label( - RichText::new(text).size(10.5).monospace().color(color), - ); - }); - row.col(|ui| mono(ui, pal, &procs::format_bytes(r.mem_bytes))); - row.col(|ui| { - ui.label( - RichText::new(r.user.as_deref().unwrap_or("—")) - .size(10.5) - .color(pal.text_dim), - ); - }); row.col(|ui| { let resp = ui .label(RichText::new(&r.name).size(11.0).color(pal.text)) @@ -285,12 +303,118 @@ fn processes_tab(ui: &mut egui::Ui, s: &Shared, pal: &Palette, state: &mut State } }); }); + row.col(|ui| { + // An unknown reading is an em dash, never 0.0 — + // "we can't see" and "idle" are different facts. + match r.cpu_pct { + None => { + heat_cell(ui, pal, "—", None); + } + // Shown as a share of the whole machine, which + // is what a task manager's CPU column means to + // a reader. `cpu_pct` itself is per-thread + // summed (1600 % on 16 threads); the raw figure + // is in the tooltip so nothing is lost. + Some(v) => { + let share = v / cpus; + heat_cell(ui, pal, &format!("{share:.1}%"), Some(v / peak_cpu)) + .on_hover_text(format!( + "{v:.1}% summed across {} logical CPUs", + snapshot.cpu_count + )); + } + } + }); + row.col(|ui| { + let t = (peak_mem > 0.0).then(|| r.mem_bytes as f32 / peak_mem); + heat_cell(ui, pal, &procs::format_bytes(r.mem_bytes), t); + }); + row.col(|ui| { + let t = r + .disk_bps + .filter(|_| peak_disk > 0.0) + .map(|v| v as f32 / peak_disk); + heat_cell(ui, pal, &procs::format_rate(r.disk_bps), t); + }); + row.col(|ui| mono(ui, pal, &r.pid.to_string())); + row.col(|ui| { + ui.label( + RichText::new(r.user.as_deref().unwrap_or("—")) + .size(10.5) + .color(pal.text_dim), + ); + }); }); }); }); } -fn sort_header(ui: &mut egui::Ui, pal: &Palette, state: &mut State, label: &str, sort: Option) { +/// A table cell shaded by how big its value is, the way Windows tints its +/// CPU/Memory/Disk columns. `intensity` is 0..=1, or `None` for "no reading", +/// which is left unshaded rather than shaded as if it were zero. +fn heat_cell( + ui: &mut egui::Ui, + pal: &Palette, + text: &str, + intensity: Option, +) -> egui::Response { + let t = intensity.unwrap_or(0.0).clamp(0.0, 1.0); + // Below this the tint is indistinguishable from the stripe and only makes + // the table look dirty, so idle rows stay clean. + if t > 0.02 { + let rect = ui.max_rect().expand2(egui::vec2(3.0, 0.0)); + ui.painter().rect_filled(rect, 1.0, heat_color(t, pal)); + } + let color = if intensity.is_none() { + pal.text_dim + } else if t > 0.55 { + // Dark enough underneath that the normal value colour stops reading. + pal.text + } else { + pal.value + }; + ui.label(RichText::new(text).size(10.5).monospace().color(color)) +} + +/// Amber → red as the value climbs, with the tint strengthening too, so both +/// hue and weight carry the signal. +fn heat_color(t: f32, pal: &Palette) -> Color32 { + let base = pal.warn.lerp_to_gamma(pal.crit, t); + base.gamma_multiply(0.16 + 0.5 * t) +} + +/// A column heading, optionally carrying the machine-wide total above it the +/// way Windows does ("CPU" with "23%" over it). +fn sort_header( + ui: &mut egui::Ui, + pal: &Palette, + state: &mut State, + label: &str, + total: Option<&str>, + sort: Option, +) { + // Vertical so the total can sit above the label; without this the header + // is a single baseline and there is nowhere to put it. The blank line + // keeps every heading the same height when a column has no total. + ui.vertical(|ui| { + ui.add_space(1.0); + ui.label( + RichText::new(total.unwrap_or(" ")) + .size(11.5) + .color(if total.is_some() { pal.text } else { pal.text_dim }), + ); + sort_label(ui, pal, state, label, sort); + }); +} + +/// The clickable label half of a column heading. +fn sort_label( + ui: &mut egui::Ui, + pal: &Palette, + state: &mut State, + label: &str, + sort: Option, +) { let Some(sort) = sort else { ui.label(RichText::new(label).size(10.5).strong().color(pal.text_dim)); return; @@ -324,16 +448,6 @@ fn mono(ui: &mut egui::Ui, pal: &Palette, text: &str) { ui.label(RichText::new(text).size(10.5).monospace().color(pal.value)); } -fn load_color(pct: f32, pal: &Palette) -> Color32 { - if pct >= 80.0 { - pal.crit - } else if pct >= 40.0 { - pal.warn - } else { - pal.value - } -} - /// Killing is destructive and irreversible, so it asks first. fn confirm_kill_modal(ui: &mut egui::Ui, pal: &Palette, state: &mut State) { let Some((pid, name, force)) = state.confirm_kill.clone() else { @@ -390,15 +504,49 @@ fn confirm_kill_modal(ui: &mut egui::Ui, pal: &Palette, state: &mut State) { /// One sidebar entry: a hardware node reduced to a headline sensor. struct Category { title: String, + /// The device's own name, kept separately from `title` so repeated roles + /// can be re-titled once the whole tree is known. See [`disambiguate`]. + device: String, subtitle: String, /// Sensor identifier whose history drives the sparkline and graph. identifier: String, value: Option, sensor_type: SensorType, + /// Kept rather than a resolved `Color32` so `build_categories` stays + /// palette-free and therefore testable without a UI context. + hardware_type: HardwareType, /// Sensors shown in the stats grid for this category. detail: Vec<(String, String)>, } +/// Windows gives each device class its own hue and uses it for everything that +/// device owns — sidebar sparkline, big graph, fill. That single choice is most +/// of what makes its Task Manager readable at a glance, so it is reproduced +/// here rather than painting every graph in one accent colour. +/// +/// These are our own approximations of that scheme, not sampled assets. +fn category_color(t: HardwareType, pal: &Palette) -> Color32 { + match t { + HardwareType::Cpu => Color32::from_rgb(0x1f, 0x7f, 0xc1), // blue + HardwareType::Ram => Color32::from_rgb(0x8b, 0x5f, 0xbf), // purple + HardwareType::Storage | HardwareType::Hdd => Color32::from_rgb(0x2e, 0x9e, 0x5b), // green + HardwareType::Network => Color32::from_rgb(0xc4, 0x51, 0x7a), // rose + HardwareType::GpuApple + | HardwareType::GpuAti + | HardwareType::GpuIntel + | HardwareType::GpuNvidia => Color32::from_rgb(0x1e, 0x9c, 0xa8), // teal + _ => pal.accent, + } +} + +/// A utilisation graph is pinned to 0-100 % the way Windows pins its CPU chart, +/// so a quiet machine reads as a low line rather than an auto-scaled one that +/// looks busy. Everything else keeps auto-scaling, because there is no +/// meaningful fixed ceiling for a clock or a temperature. +fn fixed_scale(t: SensorType) -> Option<(f32, f32)> { + matches!(t, SensorType::Load).then_some((0.0, 100.0)) +} + fn performance_tab(ui: &mut egui::Ui, s: &Shared, pal: &Palette, state: &mut State) { let frame = s.frame(); let categories = build_categories(&frame); @@ -431,27 +579,55 @@ fn performance_tab(ui: &mut egui::Ui, s: &Shared, pal: &Palette, state: &mut Sta let cat = &categories[state.selected_category]; let history = s.store.history(&cat.identifier); + let color = category_color(cat.hardware_type, pal); + egui::CentralPanel::default() - .frame(egui::Frame::new().fill(pal.bg).inner_margin(egui::Margin::same(10))) + .frame(egui::Frame::new().fill(pal.bg).inner_margin(egui::Margin::same(12))) .show(ui, |ui| { - ui.label(RichText::new(&cat.title).size(15.0).strong().color(pal.text)); - ui.label(RichText::new(&cat.subtitle).size(11.0).color(pal.text_dim)); + // Title left, device name right — Windows' header line. + ui.horizontal(|ui| { + ui.label(RichText::new(&cat.title).size(19.0).color(pal.text)); + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + ui.label(RichText::new(&cat.subtitle).size(11.5).color(pal.text_dim)); + }); + }); ui.add_space(6.0); - let graph_h = (ui.available_height() - 26.0 * cat.detail.len() as f32 - 20.0) - .clamp(120.0, 340.0); - let (rect, _) = - ui.allocate_exact_size(egui::vec2(ui.available_width(), graph_h), egui::Sense::hover()); - paint_graph(ui, rect, &history, super::widgets::type_color(cat.sensor_type, pal), pal); + let graph_h = (ui.available_height() - 22.0 * cat.detail.len().min(6) as f32 - 56.0) + .clamp(140.0, 360.0); + let (rect, _) = ui + .allocate_exact_size(egui::vec2(ui.available_width(), graph_h), egui::Sense::hover()); + paint_graph(ui, rect, &history, color, pal, cat.sensor_type); + + ui.add_space(10.0); + + // The headline reading, big and in the device's colour — the number + // Windows puts under the chart before the smaller stats. + ui.horizontal(|ui| { + ui.label( + RichText::new(crate::format::format_value(cat.value, cat.sensor_type)) + .size(26.0) + .color(color), + ); + ui.add_space(10.0); + ui.with_layout(egui::Layout::top_down(egui::Align::LEFT), |ui| { + ui.add_space(8.0); + ui.label(RichText::new(&cat.title).size(11.0).color(pal.text_dim)); + }); + }); ui.add_space(8.0); + + // Two columns of stats, as Windows lays out its detail block. egui::Grid::new("taskmgr_detail") - .num_columns(2) - .spacing([18.0, 3.0]) + .num_columns(4) + .spacing([22.0, 4.0]) .show(ui, |ui| { - for (k, v) in &cat.detail { - ui.label(RichText::new(k).size(11.0).color(pal.text_dim)); - ui.label(RichText::new(v).size(11.0).monospace().color(pal.value)); + for pair in cat.detail.chunks(2) { + for (k, v) in pair { + ui.label(RichText::new(k).size(11.0).color(pal.text_dim)); + ui.label(RichText::new(v).size(11.0).monospace().color(pal.value)); + } ui.end_row(); } }); @@ -466,90 +642,213 @@ fn category_row( cat: &Category, selected: bool, ) -> bool { + // Taller than a list row: Windows' sidebar entries are cards carrying a + // live thumbnail of the same graph, not one-line labels. let (rect, resp) = - ui.allocate_exact_size(egui::vec2(ui.available_width(), 46.0), egui::Sense::click()); + ui.allocate_exact_size(egui::vec2(ui.available_width(), 56.0), egui::Sense::click()); let p = ui.painter_at(rect); + let color = category_color(cat.hardware_type, pal); if selected { - p.rect_filled(rect, 2.0, pal.bg_header); - p.line_segment( - [ - Pos2::new(rect.left() + 1.0, rect.top() + 3.0), - Pos2::new(rect.left() + 1.0, rect.bottom() - 3.0), - ], - Stroke::new(2.0, pal.accent), + p.rect_filled(rect, 3.0, pal.bg_header); + // The selection bar takes the device's colour, so the sidebar says + // which device is selected twice over — position and hue. + p.rect_filled( + egui::Rect::from_min_max( + Pos2::new(rect.left(), rect.top() + 2.0), + Pos2::new(rect.left() + 3.0, rect.bottom() - 2.0), + ), + 1.5, + color, ); } else if resp.hovered() { - p.rect_filled(rect, 2.0, pal.row_odd); + p.rect_filled(rect, 3.0, pal.row_odd); } - p.text( - Pos2::new(rect.left() + 8.0, rect.top() + 6.0), - Align2::LEFT_TOP, - &cat.title, - FontId::proportional(11.5), + let text_x = rect.left() + 10.0; + // Device names are as long as their vendors made them ("OpenVPN Data + // Channel Offload"), so the title is elided to the space left of the + // thumbnail. Painting it unclipped ran it under the graph. + let title_w = (rect.right() - 86.0) - text_x; + let galley = { + let mut job = egui::text::LayoutJob::simple_singleline( + cat.title.clone(), + FontId::proportional(12.0), + if selected { pal.text } else { pal.text_dim }, + ); + job.wrap.max_width = title_w.max(24.0); + job.wrap.max_rows = 1; + job.wrap.break_anywhere = true; + p.layout_job(job) + }; + p.galley( + Pos2::new(text_x, rect.top() + 7.0), + galley, if selected { pal.text } else { pal.text_dim }, ); p.text( - Pos2::new(rect.left() + 8.0, rect.bottom() - 6.0), + Pos2::new(text_x, rect.bottom() - 8.0), Align2::LEFT_BOTTOM, crate::format::format_value(cat.value, cat.sensor_type), - FontId::monospace(10.5), - pal.value, + FontId::proportional(12.5), + color, ); - // Sparkline on the right half. - let spark = egui::Rect::from_min_max( - Pos2::new(rect.right() - 74.0, rect.top() + 8.0), - Pos2::new(rect.right() - 6.0, rect.bottom() - 8.0), + // Thumbnail graph on the right, filled like the big one. + let thumb = egui::Rect::from_min_max( + Pos2::new(rect.right() - 80.0, rect.top() + 8.0), + Pos2::new(rect.right() - 8.0, rect.bottom() - 8.0), ); + p.rect_filled(thumb, 1.0, pal.bg.gamma_multiply(0.6)); paint_sparkline( &p, - spark, + thumb, &s.store.history(&cat.identifier), - super::widgets::type_color(cat.sensor_type, pal), + color, + fixed_scale(cat.sensor_type), + ); + p.rect_stroke( + thumb, + 1.0, + Stroke::new(1.0, pal.grid.gamma_multiply(0.7)), + egui::StrokeKind::Inside, ); + // Two NICs can share a visible prefix once elided ("Local Area Connec…"), + // so the untruncated name has to be reachable without selecting the row. + let resp = resp.on_hover_text(&cat.subtitle); + resp.clicked() } -fn paint_sparkline(p: &egui::Painter, rect: egui::Rect, history: &[f32], line: Color32) { +fn paint_sparkline( + p: &egui::Painter, + rect: egui::Rect, + history: &[f32], + line: Color32, + fixed: Option<(f32, f32)>, +) { if history.len() < 2 { return; } - let (mut lo, mut hi) = (f32::INFINITY, f32::NEG_INFINITY); - for &v in history { - lo = lo.min(v); - hi = hi.max(v); - } - // A flat series would divide by zero; give it a band so it draws mid-height. - if (hi - lo).abs() < f32::EPSILON { - lo -= 1.0; - hi += 1.0; - } + let (lo, hi) = value_range(history, fixed); let n = history.len(); let pts: Vec = history .iter() .enumerate() .map(|(i, &v)| { let x = rect.left() + (i as f32 / (n - 1) as f32) * rect.width(); - let y = rect.bottom() - ((v - lo) / (hi - lo)) * rect.height(); - Pos2::new(x, y) + let t = ((v - lo) / (hi - lo)).clamp(0.0, 1.0); + Pos2::new(x, rect.bottom() - t * rect.height()) }) .collect(); - p.add(egui::Shape::line(pts, Stroke::new(1.0, line))); + fill_under(p, rect, &pts, line); + p.add(egui::Shape::line(pts, Stroke::new(1.2, line))); +} + +/// Work out the value range a series should be drawn against. +/// +/// Split out from painting so both the graph and its axis labels agree, and so +/// the flat-series case has one home instead of being re-derived per caller. +fn value_range(history: &[f32], fixed: Option<(f32, f32)>) -> (f32, f32) { + if let Some(range) = fixed { + return range; + } + let (mut lo, mut hi) = (f32::INFINITY, f32::NEG_INFINITY); + for &v in history { + lo = lo.min(v); + hi = hi.max(v); + } + if !lo.is_finite() || !hi.is_finite() { + return (0.0, 1.0); + } + // A flat series would divide by zero; give it a band so it draws mid-height. + if (hi - lo).abs() < f32::EPSILON { + return (lo - 1.0, hi + 1.0); + } + let pad = (hi - lo) * 0.08; + (lo - pad, hi + pad) } +/// The big graph, drawn the way Windows draws it: a fixed grid, the series +/// filled down to the baseline in the device's own colour, and the axis +/// described in words at the corners rather than with numbered ticks. fn paint_graph( ui: &egui::Ui, rect: egui::Rect, history: &[f32], line: Color32, pal: &Palette, + sensor_type: SensorType, ) { let p = ui.painter_at(rect); p.rect_filled(rect, 2.0, pal.row_odd); + // The grid is drawn whether or not there is data, so the panel has the + // same shape while the first samples are still arriving. + let cells = 10.0; + let grid = pal.grid.gamma_multiply(0.45); + for i in 1..cells as i32 { + let t = i as f32 / cells; + let x = rect.left() + t * rect.width(); + let y = rect.top() + t * rect.height(); + p.line_segment( + [Pos2::new(x, rect.top()), Pos2::new(x, rect.bottom())], + Stroke::new(1.0, grid), + ); + p.line_segment( + [Pos2::new(rect.left(), y), Pos2::new(rect.right(), y)], + Stroke::new(1.0, grid), + ); + } + p.rect_stroke( + rect, + 2.0, + Stroke::new(1.0, pal.grid), + egui::StrokeKind::Inside, + ); + + let fixed = fixed_scale(sensor_type); + let (lo, hi) = value_range(history, fixed); + + // Corner labels, as Windows captions its charts. + let caption = match sensor_type { + SensorType::Load => "% Utilisation", + SensorType::Temperature => "Temperature", + SensorType::Clock => "Clock", + SensorType::Throughput => "Throughput", + SensorType::Power => "Power", + _ => "Value", + }; + p.text( + Pos2::new(rect.left() + 6.0, rect.top() + 4.0), + Align2::LEFT_TOP, + caption, + FontId::proportional(10.5), + pal.text_dim, + ); + p.text( + Pos2::new(rect.right() - 6.0, rect.top() + 4.0), + Align2::RIGHT_TOP, + crate::format::format_value(Some(hi), sensor_type), + FontId::proportional(10.5), + pal.text_dim, + ); + p.text( + Pos2::new(rect.left() + 6.0, rect.bottom() - 4.0), + Align2::LEFT_BOTTOM, + "60 seconds", + FontId::proportional(10.5), + pal.text_dim, + ); + p.text( + Pos2::new(rect.right() - 6.0, rect.bottom() - 4.0), + Align2::RIGHT_BOTTOM, + crate::format::format_value(Some(lo), sensor_type), + FontId::proportional(10.5), + pal.text_dim, + ); + if history.len() < 2 { p.text( rect.center(), @@ -561,46 +860,46 @@ fn paint_graph( return; } - let (mut lo, mut hi) = (f32::INFINITY, f32::NEG_INFINITY); - for &v in history { - lo = lo.min(v); - hi = hi.max(v); - } - if (hi - lo).abs() < f32::EPSILON { - hi = lo + 1.0; - lo -= 1.0; - } - let pad = (hi - lo) * 0.08; - lo -= pad; - hi += pad; - - for i in 0..=4 { - let t = i as f32 / 4.0; - let y = rect.bottom() - t * rect.height(); - p.line_segment( - [Pos2::new(rect.left(), y), Pos2::new(rect.right(), y)], - Stroke::new(1.0, pal.grid.gamma_multiply(0.5)), - ); - p.text( - Pos2::new(rect.left() + 4.0, y - 1.0), - Align2::LEFT_BOTTOM, - format!("{:.1}", lo + t * (hi - lo)), - FontId::monospace(9.0), - pal.text_dim, - ); - } - let n = history.len(); let pts: Vec = history .iter() .enumerate() .map(|(i, &v)| { let x = rect.left() + (i as f32 / (n - 1) as f32) * rect.width(); - let y = rect.bottom() - ((v - lo) / (hi - lo)) * rect.height(); - Pos2::new(x, y) + let t = ((v - lo) / (hi - lo)).clamp(0.0, 1.0); + Pos2::new(x, rect.bottom() - t * rect.height()) }) .collect(); - p.add(egui::Shape::line(pts, Stroke::new(1.5, line))); + + fill_under(&p, rect, &pts, line); + p.add(egui::Shape::line(pts, Stroke::new(1.6, line))); +} + +/// Fill the area between a series and the baseline. +/// +/// Emitted as a quad per sample pair rather than one polygon: egui's polygon +/// fill wants a convex path, and a utilisation trace is anything but. +fn fill_under(p: &egui::Painter, rect: egui::Rect, pts: &[Pos2], color: Color32) { + if pts.len() < 2 { + return; + } + let fill = color.gamma_multiply(0.28); + let mut mesh = egui::Mesh::default(); + for pair in pts.windows(2) { + let (a, b) = (pair[0], pair[1]); + let base = mesh.vertices.len() as u32; + for pos in [ + a, + b, + Pos2::new(b.x, rect.bottom()), + Pos2::new(a.x, rect.bottom()), + ] { + mesh.colored_vertex(pos, fill); + } + mesh.add_triangle(base, base + 1, base + 2); + mesh.add_triangle(base, base + 2, base + 3); + } + p.add(egui::Shape::mesh(mesh)); } /// Reduce the hardware tree to sidebar categories. @@ -611,12 +910,13 @@ fn paint_graph( fn build_categories(frame: &TelemetryFrame) -> Vec { let mut out = Vec::new(); walk(&frame.tree, &mut out); + disambiguate(&mut out); out } fn walk(tree: &[Hardware], out: &mut Vec) { for hw in tree { - if let Some(cat) = category_for(hw, out) { + if let Some(cat) = category_for(hw) { out.push(cat); } walk(&hw.sub_hardware, out); @@ -625,27 +925,62 @@ fn walk(tree: &[Hardware], out: &mut Vec) { /// Sidebar titles name the *role*, not the device: on Apple Silicon the CPU /// and GPU nodes are both called "Apple M5", so device names alone produce two -/// identical rows. The device name becomes the subtitle instead. Multiple -/// disks and NICs are numbered in tree order, as "Disk 0", "Disk 1". -fn role_title(t: HardwareType, existing: &[Category]) -> String { - let base = match t { +/// identical rows. The device name becomes the subtitle instead. +fn role_title(t: HardwareType) -> &'static str { + match t { HardwareType::Cpu => "CPU", HardwareType::Ram => "Memory", HardwareType::Storage | HardwareType::Hdd => "Disk", HardwareType::Network => "Network", _ => "GPU", - }; - if !matches!( - t, - HardwareType::Storage | HardwareType::Hdd | HardwareType::Network - ) { - return base.to_string(); } - let n = existing.iter().filter(|c| c.title.starts_with(base)).count(); - format!("{base} {n}") } -fn category_for(hw: &Hardware, existing: &[Category]) -> Option { +/// Make repeated roles tell themselves apart. +/// +/// A bare role is only unambiguous when the machine has one of them. This box +/// has two RAM-class nodes ("Total Memory" and "Virtual Memory") and six NICs, +/// which as bare roles rendered as two cards both labelled "Memory" and six +/// labelled "Network" — the sidebar could not say which was which. +/// +/// Run as a post-pass because the rule depends on the final tally, which no +/// single node knows while the tree is still being walked. +/// +/// Where the devices have distinct names, those names *are* the disambiguator +/// and read far better than an index — "Wi-Fi 2" beats "Network 5", and this is +/// what Windows shows. Numbering is the fallback for genuinely identical +/// devices, such as two disks that report the same model string. +fn disambiguate(cats: &mut [Category]) { + let mut counts: std::collections::HashMap<&'static str, usize> = + std::collections::HashMap::new(); + for c in cats.iter() { + *counts.entry(role_title(c.hardware_type)).or_default() += 1; + } + + let mut seen: std::collections::HashMap<&'static str, usize> = + std::collections::HashMap::new(); + for i in 0..cats.len() { + let role = role_title(cats[i].hardware_type); + if counts.get(role).copied().unwrap_or(0) < 2 { + continue; + } + // Distinct among the *others* sharing this role, not globally. + let unique = !cats.iter().enumerate().any(|(j, other)| { + j != i + && role_title(other.hardware_type) == role + && other.device == cats[i].device + }); + let n = seen.entry(role).or_default(); + cats[i].title = if unique { + cats[i].device.clone() + } else { + format!("{role} {n}") + }; + *n += 1; + } +} + +fn category_for(hw: &Hardware) -> Option { if !is_performance_node(hw.hardware_type) { return None; } @@ -665,11 +1000,13 @@ fn category_for(hw: &Hardware, existing: &[Category]) -> Option { .collect(); Some(Category { - title: role_title(hw.hardware_type, existing), + title: role_title(hw.hardware_type).to_string(), + device: hw.name.clone(), subtitle: format!("{} · {} sensors", hw.name, hw.sensors.len()), identifier: headline.identifier.clone(), value: headline.value, sensor_type: headline.sensor_type, + hardware_type: hw.hardware_type, detail, }) } @@ -822,12 +1159,12 @@ mod tests { crate::procs::ProcessRow { pid: 1, parent: None, name: "idle".into(), cmd: String::new(), user: None, cpu_pct: Some(0.0), mem_bytes: 1, virt_bytes: 1, - disk_read_bytes: 0, disk_write_bytes: 0, run_time_s: 0, + disk_read_bytes: 0, disk_write_bytes: 0, disk_bps: None, run_time_s: 0, }, crate::procs::ProcessRow { pid: 2, parent: None, name: "busy".into(), cmd: String::new(), user: None, cpu_pct: Some(90.0), mem_bytes: 1, virt_bytes: 1, - disk_read_bytes: 0, disk_write_bytes: 0, run_time_s: 0, + disk_read_bytes: 0, disk_write_bytes: 0, disk_bps: None, run_time_s: 0, }, ]; let sorted = crate::procs::arrange(&rows, None, st.sort, st.descending); @@ -843,7 +1180,45 @@ mod tests { assert_eq!(titles, vec!["CPU", "GPU"], "roles must disambiguate: {titles:?}"); } - /// Several disks must be numbered rather than all called "Disk". + /// Two RAM-class nodes are real on Windows — physical and virtual memory — + /// and both rendered as a bare "Memory" card, which is the one thing the + /// sidebar must never do. Distinct device names disambiguate them. + #[test] + fn repeated_roles_with_distinct_names_use_those_names() { + let ram = |id: &str, name: &str| Hardware { + identifier: id.into(), + name: name.into(), + hardware_type: HardwareType::Ram, + sensors: vec![sensor("Load", id, SensorType::Load, Some(50.0))], + sub_hardware: Vec::new(), + }; + let f = TelemetryFrame { + tree: vec![ram("/ram", "Total Memory"), ram("/vram", "Virtual Memory")], + ..Default::default() + }; + let titles: Vec = build_categories(&f).iter().map(|c| c.title.clone()).collect(); + assert_eq!(titles, vec!["Total Memory", "Virtual Memory"]); + } + + /// A lone instance keeps the plain role name — Windows shows "Memory", not + /// "Memory 0", when there is only one. + #[test] + fn a_single_instance_keeps_the_bare_role_name() { + let f = TelemetryFrame { + tree: vec![Hardware { + identifier: "/ram".into(), + name: "Total Memory".into(), + hardware_type: HardwareType::Ram, + sensors: vec![sensor("Load", "/r", SensorType::Load, Some(50.0))], + sub_hardware: Vec::new(), + }], + ..Default::default() + }; + assert_eq!(build_categories(&f)[0].title, "Memory"); + } + + /// Several disks reporting the *same* model string have no name to tell + /// them apart, so they fall back to numbering. #[test] fn multiple_disks_are_numbered() { let disk = |id: &str| Hardware { @@ -929,17 +1304,78 @@ mod tests { /// draw rather than produce NaN coordinates. #[test] fn flat_history_does_not_produce_nan_points() { - let history = vec![5.0f32; 10]; - let (mut lo, mut hi) = (f32::INFINITY, f32::NEG_INFINITY); - for &v in &history { - lo = lo.min(v); - hi = hi.max(v); - } - if (hi - lo).abs() < f32::EPSILON { - lo -= 1.0; - hi += 1.0; - } - let y = ((history[0] - lo) / (hi - lo)) * 100.0; + let (lo, hi) = value_range(&[5.0f32; 10], None); + let y = ((5.0 - lo) / (hi - lo)) * 100.0; assert!(y.is_finite(), "flat series produced a non-finite coordinate"); + assert!(lo < hi, "a flat series must still get a non-empty band"); + } + + /// An empty series must not yield infinities from the min/max fold. + #[test] + fn empty_history_yields_a_usable_range() { + let (lo, hi) = value_range(&[], None); + assert!(lo.is_finite() && hi.is_finite() && lo < hi, "got {lo}..{hi}"); + } + + /// Utilisation charts are pinned to 0-100 so an idle machine reads as idle, + /// rather than being auto-scaled until noise fills the panel. + #[test] + fn load_graphs_are_pinned_to_full_scale() { + assert_eq!(fixed_scale(SensorType::Load), Some((0.0, 100.0))); + // A quiet CPU stays near the floor instead of being stretched. + let (lo, hi) = value_range(&[1.0, 2.0, 1.5], fixed_scale(SensorType::Load)); + assert_eq!((lo, hi), (0.0, 100.0)); + // Everything else keeps auto-scaling — there is no fixed ceiling for a + // clock or a temperature. + assert_eq!(fixed_scale(SensorType::Clock), None); + let (lo, hi) = value_range(&[3000.0, 4000.0], fixed_scale(SensorType::Clock)); + assert!(lo < 3000.0 && hi > 4000.0, "auto-scale should pad: {lo}..{hi}"); + } + + /// Each device class must get its own hue, or the colour carries no + /// information — that distinctness is the whole point of the scheme. + #[test] + fn device_classes_get_distinct_colors() { + let pal = Palette::of(crate::settings::ColorMode::Black); + let types = [ + HardwareType::Cpu, + HardwareType::Ram, + HardwareType::Storage, + HardwareType::Network, + HardwareType::GpuNvidia, + ]; + let colors: Vec = types.iter().map(|t| category_color(*t, &pal)).collect(); + for (i, a) in colors.iter().enumerate() { + for (j, b) in colors.iter().enumerate() { + assert!(i == j || a != b, "{:?} and {:?} share a colour", types[i], types[j]); + } + } + // Every GPU vendor is one device class and shares the GPU hue. + assert_eq!( + category_color(HardwareType::GpuAti, &pal), + category_color(HardwareType::GpuNvidia, &pal) + ); + } + + /// The heat tint must grow with the value, and stay clear of the row + /// stripe at the bottom end so an idle table doesn't look grubby. + #[test] + fn heat_intensity_increases_with_value() { + let pal = Palette::of(crate::settings::ColorMode::Black); + let low = heat_color(0.1, &pal); + let high = heat_color(1.0, &pal); + assert!( + high.a() > low.a(), + "a busier cell must be tinted more strongly: {low:?} vs {high:?}" + ); + // Amber at the bottom, red at the top: hue carries signal as well. + // Compared as a red:green *ratio* rather than raw channels, because + // these are premultiplied — the alpha ramp above scales every channel, + // so absolute values say nothing about hue on their own. + let ratio = |c: Color32| f32::from(c.r()) / f32::from(c.g()).max(1.0); + assert!( + ratio(high) > ratio(low), + "tint should shift towards red as load climbs: {low:?} -> {high:?}" + ); } }