diff --git a/crates/i3rs-app/src/panels/graph.rs b/crates/i3rs-app/src/panels/graph.rs index 3ba7fd5..8c565fd 100644 --- a/crates/i3rs-app/src/panels/graph.rs +++ b/crates/i3rs-app/src/panels/graph.rs @@ -332,11 +332,72 @@ 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) +} + +/// Copy of [`egui::Ui::dnd_drag_source`] with `Sense::click_and_drag()`. +/// +/// 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, + 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; + ui.interact(response.rect, id, egui::Sense::click_and_drag()) + .on_hover_cursor(egui::CursorIcon::Grab) + } +} + #[derive(Clone, Copy, PartialEq, Eq)] enum ManageChannelsSelection { Group(usize), @@ -1198,13 +1259,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 +1393,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,68 +1657,95 @@ impl GraphPanel { shared.data_duration.unwrap_or_default(), ); - egui::ScrollArea::vertical() - .auto_shrink([false, false]) - .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 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, }, @@ -1669,121 +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, - ); - if target_group.is_none() { - self.tile_heights.clear(); - } - 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); @@ -2334,14 +2372,74 @@ impl GraphPanel { self.tile_heights.clear(); return; } - let default_height = (available_height / tile_count as f32).max(80.0); - if self.tile_heights.len() < tile_count { - self.tile_heights.resize(tile_count, default_height); - } 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 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); + Self::fit_tile_heights(&mut self.tile_heights, available_height); } - for height in &mut self.tile_heights { - *height = height.max(80.0); + } + + /// 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; + } + + 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; + } } } @@ -2349,9 +2447,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 +2782,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 +3050,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, @@ -2965,22 +3062,21 @@ 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(), - ) - }) - .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 +3109,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 +4526,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 +4534,107 @@ 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; removing one has to give the + /// space back. + #[test] + 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]); + + 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. + #[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 + ); + } + + /// 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 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); + + let total: f32 = panel.tile_heights.iter().sum(); + 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 + /// 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 + /// `ensure_tile_heights` must leave dragged heights alone. + #[test] + 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); + + 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]; diff --git a/crates/i3rs-core/src/ld_parser.rs b/crates/i3rs-core/src/ld_parser.rs index 7231734..f8992ac 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,9 +114,14 @@ impl DataType { match (dtype_a, dtype_code) { (0x07, 4) => DataType::Float32, (0x07, 2) => DataType::Float16, - (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), } } @@ -121,6 +131,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 +143,7 @@ impl DataType { match self { DataType::Float32 => "float32", DataType::Float16 => "float16", + DataType::Int8 => "int8", DataType::Int16 => "int16", DataType::Int32 => "int32", DataType::Float64 => "float64", @@ -331,8 +343,9 @@ impl LdFile { return Some(vec![]); } - let raw = self.read_raw_samples(offset, actual_count, channel.data_type); - Some(self.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. @@ -365,8 +378,9 @@ impl LdFile { return Some(vec![]); } - let raw = self.read_raw_samples(offset, actual, channel.data_type); - Some(self.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. @@ -396,6 +410,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 +421,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 +430,31 @@ impl LdFile { // Internal parsers // --------------------------------------------------------------------------- +/// Convert raw samples to engineering units: +/// +/// ```text +/// value = raw / scale * 10^(-dec_places) * mul + shift +/// ``` +/// +/// `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); + + for v in values { + *v = *v / scale_f * dec_factor * mul_f + shift_f; + } +} + fn parse_session(data: &[u8]) -> Session { Session { date: read_string(data, 0x5E, 16), @@ -727,4 +752,101 @@ 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()), + } + } + + 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 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]); + } + + #[test] + fn scaling_uses_scale_and_dec_places() { + // Steering Wheel Position: shift=-6, mul=250, scale=9, dec_places=3. + 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 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 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 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 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(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), + 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/crates/i3rs-core/tests/integration_tests.rs b/crates/i3rs-core/tests/integration_tests.rs index f9c7e14..2af6044 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(); @@ -304,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); @@ -337,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!( 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.