From b81009081fffb1df37f8992c9222228eaa96c006 Mon Sep 17 00:00:00 2001 From: Zachary Friss Date: Sun, 26 Jul 2026 22:10:28 -0400 Subject: [PATCH 1/4] Fix sample scaling order and decode 1-byte channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two parsing bugs that only surface on sim-exported ADL logs, both reported in #18. Scaling applied `shift` before `mul`: (raw / scale * 10^-dec + shift) * mul `shift` is an offset already in engineering units, so it belongs after the multiplier. The wrong order offsets a channel by shift * (mul - 1), which is zero whenever mul == 1 — the case in every .ld file in the repo, hence the blind spot. Sim exporters use mul = 2 with a non-zero shift to pack a signed range into int16, so Throttle Pos (shift=50, mul=2, dec_places=3) read 50-150 % instead of 0-100 %, and Ground Speed (shift=641, mul=2) carried a constant +641 km/h. Verified against the reporter's log: ground speed 0-301 km/h, water temp 52-94 C, tyre temps 19.6-219.6 C, brake bias 47.2 %, and wheel rotation speed that now agrees with ground speed for a 0.337 m rolling radius. Separately, `from_codes` had no case for a 1-byte sample width, so Gear and Marker fell through to Unknown and failed to load entirely. For integer channels `dtype_code` is the width in bytes — confirmed by walking the channel list by data_ptr, where the gap divided by n_data is exactly 1.000 for those two channels and 2.000 for every dt=2 channel. Added DataType::Int8, decoded signed since reverse gear logs as -1. Fixes #18 Claude-Session: https://claude.ai/code/session_01PHdY3DhzFpD3QtSJUN1PQv --- crates/i3rs-core/src/ld_parser.rs | 154 ++++++++++++++++++++++++++---- docs/ld-file-format.md | 13 ++- 2 files changed, 148 insertions(+), 19 deletions(-) diff --git a/crates/i3rs-core/src/ld_parser.rs b/crates/i3rs-core/src/ld_parser.rs index 7231734..31c5bab 100644 --- a/crates/i3rs-core/src/ld_parser.rs +++ b/crates/i3rs-core/src/ld_parser.rs @@ -38,6 +38,10 @@ fn read_u32(data: &[u8], offset: usize) -> u32 { ]) } +fn read_i8(data: &[u8], offset: usize) -> i8 { + data[offset] as i8 +} + fn read_i16(data: &[u8], offset: usize) -> i16 { i16::from_le_bytes([data[offset], data[offset + 1]]) } @@ -98,6 +102,7 @@ pub fn normalize_channel_name(name: &str) -> String { pub enum DataType { Float32, Float16, + Int8, Int16, Int32, Float64, @@ -109,6 +114,10 @@ impl DataType { match (dtype_a, dtype_code) { (0x07, 4) => DataType::Float32, (0x07, 2) => DataType::Float16, + // For integer channels `dtype_code` is the sample width in bytes. + // 1-byte channels are used for low-cardinality state values such as + // Gear and Marker. + (0x00 | 0x03 | 0x05, 1) => DataType::Int8, (0x00 | 0x03 | 0x05, 2) => DataType::Int16, (0x00 | 0x03 | 0x05, 4) => DataType::Int32, (0x08, 0x08) => DataType::Float64, @@ -121,6 +130,7 @@ impl DataType { match self { DataType::Float32 => Some(4), DataType::Float16 => Some(2), + DataType::Int8 => Some(1), DataType::Int16 => Some(2), DataType::Int32 => Some(4), DataType::Float64 => Some(8), @@ -132,6 +142,7 @@ impl DataType { match self { DataType::Float32 => "float32", DataType::Float16 => "float16", + DataType::Int8 => "int8", DataType::Int16 => "int16", DataType::Int32 => "int32", DataType::Float64 => "float64", @@ -332,7 +343,7 @@ impl LdFile { } let raw = self.read_raw_samples(offset, actual_count, channel.data_type); - Some(self.apply_scaling(&raw, channel)) + Some(apply_scaling(&raw, channel)) } /// Read a range of samples for a channel (by sample indices), with scaling. @@ -366,7 +377,7 @@ impl LdFile { } let raw = self.read_raw_samples(offset, actual, channel.data_type); - Some(self.apply_scaling(&raw, channel)) + Some(apply_scaling(&raw, channel)) } /// Look up a text label for a state/enum channel value. @@ -396,6 +407,7 @@ impl LdFile { let bits = read_u16(data, pos); f16::from_bits(bits).to_f64() } + DataType::Int8 => read_i8(data, pos) as f64, DataType::Int16 => read_i16(data, pos) as f64, DataType::Int32 => read_i32(data, pos) as f64, DataType::Float64 => read_f64(data, pos), @@ -406,21 +418,6 @@ impl LdFile { vals } - fn apply_scaling(&self, raw: &[f64], channel: &ChannelMeta) -> Vec { - let scale_f = channel.scale as f64; - let shift_f = channel.shift as f64; - let mul_f = channel.mul as f64; - let dec_factor = 10.0_f64.powi(-channel.dec_places as i32); - - if scale_f == 0.0 { - raw.to_vec() - } else { - raw.iter() - .map(|v| (v / scale_f * dec_factor + shift_f) * mul_f) - .collect() - } - } - fn data(&self) -> &[u8] { self.backing.as_slice() } @@ -430,6 +427,33 @@ impl LdFile { // Internal parsers // --------------------------------------------------------------------------- +/// Convert raw samples to engineering units: +/// +/// ```text +/// value = raw / scale * 10^(-dec_places) * mul + shift +/// ``` +/// +/// `shift` is an offset already expressed in engineering units, so it is applied +/// *after* `mul` rather than being scaled by it. The two orderings agree whenever +/// `mul == 1` — the common case in M1 logs — but a channel with `mul != 1` and +/// `shift != 0` comes out offset by `shift * (mul - 1)` if `shift` is folded in +/// first. Sim-exported ADL logs use `mul = 2` with a non-zero `shift` to pack a +/// range into int16, so the order is load-bearing there. +fn apply_scaling(raw: &[f64], channel: &ChannelMeta) -> Vec { + let scale_f = channel.scale as f64; + let shift_f = channel.shift as f64; + let mul_f = channel.mul as f64; + let dec_factor = 10.0_f64.powi(-channel.dec_places as i32); + + if scale_f == 0.0 { + raw.to_vec() + } else { + raw.iter() + .map(|v| v / scale_f * dec_factor * mul_f + shift_f) + .collect() + } +} + fn parse_session(data: &[u8]) -> Session { Session { date: read_string(data, 0x5E, 16), @@ -727,4 +751,100 @@ mod tests { assert_eq!(entries.get(&2).map(String::as_str), Some("On")); assert_eq!(next_pos, data.len()); } + + fn scaling_channel(shift: i16, mul: i16, scale: i16, dec_places: i16) -> ChannelMeta { + ChannelMeta { + index: 0, + name: String::new(), + short_name: String::new(), + unit: String::new(), + freq: 10, + n_data: 0, + data_type: DataType::Int16, + shift, + mul, + scale, + dec_places, + data_ptr: 0, + enum_labels: Arc::new(HashMap::new()), + } + } + + /// A sim-exported ADL log packs 0–100 % into int16 via `shift=50, mul=2, + /// dec_places=3`. Folding `shift` in before `mul` reports 50–150 % instead. + #[test] + fn shift_is_applied_after_the_multiplier() { + let chan = scaling_channel(50, 2, 1, 3); + let out = apply_scaling(&[-25_000.0, 0.0, 25_000.0], &chan); + assert!((out[0] - 0.0).abs() < 1e-9, "got {}", out[0]); + assert!((out[1] - 50.0).abs() < 1e-9, "got {}", out[1]); + assert!((out[2] - 100.0).abs() < 1e-9, "got {}", out[2]); + } + + #[test] + fn scaling_uses_scale_and_dec_places() { + // Steering Wheel Position: shift=-6, mul=250, scale=9, dec_places=3. + let chan = scaling_channel(-6, 250, 9, 3); + let out = apply_scaling(&[216.0], &chan); + assert!(out[0].abs() < 1e-9, "got {}", out[0]); + } + + #[test] + fn scaling_is_unchanged_when_mul_is_one() { + // Vast majority of M1 channels: the operator order must not matter here. + let chan = scaling_channel(32_760, 1, 1, 0); + let out = apply_scaling(&[-32_760.0, -30_000.0], &chan); + assert!(out[0].abs() < 1e-9, "got {}", out[0]); + assert!((out[1] - 2_760.0).abs() < 1e-9, "got {}", out[1]); + } + + #[test] + fn zero_scale_passes_raw_values_through() { + let chan = scaling_channel(50, 2, 0, 3); + assert_eq!(apply_scaling(&[7.0, -3.0], &chan), vec![7.0, -3.0]); + } + + /// For integer channels `dtype_code` is the sample width in bytes. Gear and + /// Marker are logged as 1-byte channels; before Int8 existed they decoded as + /// Unknown and the channel failed to load entirely. + #[test] + fn one_byte_integer_channels_are_recognised() { + assert_eq!(DataType::from_codes(0x03, 1), DataType::Int8); + assert_eq!(DataType::from_codes(0x00, 1), DataType::Int8); + assert_eq!(DataType::from_codes(0x05, 1), DataType::Int8); + assert_eq!(DataType::Int8.bytes_per_sample(), Some(1)); + } + + #[test] + fn existing_data_type_codes_are_unaffected() { + assert_eq!(DataType::from_codes(0x07, 4), DataType::Float32); + assert_eq!(DataType::from_codes(0x07, 2), DataType::Float16); + assert_eq!(DataType::from_codes(0x03, 2), DataType::Int16); + assert_eq!(DataType::from_codes(0x05, 4), DataType::Int32); + assert_eq!(DataType::from_codes(0x08, 0x08), DataType::Float64); + // Float16 must keep winning over Int8 for the 0x07 family. + assert_eq!( + DataType::from_codes(0x07, 1), + DataType::Unknown(0x07, 1), + "unmapped codes should stay Unknown rather than silently decoding" + ); + } + + /// Int8 is signed: MoTeC logs reverse gear as -1. + #[test] + fn int8_samples_decode_as_signed() { + let mut file = vec![0u8; HEAD_SIZE]; + file[0] = 0x40; // magic + let data_ptr = file.len(); + file.extend_from_slice(&[0x00, 0x01, 0x07, 0xFF]); // 0, 1, 7, -1 + + let mut chan = scaling_channel(0, 1, 1, 0); + chan.data_type = DataType::Int8; + chan.n_data = 4; + chan.data_ptr = data_ptr as u32; + + let ld = LdFile::from_bytes(file).expect("synthetic file should open"); + let vals = ld.read_channel_data(&chan).expect("Int8 should decode"); + assert_eq!(vals, vec![0.0, 1.0, 7.0, -1.0]); + } } diff --git a/docs/ld-file-format.md b/docs/ld-file-format.md index af9ea66..4f4d78c 100644 --- a/docs/ld-file-format.md +++ b/docs/ld-file-format.md @@ -210,15 +210,24 @@ Raw sample values are converted to engineering units using four scaling paramete from the channel metadata: ``` -converted = ((raw_value / scale) * 10^(-dec_places) + shift) * mul +converted = (raw_value / scale) * 10^(-dec_places) * mul + shift ``` Where: - `scale` — divisor (int16, typically 1) - `dec_places` — decimal shift as power of 10 (int16, typically 0) -- `shift` — additive offset (int16, typically 0) +- `shift` — additive offset, in engineering units (int16, typically 0) - `mul` — multiplier (int16, typically 1) +**Operator order matters.** `shift` is an offset already expressed in engineering +units, so it is applied *after* `mul`, not before. The two orderings agree whenever +`mul = 1`, which hides the difference in most M1 logs — but a file with `mul != 1` +and `shift != 0` comes out offset by `shift * (mul - 1)` if `shift` is folded in +first. Sim-derived logs (ADL device records exported by racing simulators) commonly +use `mul = 2` with a non-zero `shift` to pack a signed range into int16: e.g. +`Throttle Pos` with `shift=50, mul=2, scale=1, dec_places=3` stores 0–100 % as raw +−25000…25000, and the wrong order reports it as 50–150 %. + In practice, most channels in M1 logs have `scale=1, dec_places=0, shift=0, mul=1`, meaning the raw float32 values are already in engineering units. The scaling parameters are more commonly used with integer-encoded channels. From 73c1a0bbb639ff6c90ca221606494db5c0da4d94 Mon Sep 17 00:00:00 2001 From: Zachary Friss Date: Sun, 26 Jul 2026 22:10:43 -0400 Subject: [PATCH 2/4] Fix graph tile reflow, Manage Channels rows, and scrollbar gutter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three unrelated graph-panel issues found while testing #18. Tiles did not reflow when a graph was added. `ensure_tile_heights` computed available_height / tile_count but passed it to Vec::resize, which only fills newly appended slots — the existing tiles kept their old heights, so the stack overflowed into the scroll area instead of shrinking to make room. Redistribution now happens inside ensure_tile_heights whenever the count changes, scaling rather than resetting so proportions set by dragging survive. That also replaces three ad-hoc tile_heights.clear() calls which only papered over the add-by-drag path and discarded those proportions; the paths that went through add_channel, the channel picker or move_channel_to_new_graph never reset at all. Manage Channels rows could not be clicked. egui's dnd_drag_source layers a bare Sense::drag() over the contents it just added, and because that interaction registers last it sat on top of the row button and swallowed the press. Selection never happened, so the Remove button stayed disabled — and the context menu, attached to the same dead response, could not open either, leaving no way to remove a channel. Replaced with a clickable_drag_source helper that senses click_and_drag on the row itself, matching the pattern channel_browser.rs already uses. Added a per-row remove button so removal does not depend on discovering right-click or select-then-Remove. The main graph scroll area now reserves a permanent gutter (ScrollStyle::solid plus AlwaysVisible) so plot width no longer shifts as tiles are added or removed. egui's default scroll style floats over the content, so visibility alone would not have been enough. Claude-Session: https://claude.ai/code/session_01PHdY3DhzFpD3QtSJUN1PQv --- crates/i3rs-app/src/panels/graph.rs | 221 ++++++++++++++++++++++++---- 1 file changed, 195 insertions(+), 26 deletions(-) diff --git a/crates/i3rs-app/src/panels/graph.rs b/crates/i3rs-app/src/panels/graph.rs index 3ba7fd5..d477a44 100644 --- a/crates/i3rs-app/src/panels/graph.rs +++ b/crates/i3rs-app/src/panels/graph.rs @@ -332,11 +332,74 @@ const MANAGE_NAME_COL_W: f32 = 240.0; const MANAGE_MODE_COL_W: f32 = 80.0; const MANAGE_NUM_COL_W: f32 = 70.0; +/// Floor for a single tiled graph's height. Once `tile_count * MIN_TILE_HEIGHT` +/// exceeds the panel height the tiles stop shrinking and the stack scrolls. +const MIN_TILE_HEIGHT: f32 = 80.0; + #[derive(Clone, Copy)] struct ManageChannelDragPayload { channel_id: ChannelId, } +/// Scroll area for the main graph stack, with a permanent scrollbar gutter. +/// +/// `ScrollStyle::solid` makes the bar allocate its own space instead of floating +/// over the contents, and `AlwaysVisible` keeps that space reserved even when the +/// tiles fit. Together they stop the plots changing width — and the y-axis on the +/// right shifting — as tiles are added, removed or resized past the point where +/// the stack starts to overflow. +fn graph_stack_scroll_area(ui: &mut egui::Ui) -> egui::ScrollArea { + ui.style_mut().spacing.scroll = egui::style::ScrollStyle::solid(); + egui::ScrollArea::vertical() + .auto_shrink([false, false]) + .scroll_bar_visibility(egui::scroll_area::ScrollBarVisibility::AlwaysVisible) +} + +/// A drag source whose row also senses clicks. +/// +/// [`egui::Ui::dnd_drag_source`] layers a bare `Sense::drag()` interaction over +/// the contents it just added. Because that interaction is registered last it +/// sits on top of anything inside and swallows the press, so a row built with it +/// can neither be selected by a click nor opened with a secondary click — its +/// context menu is unreachable too. Sensing `click_and_drag` on the row itself +/// keeps the dragging behaviour while letting both kinds of click through. +fn clickable_drag_source( + ui: &mut egui::Ui, + id: egui::Id, + payload: Payload, + add_contents: impl FnOnce(&mut egui::Ui), +) -> egui::Response +where + Payload: std::any::Any + Send + Sync, +{ + if ui.ctx().is_being_dragged(id) { + egui::DragAndDrop::set_payload(ui.ctx(), payload); + + // Paint the row into a floating layer that follows the pointer. + let layer_id = egui::LayerId::new(egui::Order::Tooltip, id); + let response = ui + .scope_builder(egui::UiBuilder::new().layer_id(layer_id), add_contents) + .response; + if let Some(pointer_pos) = ui.ctx().pointer_interact_pos() { + let delta = pointer_pos - response.rect.center(); + ui.ctx().transform_layer_shapes( + layer_id, + egui::emath::TSTransform::from_translation(delta), + ); + } + response + } else { + let response = ui.scope(add_contents).response; + let row = ui + .interact(response.rect, id, egui::Sense::click_and_drag()) + .on_hover_cursor(egui::CursorIcon::Grab); + if row.drag_started() { + egui::DragAndDrop::set_payload(ui.ctx(), payload); + } + row + } +} + #[derive(Clone, Copy, PartialEq, Eq)] enum ManageChannelsSelection { Group(usize), @@ -1198,13 +1261,12 @@ impl GraphPanel { let mut first_x_bounds: Option<(f64, f64)> = None; let mut responses = Vec::new(); - egui::ScrollArea::vertical() - .auto_shrink([false, false]) + graph_stack_scroll_area(ui) .scroll_source(egui::scroll_area::ScrollSource::SCROLL_BAR) .show(ui, |ui| { for (tile_idx, group) in tile_groups.iter().enumerate() { let plot_id = format!("tile_{}_{}", self.id, tile_idx); - let tile_height = self.tile_heights[tile_idx].max(80.0); + let tile_height = self.tile_heights[tile_idx]; let mut plot = Plot::new(plot_id) .height(tile_height) @@ -1333,9 +1395,6 @@ impl GraphPanel { target_group.unwrap_or_else(|| self.next_tile_group()), shared, ); - if target_group.is_none() { - self.tile_heights.clear(); - } self.needs_zoom_reset = true; } } @@ -1600,12 +1659,11 @@ impl GraphPanel { shared.data_duration.unwrap_or_default(), ); - egui::ScrollArea::vertical() - .auto_shrink([false, false]) + graph_stack_scroll_area(ui) .show(ui, |ui| { let mut responses = Vec::new(); for (tile_idx, group) in tile_groups.iter().enumerate() { - let tile_height = self.tile_heights[tile_idx].max(80.0); + let tile_height = self.tile_heights[tile_idx]; let grouped: Vec<&PlottedChannel> = group .iter() .map(|&channel_idx| &self.plotted_channels[channel_idx]) @@ -1777,9 +1835,6 @@ impl GraphPanel { target_group.unwrap_or_else(|| self.next_tile_group()), shared, ); - if target_group.is_none() { - self.tile_heights.clear(); - } self.needs_zoom_reset = true; } } @@ -2334,14 +2389,33 @@ impl GraphPanel { self.tile_heights.clear(); return; } - let default_height = (available_height / tile_count as f32).max(80.0); + let equal_share = (available_height / tile_count as f32).max(MIN_TILE_HEIGHT); + let count_changed = self.tile_heights.len() != tile_count; + if self.tile_heights.len() < tile_count { - self.tile_heights.resize(tile_count, default_height); + self.tile_heights.resize(tile_count, equal_share); } else if self.tile_heights.len() > tile_count { self.tile_heights.truncate(tile_count); } + + // Adding or removing a tile changes how much height there is to go + // around, so rescale the whole stack to fill the panel again. Without + // this the existing tiles keep their old heights, the new one is simply + // appended, and the stack overflows into the scroll area instead of the + // graphs shrinking to make room. Scaling (rather than resetting to equal + // shares) keeps whatever proportions the user dragged. + if count_changed && available_height >= tile_count as f32 * MIN_TILE_HEIGHT { + let total: f32 = self.tile_heights.iter().sum(); + if total > 0.0 { + let factor = available_height / total; + for height in &mut self.tile_heights { + *height *= factor; + } + } + } + for height in &mut self.tile_heights { - *height = height.max(80.0); + *height = height.max(MIN_TILE_HEIGHT); } } @@ -2349,9 +2423,8 @@ impl GraphPanel { if upper_idx + 1 >= tile_heights.len() { return; } - let min_height = 80.0; - let max_down = tile_heights[upper_idx + 1] - min_height; - let max_up = tile_heights[upper_idx] - min_height; + let max_down = tile_heights[upper_idx + 1] - MIN_TILE_HEIGHT; + let max_up = tile_heights[upper_idx] - MIN_TILE_HEIGHT; let applied = delta.clamp(-max_up, max_down); tile_heights[upper_idx] += applied; tile_heights[upper_idx + 1] -= applied; @@ -2685,7 +2758,6 @@ impl GraphPanel { channel.tile_group = self.next_tile_group(); self.plotted_channels.push(channel); self.normalize_tile_groups(); - self.tile_heights.clear(); } fn sync_manage_channels_state(&mut self) { @@ -2954,7 +3026,8 @@ impl GraphPanel { )); let raw_unit = resolve_channel_meta(pc.channel_id, shared).1; ui.horizontal(|ui| { - let drag_response = ui.dnd_drag_source( + let row_response = clickable_drag_source( + ui, drag_id, ManageChannelDragPayload { channel_id: pc.channel_id, @@ -2968,19 +3041,18 @@ impl GraphPanel { egui::Button::new(row_label.clone()) .selected(row_selected) .truncate(), - ) - }) - .inner + ); + }); }, ); - if drag_response.inner.clicked() { + if row_response.clicked() { pending_selection = Some( ManageChannelsSelection::Channel(pc.channel_id), ); } - drag_response.inner.context_menu(|ui| { + row_response.context_menu(|ui| { let mut action = None; Self::show_channel_menu( ui, @@ -3013,6 +3085,17 @@ impl GraphPanel { ); edit_request = Some(pc.channel_id); } + if ui + .add(egui::Button::new( + egui::RichText::new("X").small(), + )) + .on_hover_text("Remove channel from this graph") + .clicked() + { + pending_actions.push(ManageChannelsAction::Remove( + pc.channel_id, + )); + } }); let drop_label = if row_pos + 1 == indices.len() { @@ -4419,7 +4502,7 @@ impl NormalizedNameExt for str { mod tests { use super::{ ActiveGraphXAxis, GraphAxisFingerprint, GraphPanel, GraphRenderFingerprint, - LapRenderFingerprint, OverlaySource, PlotXRange, SampleRange, + LapRenderFingerprint, MIN_TILE_HEIGHT, OverlaySource, PlotXRange, SampleRange, display_transform_fingerprint, integrate_speed_series, is_monotonic_non_decreasing, native_plot_points_for_range, overlay_axis_offset, time_from_monotonic_axis, }; @@ -4427,6 +4510,92 @@ mod tests { use i3rs_core::Lap; use std::sync::Arc; + /// Adding a graph has to shrink the existing tiles to make room, rather than + /// appending a tile and overflowing the panel. + #[test] + fn adding_a_tile_reflows_existing_tiles_to_fit() { + let panel_height = 600.0; + let mut panel = GraphPanel::new(0, "test"); + + panel.ensure_tile_heights(3, panel_height); + assert_eq!(panel.tile_heights, vec![200.0, 200.0, 200.0]); + + panel.ensure_tile_heights(4, panel_height); + assert_eq!(panel.tile_heights.len(), 4); + let total: f32 = panel.tile_heights.iter().sum(); + assert!( + (total - panel_height).abs() < 0.01, + "tiles should still fill the panel, got {total} for {:?}", + panel.tile_heights + ); + } + + /// Reflowing must not discard proportions the user set by dragging handles. + #[test] + fn reflow_preserves_user_dragged_proportions() { + let panel_height = 600.0; + let mut panel = GraphPanel::new(0, "test"); + panel.tile_heights = vec![300.0, 150.0, 150.0]; + + panel.ensure_tile_heights(4, panel_height); + + let total: f32 = panel.tile_heights.iter().sum(); + assert!((total - panel_height).abs() < 0.01, "got {total}"); + // The first tile stayed twice the height of its two original neighbours. + assert!( + (panel.tile_heights[0] / panel.tile_heights[1] - 2.0).abs() < 0.01, + "expected 2:1 ratio to survive, got {:?}", + panel.tile_heights + ); + } + + #[test] + fn removing_a_tile_reflows_to_fill_the_panel() { + let panel_height = 600.0; + let mut panel = GraphPanel::new(0, "test"); + panel.ensure_tile_heights(4, panel_height); + panel.ensure_tile_heights(3, panel_height); + + assert_eq!(panel.tile_heights.len(), 3); + let total: f32 = panel.tile_heights.iter().sum(); + assert!((total - panel_height).abs() < 0.01, "got {total}"); + } + + /// Past the point where every tile can hold its floor the stack scrolls + /// instead, so heights stop shrinking. + #[test] + fn tiles_stop_shrinking_at_the_minimum_height() { + let panel_height = 300.0; + let mut panel = GraphPanel::new(0, "test"); + panel.ensure_tile_heights(10, panel_height); + + assert_eq!(panel.tile_heights.len(), 10); + assert!( + panel.tile_heights.iter().all(|h| *h >= MIN_TILE_HEIGHT), + "no tile may go under the floor, got {:?}", + panel.tile_heights + ); + } + + /// A resize drag is zero-sum, so a stack that already fits stays fitting and + /// repeated `ensure_tile_heights` calls are a no-op. + #[test] + fn dragging_a_handle_conserves_total_height() { + let panel_height = 600.0; + let mut panel = GraphPanel::new(0, "test"); + panel.ensure_tile_heights(3, panel_height); + + GraphPanel::apply_adjacent_tile_resize(&mut panel.tile_heights, 0, 50.0); + assert_eq!(panel.tile_heights, vec![250.0, 150.0, 200.0]); + + let before = panel.tile_heights.clone(); + panel.ensure_tile_heights(3, panel_height); + assert_eq!( + panel.tile_heights, before, + "an unchanged tile count must not disturb dragged heights" + ); + } + #[test] fn integrates_kmh_speed_into_distance() { let speed = vec![36.0, 36.0, 36.0]; From b733fd0d6e556094a72096540b2a9796e71ced76 Mon Sep 17 00:00:00 2001 From: Zachary Friss Date: Sun, 26 Jul 2026 22:19:11 -0400 Subject: [PATCH 3/4] Decode the 0x06 integer family, scale in place, simplify reflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up from reviewing the previous two commits. Auditing every (dtype_a, dtype_code) pair across all six .ld files on hand turned up a third integer family, 0x06, missing from the data-type table. Four channels per file fell through to Unknown and silently failed to load — including in VIR_LAP.ld, the checked-in fixture: CP Lotus ESP System State, Lap Beacon Ticks, Lap GPS Closest Beacon and GPS Satellites. Decoded as int32 they read 1, ~248-374M ticks, beacon indices ~26000, and 6-10 satellites; as float32 they are all denormal garbage, so the family is unambiguous. Data-block spacing confirms the 4-byte width. The snapshot test counts metadata entries, so it was blind to this. apply_scaling now scales in place instead of collecting into a second Vec — a channel can hold ~190k samples, so that was a 1.5 MB transient allocation per read for nothing. Bit-exact: same operations, same order. fit_tile_heights replaces the scale-then-clamp in ensure_tile_heights. Clamping after scaling overflows the panel whenever a tile already sits near the floor, because clamping it back up adds height the others never gave away: [440, 80, 80] plus a fourth tile came to 632 px in a 600 px panel. Now tiles that would land under the floor are pinned there and the rest rescaled into the height that frees up, which fits exactly whenever the floor fits at all. Also from review: read_i8 helper so the Int8 arm matches its five siblings; the row remove button uses the existing RichText("X").small() idiom rather than a new glyph; clickable_drag_source documented as a fork of egui's dnd_drag_source with a pointer for re-diffing on upgrade, and its redundant drag_started payload set dropped to match upstream; dropped the render-site floor clamps that duplicated the invariant ensure_tile_heights already guarantees; consolidated the data-type and reflow assertion tables. Claude-Session: https://claude.ai/code/session_01PHdY3DhzFpD3QtSJUN1PQv --- crates/i3rs-app/src/panels/graph.rs | 162 +++++++++++++------- crates/i3rs-core/src/ld_parser.rs | 92 +++++------ crates/i3rs-core/tests/integration_tests.rs | 38 +++++ 3 files changed, 193 insertions(+), 99 deletions(-) diff --git a/crates/i3rs-app/src/panels/graph.rs b/crates/i3rs-app/src/panels/graph.rs index d477a44..5c16d7e 100644 --- a/crates/i3rs-app/src/panels/graph.rs +++ b/crates/i3rs-app/src/panels/graph.rs @@ -355,14 +355,17 @@ fn graph_stack_scroll_area(ui: &mut egui::Ui) -> egui::ScrollArea { .scroll_bar_visibility(egui::scroll_area::ScrollBarVisibility::AlwaysVisible) } -/// A drag source whose row also senses clicks. +/// Copy of [`egui::Ui::dnd_drag_source`] with `Sense::click_and_drag()`. /// -/// [`egui::Ui::dnd_drag_source`] layers a bare `Sense::drag()` interaction over -/// the contents it just added. Because that interaction is registered last it -/// sits on top of anything inside and swallows the press, so a row built with it -/// can neither be selected by a click nor opened with a secondary click — its -/// context menu is unreachable too. Sensing `click_and_drag` on the row itself -/// keeps the dragging behaviour while letting both kinds of click through. +/// Upstream senses bare `Sense::drag()`, and because that interaction is +/// registered *after* the contents it wraps, it sits on top of them and swallows +/// the press — a row built with it can neither be selected by a click nor opened +/// with a secondary click, leaving its context menu unreachable. Sensing +/// click-and-drag on the row itself keeps dragging while letting both kinds of +/// click through. +/// +/// Otherwise identical to upstream (egui 0.35, `src/ui.rs:2641`); re-diff against +/// it when upgrading egui. fn clickable_drag_source( ui: &mut egui::Ui, id: egui::Id, @@ -390,13 +393,8 @@ where response } else { let response = ui.scope(add_contents).response; - let row = ui - .interact(response.rect, id, egui::Sense::click_and_drag()) - .on_hover_cursor(egui::CursorIcon::Grab); - if row.drag_started() { - egui::DragAndDrop::set_payload(ui.ctx(), payload); - } - row + ui.interact(response.rect, id, egui::Sense::click_and_drag()) + .on_hover_cursor(egui::CursorIcon::Grab) } } @@ -2389,33 +2387,74 @@ impl GraphPanel { self.tile_heights.clear(); return; } - let equal_share = (available_height / tile_count as f32).max(MIN_TILE_HEIGHT); - let count_changed = self.tile_heights.len() != tile_count; - - if self.tile_heights.len() < tile_count { + // Adding or removing a tile changes how much height there is to go + // around, so refit the whole stack. Without this the existing tiles keep + // their old heights, the new one is simply appended, and the stack + // overflows into the scroll area instead of the graphs shrinking to make + // room. + if self.tile_heights.len() != tile_count { + let equal_share = (available_height / tile_count as f32).max(MIN_TILE_HEIGHT); self.tile_heights.resize(tile_count, equal_share); - } else if self.tile_heights.len() > tile_count { - self.tile_heights.truncate(tile_count); + Self::fit_tile_heights(&mut self.tile_heights, available_height); } + } - // Adding or removing a tile changes how much height there is to go - // around, so rescale the whole stack to fill the panel again. Without - // this the existing tiles keep their old heights, the new one is simply - // appended, and the stack overflows into the scroll area instead of the - // graphs shrinking to make room. Scaling (rather than resetting to equal - // shares) keeps whatever proportions the user dragged. - if count_changed && available_height >= tile_count as f32 * MIN_TILE_HEIGHT { - let total: f32 = self.tile_heights.iter().sum(); - if total > 0.0 { - let factor = available_height / total; - for height in &mut self.tile_heights { - *height *= factor; - } - } + /// Scale `heights` to sum to `available`, keeping their relative proportions + /// so heights the user dragged survive a tile being added or removed. + /// + /// Tiles that would scale below [`MIN_TILE_HEIGHT`] are pinned to it and the + /// rest rescaled into the height that frees up — plain proportional scaling + /// plus a clamp overflows the panel whenever one tile already sits near the + /// floor, since clamping it back up adds height the others never gave away. + fn fit_tile_heights(heights: &mut [f32], available: f32) { + let n = heights.len(); + if n == 0 { + return; + } + let total: f32 = heights.iter().sum(); + // Degenerate weights, or too many tiles to give each one its floor — the + // stack scrolls instead, so equal shares at the floor is the best fit. + if total <= 0.0 || available < n as f32 * MIN_TILE_HEIGHT { + heights.fill((available / n as f32).max(MIN_TILE_HEIGHT)); + return; } - for height in &mut self.tile_heights { - *height = height.max(MIN_TILE_HEIGHT); + let weights = heights.to_vec(); + let mut pinned = vec![false; n]; + loop { + let free_height = + available - pinned.iter().filter(|p| **p).count() as f32 * MIN_TILE_HEIGHT; + let free_weight: f32 = weights + .iter() + .zip(&pinned) + .filter(|(_, p)| !**p) + .map(|(w, _)| *w) + .sum(); + if free_weight <= 0.0 { + heights.fill(MIN_TILE_HEIGHT); + return; + } + let factor = free_height / free_weight; + + // Pinning a tile frees height for the others, so re-solve. Each pass + // pins at least one tile, which bounds this at `n` passes. + let mut newly_pinned = false; + for (i, weight) in weights.iter().enumerate() { + if !pinned[i] && weight * factor < MIN_TILE_HEIGHT { + pinned[i] = true; + newly_pinned = true; + } + } + if !newly_pinned { + for (i, height) in heights.iter_mut().enumerate() { + *height = if pinned[i] { + MIN_TILE_HEIGHT + } else { + weights[i] * factor + }; + } + return; + } } } @@ -3038,7 +3077,7 @@ impl GraphPanel { ui.colored_label(pc.color, "■"); ui.add_sized( [MANAGE_NAME_COL_W - 34.0, row_h], - egui::Button::new(row_label.clone()) + egui::Button::new(row_label) .selected(row_selected) .truncate(), ); @@ -4510,24 +4549,27 @@ mod tests { use i3rs_core::Lap; use std::sync::Arc; - /// Adding a graph has to shrink the existing tiles to make room, rather than - /// appending a tile and overflowing the panel. + /// Adding a graph has to shrink the existing tiles to make room rather than + /// appending a tile and overflowing the panel; removing one has to give the + /// space back. #[test] - fn adding_a_tile_reflows_existing_tiles_to_fit() { + fn changing_the_tile_count_reflows_the_stack_to_fit() { let panel_height = 600.0; let mut panel = GraphPanel::new(0, "test"); panel.ensure_tile_heights(3, panel_height); assert_eq!(panel.tile_heights, vec![200.0, 200.0, 200.0]); - panel.ensure_tile_heights(4, panel_height); - assert_eq!(panel.tile_heights.len(), 4); - let total: f32 = panel.tile_heights.iter().sum(); - assert!( - (total - panel_height).abs() < 0.01, - "tiles should still fill the panel, got {total} for {:?}", - panel.tile_heights - ); + for count in [4, 3, 7, 2] { + panel.ensure_tile_heights(count, panel_height); + assert_eq!(panel.tile_heights.len(), count); + let total: f32 = panel.tile_heights.iter().sum(); + assert!( + (total - panel_height).abs() < 0.01, + "{count} tiles should fill the panel, got {total} for {:?}", + panel.tile_heights + ); + } } /// Reflowing must not discard proportions the user set by dragging handles. @@ -4549,16 +4591,28 @@ mod tests { ); } + /// A tile already near the floor can't absorb its share of the shrink, so the + /// height has to come from the tiles that do have slack. Scaling everything + /// and then clamping would overflow the panel. #[test] - fn removing_a_tile_reflows_to_fill_the_panel() { + fn a_tile_at_the_floor_does_not_push_the_stack_over() { let panel_height = 600.0; let mut panel = GraphPanel::new(0, "test"); + panel.tile_heights = vec![440.0, MIN_TILE_HEIGHT, MIN_TILE_HEIGHT]; + panel.ensure_tile_heights(4, panel_height); - panel.ensure_tile_heights(3, panel_height); - assert_eq!(panel.tile_heights.len(), 3); let total: f32 = panel.tile_heights.iter().sum(); - assert!((total - panel_height).abs() < 0.01, "got {total}"); + assert!( + (total - panel_height).abs() < 0.01, + "expected an exact fit, got {total} for {:?}", + panel.tile_heights + ); + assert!( + panel.tile_heights.iter().all(|h| *h >= MIN_TILE_HEIGHT), + "no tile may go under the floor, got {:?}", + panel.tile_heights + ); } /// Past the point where every tile can hold its floor the stack scrolls @@ -4578,9 +4632,9 @@ mod tests { } /// A resize drag is zero-sum, so a stack that already fits stays fitting and - /// repeated `ensure_tile_heights` calls are a no-op. + /// `ensure_tile_heights` must leave dragged heights alone. #[test] - fn dragging_a_handle_conserves_total_height() { + fn unchanged_tile_count_preserves_dragged_heights() { let panel_height = 600.0; let mut panel = GraphPanel::new(0, "test"); panel.ensure_tile_heights(3, panel_height); diff --git a/crates/i3rs-core/src/ld_parser.rs b/crates/i3rs-core/src/ld_parser.rs index 31c5bab..f8992ac 100644 --- a/crates/i3rs-core/src/ld_parser.rs +++ b/crates/i3rs-core/src/ld_parser.rs @@ -114,13 +114,14 @@ impl DataType { match (dtype_a, dtype_code) { (0x07, 4) => DataType::Float32, (0x07, 2) => DataType::Float16, - // For integer channels `dtype_code` is the sample width in bytes. - // 1-byte channels are used for low-cardinality state values such as - // Gear and Marker. - (0x00 | 0x03 | 0x05, 1) => DataType::Int8, - (0x00 | 0x03 | 0x05, 2) => DataType::Int16, - (0x00 | 0x03 | 0x05, 4) => DataType::Int32, (0x08, 0x08) => DataType::Float64, + // For integer channels `dtype_code` is the sample width in bytes, and + // `dtype_a` only picks the family. 1-byte channels carry + // low-cardinality state values (Gear, Marker); 0x06 shows up on + // counters and indices (GPS Satellites, Lap Beacon Ticks). + (0x00 | 0x03 | 0x05 | 0x06, 1) => DataType::Int8, + (0x00 | 0x03 | 0x05 | 0x06, 2) => DataType::Int16, + (0x00 | 0x03 | 0x05 | 0x06, 4) => DataType::Int32, _ => DataType::Unknown(dtype_a, dtype_code), } } @@ -342,8 +343,9 @@ impl LdFile { return Some(vec![]); } - let raw = self.read_raw_samples(offset, actual_count, channel.data_type); - Some(apply_scaling(&raw, channel)) + let mut vals = self.read_raw_samples(offset, actual_count, channel.data_type); + apply_scaling(&mut vals, channel); + Some(vals) } /// Read a range of samples for a channel (by sample indices), with scaling. @@ -376,8 +378,9 @@ impl LdFile { return Some(vec![]); } - let raw = self.read_raw_samples(offset, actual, channel.data_type); - Some(apply_scaling(&raw, channel)) + let mut vals = self.read_raw_samples(offset, actual, channel.data_type); + apply_scaling(&mut vals, channel); + Some(vals) } /// Look up a text label for a state/enum channel value. @@ -433,24 +436,22 @@ impl LdFile { /// value = raw / scale * 10^(-dec_places) * mul + shift /// ``` /// -/// `shift` is an offset already expressed in engineering units, so it is applied -/// *after* `mul` rather than being scaled by it. The two orderings agree whenever -/// `mul == 1` — the common case in M1 logs — but a channel with `mul != 1` and -/// `shift != 0` comes out offset by `shift * (mul - 1)` if `shift` is folded in -/// first. Sim-exported ADL logs use `mul = 2` with a non-zero `shift` to pack a -/// range into int16, so the order is load-bearing there. -fn apply_scaling(raw: &[f64], channel: &ChannelMeta) -> Vec { +/// `shift` is already in engineering units, so it is applied *after* `mul` — see +/// `docs/ld-file-format.md` for why the order is load-bearing on ADL logs. +/// +/// Scales in place: a channel can hold ~190k samples, so this avoids allocating a +/// second buffer the size of the one just decoded. +fn apply_scaling(values: &mut [f64], channel: &ChannelMeta) { let scale_f = channel.scale as f64; + if scale_f == 0.0 { + return; + } let shift_f = channel.shift as f64; let mul_f = channel.mul as f64; let dec_factor = 10.0_f64.powi(-channel.dec_places as i32); - if scale_f == 0.0 { - raw.to_vec() - } else { - raw.iter() - .map(|v| v / scale_f * dec_factor * mul_f + shift_f) - .collect() + for v in values { + *v = *v / scale_f * dec_factor * mul_f + shift_f; } } @@ -770,12 +771,17 @@ mod tests { } } + fn scaled(chan: &ChannelMeta, raw: &[f64]) -> Vec { + let mut vals = raw.to_vec(); + apply_scaling(&mut vals, chan); + vals + } + /// A sim-exported ADL log packs 0–100 % into int16 via `shift=50, mul=2, /// dec_places=3`. Folding `shift` in before `mul` reports 50–150 % instead. #[test] fn shift_is_applied_after_the_multiplier() { - let chan = scaling_channel(50, 2, 1, 3); - let out = apply_scaling(&[-25_000.0, 0.0, 25_000.0], &chan); + let out = scaled(&scaling_channel(50, 2, 1, 3), &[-25_000.0, 0.0, 25_000.0]); assert!((out[0] - 0.0).abs() < 1e-9, "got {}", out[0]); assert!((out[1] - 50.0).abs() < 1e-9, "got {}", out[1]); assert!((out[2] - 100.0).abs() < 1e-9, "got {}", out[2]); @@ -784,44 +790,40 @@ mod tests { #[test] fn scaling_uses_scale_and_dec_places() { // Steering Wheel Position: shift=-6, mul=250, scale=9, dec_places=3. - let chan = scaling_channel(-6, 250, 9, 3); - let out = apply_scaling(&[216.0], &chan); + let out = scaled(&scaling_channel(-6, 250, 9, 3), &[216.0]); assert!(out[0].abs() < 1e-9, "got {}", out[0]); } #[test] fn scaling_is_unchanged_when_mul_is_one() { // Vast majority of M1 channels: the operator order must not matter here. - let chan = scaling_channel(32_760, 1, 1, 0); - let out = apply_scaling(&[-32_760.0, -30_000.0], &chan); + let out = scaled(&scaling_channel(32_760, 1, 1, 0), &[-32_760.0, -30_000.0]); assert!(out[0].abs() < 1e-9, "got {}", out[0]); assert!((out[1] - 2_760.0).abs() < 1e-9, "got {}", out[1]); } #[test] fn zero_scale_passes_raw_values_through() { - let chan = scaling_channel(50, 2, 0, 3); - assert_eq!(apply_scaling(&[7.0, -3.0], &chan), vec![7.0, -3.0]); + let out = scaled(&scaling_channel(50, 2, 0, 3), &[7.0, -3.0]); + assert_eq!(out, vec![7.0, -3.0]); } - /// For integer channels `dtype_code` is the sample width in bytes. Gear and - /// Marker are logged as 1-byte channels; before Int8 existed they decoded as - /// Unknown and the channel failed to load entirely. + /// For integer channels `dtype_code` is the sample width in bytes and + /// `dtype_a` only picks the family. Two families were missing: 1-byte widths + /// (Gear, Marker) and the whole of `0x06` (GPS Satellites, Lap Beacon + /// Ticks) — both decoded as Unknown and failed to load, the latter even in + /// the checked-in VIR_LAP fixture. #[test] - fn one_byte_integer_channels_are_recognised() { - assert_eq!(DataType::from_codes(0x03, 1), DataType::Int8); - assert_eq!(DataType::from_codes(0x00, 1), DataType::Int8); - assert_eq!(DataType::from_codes(0x05, 1), DataType::Int8); - assert_eq!(DataType::Int8.bytes_per_sample(), Some(1)); - } - - #[test] - fn existing_data_type_codes_are_unaffected() { + fn data_type_codes_map_as_expected() { + for family in [0x00, 0x03, 0x05, 0x06] { + assert_eq!(DataType::from_codes(family, 1), DataType::Int8); + assert_eq!(DataType::from_codes(family, 2), DataType::Int16); + assert_eq!(DataType::from_codes(family, 4), DataType::Int32); + } assert_eq!(DataType::from_codes(0x07, 4), DataType::Float32); assert_eq!(DataType::from_codes(0x07, 2), DataType::Float16); - assert_eq!(DataType::from_codes(0x03, 2), DataType::Int16); - assert_eq!(DataType::from_codes(0x05, 4), DataType::Int32); assert_eq!(DataType::from_codes(0x08, 0x08), DataType::Float64); + assert_eq!(DataType::Int8.bytes_per_sample(), Some(1)); // Float16 must keep winning over Int8 for the 0x07 family. assert_eq!( DataType::from_codes(0x07, 1), diff --git a/crates/i3rs-core/tests/integration_tests.rs b/crates/i3rs-core/tests/integration_tests.rs index f9c7e14..e59ea2b 100644 --- a/crates/i3rs-core/tests/integration_tests.rs +++ b/crates/i3rs-core/tests/integration_tests.rs @@ -111,6 +111,44 @@ fn vir_lap_parser_snapshot() { assert_eq!(gps_latitude.n_data, 2660); } +/// Four channels in this fixture use the 0x06 integer family, which was missing +/// from the data-type table — they decoded as Unknown and silently failed to +/// load. GPS Satellites is the readable canary: a satellite count, not garbage. +#[test] +fn integer_family_0x06_channels_decode() { + let ld = LdFile::open(TEST_LD).unwrap(); + for name in [ + "CP Lotus ESP System State", + "Lap Beacon Ticks", + "Lap GPS Closest Beacon", + "GPS Satellites", + ] { + let channel = ld + .channels + .iter() + .find(|channel| channel.name == name) + .unwrap_or_else(|| panic!("{name} channel missing")); + let data = ld + .read_channel_data(channel) + .unwrap_or_else(|| panic!("{name} should decode")); + assert_eq!(data.len(), channel.n_data as usize, "{name} sample count"); + } + + let satellites = ld + .channels + .iter() + .find(|channel| channel.name == "GPS Satellites") + .expect("GPS Satellites channel missing"); + let data = ld.read_channel_data(satellites).expect("should decode"); + let (min, max) = data + .iter() + .fold((f64::MAX, f64::MIN), |(lo, hi), v| (lo.min(*v), hi.max(*v))); + assert!( + (4.0..=32.0).contains(&min) && (4.0..=32.0).contains(&max), + "satellite count should be a plausible fix, got {min}..{max}" + ); +} + #[test] fn duration_is_plausible() { let ld = LdFile::open(TEST_LD).unwrap(); From 364055c1e20d41ebd48cb37acf975615137bc545 Mon Sep 17 00:00:00 2001 From: Zachary Friss Date: Sun, 26 Jul 2026 22:22:24 -0400 Subject: [PATCH 4/4] Fix CI: apply rustfmt and update stale data-type assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The graph panel edits left a block that rustfmt reflows, failing the format check. The data-type coverage test asserted the fixture still contained undecodable channels, which stopped being true once the 0x06 integer family was mapped — invert it to assert full coverage instead. Claude-Session: https://claude.ai/code/session_01FV6mm15SerkbAhgKByu2e9 --- crates/i3rs-app/src/panels/graph.rs | 289 ++++++++++---------- crates/i3rs-core/tests/integration_tests.rs | 11 +- 2 files changed, 144 insertions(+), 156 deletions(-) diff --git a/crates/i3rs-app/src/panels/graph.rs b/crates/i3rs-app/src/panels/graph.rs index 5c16d7e..8c565fd 100644 --- a/crates/i3rs-app/src/panels/graph.rs +++ b/crates/i3rs-app/src/panels/graph.rs @@ -1657,67 +1657,95 @@ impl GraphPanel { shared.data_duration.unwrap_or_default(), ); - graph_stack_scroll_area(ui) - .show(ui, |ui| { - let mut responses = Vec::new(); - for (tile_idx, group) in tile_groups.iter().enumerate() { - let tile_height = self.tile_heights[tile_idx]; - let grouped: Vec<&PlottedChannel> = group - .iter() - .map(|&channel_idx| &self.plotted_channels[channel_idx]) - .collect(); + graph_stack_scroll_area(ui).show(ui, |ui| { + let mut responses = Vec::new(); + for (tile_idx, group) in tile_groups.iter().enumerate() { + let tile_height = self.tile_heights[tile_idx]; + let grouped: Vec<&PlottedChannel> = group + .iter() + .map(|&channel_idx| &self.plotted_channels[channel_idx]) + .collect(); + + let mut plot = Plot::new(format!("lap_overlay_tile_{}_{}", self.id, tile_idx)) + .height(tile_height) + .allow_drag(egui::Vec2b::new(true, false)) + .allow_zoom(egui::Vec2b::new(true, false)) + .allow_scroll(false) + .show_axes(true) + .show_grid(true) + .y_axis_min_width(36.0) + .link_cursor(cursor_group, egui::Vec2b::new(true, false)); + if tile_idx == tile_groups.len() - 1 { + plot = plot.x_axis_label(viewport.axis_label()); + } - let mut plot = Plot::new(format!("lap_overlay_tile_{}_{}", self.id, tile_idx)) - .height(tile_height) - .allow_drag(egui::Vec2b::new(true, false)) - .allow_zoom(egui::Vec2b::new(true, false)) - .allow_scroll(false) - .show_axes(true) - .show_grid(true) - .y_axis_min_width(36.0) - .link_cursor(cursor_group, egui::Vec2b::new(true, false)); - if tile_idx == tile_groups.len() - 1 { - plot = plot.x_axis_label(viewport.axis_label()); - } + let y_range = Self::compute_y_range(&grouped); + let mut tile_cursor = None; + let resp = plot.show(ui, |plot_ui| { + let mut draw_specs = Vec::new(); + let (x_min, x_max) = if needs_zoom_reset { + viewport.full_range() + } else if let Some((z_min, z_max)) = zoom_range { + viewport.axis_range_for_time_range(z_min, z_max) + } else { + viewport.full_range() + }; + plot_ui.set_plot_bounds_x(x_min..=x_max); - let y_range = Self::compute_y_range(&grouped); - let mut tile_cursor = None; - let resp = plot.show(ui, |plot_ui| { - let mut draw_specs = Vec::new(); - let (x_min, x_max) = if needs_zoom_reset { - viewport.full_range() - } else if let Some((z_min, z_max)) = zoom_range { - viewport.axis_range_for_time_range(z_min, z_max) - } else { - viewport.full_range() - }; - plot_ui.set_plot_bounds_x(x_min..=x_max); + if let Some((y_min, y_max)) = y_range { + plot_ui.set_plot_bounds_y(y_min..=y_max); + } - if let Some((y_min, y_max)) = y_range { - plot_ui.set_plot_bounds_y(y_min..=y_max); - } + let target_width = plot_ui.response().rect.width().max(100.0) as usize; + let lap_render_cache = &mut self.lap_render_cache; + for pc in &grouped { + let freq = freq_map.get(&pc.channel_id).copied().unwrap_or(0); + let preserve_native_samples = + preserve_native_samples_for_channel(pc.channel_id, shared); + let reference_key = LapRenderCacheKey::Reference(pc.channel_id); + ensure_lap_series_cache( + plot_ui, + shared, + lap_render_cache, + reference_key, + LapSeriesRenderParams { + data: &pc.data, + freq, + lap: &viewport.reference_lap, + axis: &viewport.reference_axis, + transform: PlotTransform { + origin_axis: viewport.reference_origin_axis, + x_scale: 1.0, + x_offset: 0.0, + y_scale: pc.display_scale, + y_offset: pc.display_offset, + }, + target_width, + preserve_native_samples, + }, + ); + draw_specs.push((reference_key, pc.color, 2.0)); - let target_width = plot_ui.response().rect.width().max(100.0) as usize; - let lap_render_cache = &mut self.lap_render_cache; - for pc in &grouped { - let freq = freq_map.get(&pc.channel_id).copied().unwrap_or(0); - let preserve_native_samples = - preserve_native_samples_for_channel(pc.channel_id, shared); - let reference_key = LapRenderCacheKey::Reference(pc.channel_id); + for prepared in prepared_overlays.get(&pc.channel_id).into_iter().flatten() + { + let overlay_key = LapRenderCacheKey::Overlay { + channel_id: pc.channel_id, + overlay_idx: prepared.overlay_idx, + }; ensure_lap_series_cache( plot_ui, shared, lap_render_cache, - reference_key, + overlay_key, LapSeriesRenderParams { - data: &pc.data, - freq, - lap: &viewport.reference_lap, - axis: &viewport.reference_axis, + data: &prepared.data, + freq: prepared.freq, + lap: &prepared.lap, + axis: &prepared.axis, transform: PlotTransform { - origin_axis: viewport.reference_origin_axis, - x_scale: 1.0, - x_offset: 0.0, + origin_axis: prepared.origin_axis, + x_scale: prepared.scale, + x_offset: prepared.offset, y_scale: pc.display_scale, y_offset: pc.display_offset, }, @@ -1725,118 +1753,75 @@ impl GraphPanel { preserve_native_samples, }, ); - draw_specs.push((reference_key, pc.color, 2.0)); - - for prepared in - prepared_overlays.get(&pc.channel_id).into_iter().flatten() - { - let overlay_key = LapRenderCacheKey::Overlay { - channel_id: pc.channel_id, - overlay_idx: prepared.overlay_idx, - }; - ensure_lap_series_cache( - plot_ui, - shared, - lap_render_cache, - overlay_key, - LapSeriesRenderParams { - data: &prepared.data, - freq: prepared.freq, - lap: &prepared.lap, - axis: &prepared.axis, - transform: PlotTransform { - origin_axis: prepared.origin_axis, - x_scale: prepared.scale, - x_offset: prepared.offset, - y_scale: pc.display_scale, - y_offset: pc.display_offset, - }, - target_width, - preserve_native_samples, - }, - ); - draw_specs.push((overlay_key, prepared.color, prepared.width)); - } - } - - let lap_render_cache = &self.lap_render_cache; - for (cache_key, color, width) in draw_specs { - draw_cached_lap_series( - plot_ui, - lap_render_cache, - &cache_key, - color, - width, - ); + draw_specs.push((overlay_key, prepared.color, prepared.width)); } + } - if let Some(cursor_time) = cursor_time { - Self::draw_cursor_line( - plot_ui, - viewport.axis_value_for_time(cursor_time), - ); - } - if let Some(coord) = plot_ui.pointer_coordinate() { - tile_cursor = viewport.time_from_axis_value(coord.x); - } - }); + let lap_render_cache = &self.lap_render_cache; + for (cache_key, color, width) in draw_specs { + draw_cached_lap_series(plot_ui, lap_render_cache, &cache_key, color, width); + } - let bounds = resp.transform.bounds(); - let pair = (bounds.min()[0], bounds.max()[0]); - if first_bounds.is_none() { - first_bounds = Some(pair); + if let Some(cursor_time) = cursor_time { + Self::draw_cursor_line(plot_ui, viewport.axis_value_for_time(cursor_time)); } - if resp.response.hovered() { - hovered_bounds = Some(pair); - hovered_cursor = tile_cursor; + if let Some(coord) = plot_ui.pointer_coordinate() { + tile_cursor = viewport.time_from_axis_value(coord.x); } - Self::draw_group_legend( - ui, - resp.response.rect, - &grouped, - &channel_meta, - group, - cursor_time, - Some(viewport.time_range_from_plot_bounds(pair.0, pair.1)), - ); - responses.push((group.clone(), resp.response)); + }); - if tile_idx + 1 < tile_groups.len() { - let delta = self.show_resize_handle(ui).y; - if delta.abs() > 0.0 { - Self::apply_adjacent_tile_resize( - &mut self.tile_heights, - tile_idx, - delta, - ); - } - } + let bounds = resp.transform.bounds(); + let pair = (bounds.min()[0], bounds.max()[0]); + if first_bounds.is_none() { + first_bounds = Some(pair); } - - for (group, response) in &responses { - self.handle_tile_group_context_menu(response, group, &channel_meta, shared); + if resp.response.hovered() { + hovered_bounds = Some(pair); + hovered_cursor = tile_cursor; } + Self::draw_group_legend( + ui, + resp.response.rect, + &grouped, + &channel_meta, + group, + cursor_time, + Some(viewport.time_range_from_plot_bounds(pair.0, pair.1)), + ); + responses.push((group.clone(), resp.response)); - if shared.dragging_channel.is_some() - && ui.input(|i| i.pointer.any_released()) - && ui.ui_contains_pointer() - { - let target_group = responses - .iter() - .find(|(_, resp)| resp.contains_pointer()) - .and_then(|(group, _)| group.first()) - .and_then(|&idx| self.plotted_channels.get(idx)) - .map(|channel| channel.tile_group); - if let Some(ch_id) = shared.dragging_channel.take() { - self.add_channel_to_group( - ch_id, - target_group.unwrap_or_else(|| self.next_tile_group()), - shared, - ); - self.needs_zoom_reset = true; + if tile_idx + 1 < tile_groups.len() { + let delta = self.show_resize_handle(ui).y; + if delta.abs() > 0.0 { + Self::apply_adjacent_tile_resize(&mut self.tile_heights, tile_idx, delta); } } - }); + } + + for (group, response) in &responses { + self.handle_tile_group_context_menu(response, group, &channel_meta, shared); + } + + if shared.dragging_channel.is_some() + && ui.input(|i| i.pointer.any_released()) + && ui.ui_contains_pointer() + { + let target_group = responses + .iter() + .find(|(_, resp)| resp.contains_pointer()) + .and_then(|(group, _)| group.first()) + .and_then(|&idx| self.plotted_channels.get(idx)) + .map(|channel| channel.tile_group); + if let Some(ch_id) = shared.dragging_channel.take() { + self.add_channel_to_group( + ch_id, + target_group.unwrap_or_else(|| self.next_tile_group()), + shared, + ); + self.needs_zoom_reset = true; + } + } + }); if let Some(cursor_time) = hovered_cursor { shared.cursor_time = Some(cursor_time); diff --git a/crates/i3rs-core/tests/integration_tests.rs b/crates/i3rs-core/tests/integration_tests.rs index e59ea2b..2af6044 100644 --- a/crates/i3rs-core/tests/integration_tests.rs +++ b/crates/i3rs-core/tests/integration_tests.rs @@ -342,12 +342,12 @@ fn known_data_types_are_readable() { let ld = LdFile::open(TEST_LD).unwrap(); let mut types_seen = std::collections::HashSet::new(); let mut readable_count = 0; - let mut unknown_count = 0; + let mut unknown = Vec::new(); for ch in &ld.channels { types_seen.insert(ch.data_type.name()); if ch.data_type.name() == "unknown" { // Unknown data types may not be readable — just skip - unknown_count += 1; + unknown.push((ch.name.clone(), ch.data_type)); continue; } let data = ld.read_channel_data(ch); @@ -375,9 +375,12 @@ fn known_data_types_are_readable() { readable_count > 190, "only {readable_count} channels readable" ); + // Every data type in this file is decodable — the 0x06 integer family used + // to fall through to Unknown. A regression here means a family stopped + // mapping, not that the fixture changed. assert!( - unknown_count > 0, - "expected some unknown data types in test data" + unknown.is_empty(), + "expected every channel to decode, but these did not: {unknown:?}" ); // We should see more than one data type across 199 channels assert!(