Chart UX fixes: zoom, jitter, grid toggle, MLG timestamp#67
Merged
SomethingNew71 merged 6 commits intoClassicMiniDIY:devfrom Apr 25, 2026
Merged
Chart UX fixes: zoom, jitter, grid toggle, MLG timestamp#67SomethingNew71 merged 6 commits intoClassicMiniDIY:devfrom
SomethingNew71 merged 6 commits intoClassicMiniDIY:devfrom
Conversation
Pinch and cmd+wheel zoom were silently no-ops while cursor_tracking was on (the default). The cursor-tracking branch in the chart closure forces bounds to a window of view_window_seconds centered on the cursor every frame, overwriting any zoom egui_plot just applied. Translate zoom_delta (and smooth_scroll_delta.y when scroll_to_zoom is enabled) into changes of view_window_seconds itself, so the window shrinks/grows around the cursor as the user expects.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces viewport-aware downsampling to improve chart detail during zoom, adds a configurable background grid with opacity settings, and corrects the Speeduino MLG timestamp scaling to 10 µs per tick. It also enables zooming while in cursor-tracking mode by introducing a transient view window state. Feedback highlights a high-severity performance issue where large data vectors are cloned every frame in compute_viewport_points, suggesting a move to references or better caching. Additionally, the now-obsolete downsample_cache should be removed to clean up the application state.
irudoy
force-pushed
the
fixes
branch
3 times, most recently
from
645eba0 to
4bda287
Compare
Collaborator
|
@irudoy Hey, thanks for the PR I see you're still actively working on things. Whenever you're ready, I can go through and review. Also, if you'd like to leave the tests, I can take care of those myself. |
Two MLG-related fixes bundled together: 1. Parser: MLG raw u16 timestamps are 10 µs/tick per the EFI Analytics MLG Binary Log Format spec, not 1 ms/tick. The previous formula compounded a 10× error in both the raw remainder and the wrap count for a net 100× over-estimate of wall-clock time (a ~30 min log showed as ~48 hours). Fixed via named constants MLG_TICK_SECONDS = 1e-5 and MLG_WRAP_TICKS = 65536, plus a regression test covering rusefi/speeduino sample logs. 2. UI: chart downsampling now slices raw data to the visible viewport before LTTB so detail scales with zoom. Previously LTTB compressed the full log to MAX_CHART_POINTS regardless of zoom, leaving fine detail invisible. Viewport bounds are remembered per plot area between frames; Y normalization uses the channel's global min/max so heights stay stable across pans.
LTTB partitions a slice into N equal buckets *by index* and picks the "best" point per bucket. During cursor-tracked playback the slice itself shifts a few samples each frame, so each bucket gets a slightly different sample set and the chosen peak flickers — visibly so when zoomed far out, where peaks are flattened to single buckets. Replace LTTB in compute_viewport_points with min/max-per-bucket, where bucket boundaries are anchored to absolute time (k × bucket_size from t = 0) instead of the viewport. As the cursor advances, samples slide through a fixed grid and each bucket's contents are invariant to the viewport offset, so peaks stay put. Two points per bucket (min, max) preserve the envelope; the bucket count is halved so total output stays under MAX_CHART_POINTS.
Add a Show Grid checkbox in the View menu and a Show Grid / Opacity slider in Settings → Display. Both knobs persist in UserSettings so the choice survives across sessions. Wire egui_plot's show_grid([x, y]) and grid_color() into both Plot::new chains (single and multi-area). The opacity slider overrides only the alpha of the base grid color, leaving the distance-based fade and theme RGB intact. Adds en/ru translations for menu.show_grid, settings.show_grid, settings.show_grid_desc, settings.grid_opacity.
Pinch / cmd+wheel / scroll-to-zoom in cursor-tracking mode used to write straight into view_window_seconds — the same field bound to the Window slider in Settings. Scrolling the chart visibly dragged the slider, and the zoomed value got persisted as the new default. Split the field in two: - `view_window_seconds` stays the user-set persistent default (Slider in Settings / Sidebar). The slider's write-path also resets the live render width. - `current_view_window` is the live render width used by the chart in cursor-tracking mode. Zoom interactions update only this field, leaving the slider and persisted value alone. Also gate the zoom helper on `ui.rect_contains_pointer(ui.max_rect())` so wheel/pinch events that land over the side panel (Settings, etc.) no longer reach the chart at all.
The IPC server set the listener to non-blocking and slept 100 ms between WouldBlock errors. There is no shutdown signal feeding the loop, so the non-blocking pattern only added up to ~100 ms of latency before each accepted connection plus continuous busy-poll wakeups. Switching to blocking accept removes the latency and the wakeups, and restores deterministic ordering for the in-tree IPC integration tests that previously flaked under cargo test parallelism (the listener could be mid-sleep when a connection arrived, pushing command delivery past the test's 2 s polling window).
Contributor
Author
|
Hey @SomethingNew71, all wrapped up — tests are green, ready for your review whenever you have time. Quick summary of the changes since your message:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug fixes
Correct MLG timestamp unit and viewport-aware chart detail
The Speeduino/rusEFI/MS MLG parser treated the u16 timestamp as milliseconds. Per EFI Analytics' MLG Binary Log Format 1.0 spec, each tick is 10 µs, so durations were inflated ~100× (e.g. a 30-minute log displayed as ~48 hours and chart detail was correspondingly sparse). Fixed by switching to
MLG_TICK_SECONDS = 1e-5with a regression test against the bundled example logs.Commit: 29ca8bb
Apply zoom to view_window in cursor-tracking mode
In cursor-tracking mode pinch / cmd+wheel was silently ignored — the view stayed locked to the saved Window seconds regardless of input. Now zoom gestures actually scale the tracked window, with a hover guard so they only fire when the pointer is over the chart area.
Commit: d334ffc
Stop chart jitter on play with anchored bucket downsampling
LTTB partitions the input slice by index, so as the viewport pans during playback the per-bucket pick shifts and visible peaks visibly dance. Replaced with min/max-per-bucket downsampling where bucket boundaries are anchored to absolute time (
k * bucket_sizefrom t=0), making the picked points invariant to viewport offset. Constant-channel handling preserved.Commit: 7e09537
Decouple chart zoom from the Window-size setting
Previously, scroll-to-zoom on the chart wrote directly into
view_window_seconds, which is the persisted Window slider — meaning a quick scroll silently rewrote the user's saved tracking window, and even worse, scrolling inside the Settings panel bled into that same field. Split into two states:view_window_seconds(slider-bound, persisted) andcurrent_view_window(transient, mutated by zoom gestures). The slider syncs into the transient on user change, but zoom never writes back.Commit: c685ab7
New feature
Grid toggle and opacity control for chart
Adds Show Grid to the View menu and a Grid Opacity slider in Settings → Display. Both persist in user settings via additive serde defaults so existing config files keep loading. The opacity slider only writes to disk on
drag_stopped/lost_focusto avoid thrashing settings.json on every drag pixel.Commit: 54f666f