From 947069eeee19d83853525741baa5c75e9044b84b Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 10 Jun 2026 23:31:36 -0500 Subject: [PATCH] code review fixes --- .../Controls/PlotViewTests.cs | 57 +++++++++++++++++++ .../Controls/PlotView.cs | 55 +++++++++++++++++- .../Services/WindowPlacementService.cs | 19 +++++-- 3 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 LibreHardwareMonitor.Windows.WinUI.Tests/Controls/PlotViewTests.cs diff --git a/LibreHardwareMonitor.Windows.WinUI.Tests/Controls/PlotViewTests.cs b/LibreHardwareMonitor.Windows.WinUI.Tests/Controls/PlotViewTests.cs new file mode 100644 index 0000000..b488916 --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI.Tests/Controls/PlotViewTests.cs @@ -0,0 +1,57 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using LibreHardwareMonitor.Windows.WinUI.Controls; +using LibreHardwareMonitor.Windows.WinUI.ViewModels; +using Xunit; + +namespace LibreHardwareMonitor.Windows.WinUI.Tests.Controls; + +public class PlotViewTests +{ + [Fact] + public void DecimatePoints_ReturnsOriginalList_WhenPointsCountSmall() + { + var points = new List + { + new(DateTime.UtcNow, 1.0), + new(DateTime.UtcNow.AddSeconds(1), 2.0), + new(DateTime.UtcNow.AddSeconds(2), 3.0), + }; + + var result = PlotView.DecimatePoints(points, 10); + + Assert.Equal(points.Count, result.Count); + Assert.Equal(points, result); + } + + [Fact] + public void DecimatePoints_DecimatesAndMaintainsEnvelope_WhenPointsCountLarge() + { + // 1000 points, decimate to targetWidth = 100 + var points = new List(); + var baseTime = DateTime.UtcNow; + for (int i = 0; i < 1000; i++) + { + // Create a sine wave with some noise/spikes + double val = Math.Sin(i * 0.1); + if (i == 505) val = 100.0; // extreme max spike + if (i == 510) val = -100.0; // extreme min spike + points.Add(new PlotPointViewModel(baseTime.AddSeconds(i), val)); + } + + var result = PlotView.DecimatePoints(points, 100); + + // Result should be decimated (less than original, but bounds preserved) + Assert.True(result.Count < points.Count); + Assert.True(result.Count >= 100); + + // Extreme spike values must be preserved in the output + Assert.Contains(result, p => p.Value == 100.0); + Assert.Contains(result, p => p.Value == -100.0); + + // Rightmost point (latest sample) must be exactly preserved + Assert.Equal(points[^1].Timestamp, result[^1].Timestamp); + Assert.Equal(points[^1].Value, result[^1].Value); + } +} diff --git a/LibreHardwareMonitor.Windows.WinUI/Controls/PlotView.cs b/LibreHardwareMonitor.Windows.WinUI/Controls/PlotView.cs index 5b01654..088a7bd 100644 --- a/LibreHardwareMonitor.Windows.WinUI/Controls/PlotView.cs +++ b/LibreHardwareMonitor.Windows.WinUI/Controls/PlotView.cs @@ -458,7 +458,9 @@ public sealed class PlotView : Grid StrokeLineJoin = PenLineJoin.Round }; - foreach (PlotPointViewModel point in sample.Points) + IReadOnlyList decimatedPoints = DecimatePoints(sample.Points, (int)Math.Max(100, bounds.Width)); + + foreach (PlotPointViewModel point in decimatedPoints) { double x = bounds.Left + (point.Timestamp - minTimestamp).Ticks / (double)rangeTicks * bounds.Width; double y = axis.Bottom - ((point.Value - axis.MinValue) / (axis.MaxValue - axis.MinValue) * axis.Height); @@ -470,6 +472,57 @@ public sealed class PlotView : Grid DrawPointMarker(canvas, line.Points[^1], stroke); } + internal static IReadOnlyList DecimatePoints(IReadOnlyList points, int targetWidth) + { + if (points.Count <= targetWidth * 2) + return points; + + List result = new(); + int bucketSize = points.Count / targetWidth; + if (bucketSize <= 1) + return points; + + for (int i = 0; i < targetWidth; i++) + { + int start = i * bucketSize; + int end = (i == targetWidth - 1) ? points.Count : (i + 1) * bucketSize; + if (start >= end) + break; + + int minIdx = start; + int maxIdx = start; + for (int j = start + 1; j < end; j++) + { + if (points[j].Value < points[minIdx].Value) + minIdx = j; + if (points[j].Value > points[maxIdx].Value) + maxIdx = j; + } + + if (minIdx == maxIdx) + { + result.Add(points[minIdx]); + } + else if (minIdx < maxIdx) + { + result.Add(points[minIdx]); + result.Add(points[maxIdx]); + } + else + { + result.Add(points[maxIdx]); + result.Add(points[minIdx]); + } + } + + if (result.Count > 0 && result[^1].Timestamp != points[^1].Timestamp) + { + result.Add(points[^1]); + } + + return result; + } + private void DrawPlotMessage(Canvas canvas, PlotBounds bounds, string message) { TextBlock label = CreatePlotLabel(message, new SolidColorBrush(GetPlotTextColor()), 13, 600); diff --git a/LibreHardwareMonitor.Windows.WinUI/Services/WindowPlacementService.cs b/LibreHardwareMonitor.Windows.WinUI/Services/WindowPlacementService.cs index 3998c92..3ff5594 100644 --- a/LibreHardwareMonitor.Windows.WinUI/Services/WindowPlacementService.cs +++ b/LibreHardwareMonitor.Windows.WinUI/Services/WindowPlacementService.cs @@ -66,16 +66,25 @@ internal sealed class WindowPlacementService public void Maximize() { - if (_appWindow.Presenter is OverlappedPresenter presenter) + if (_settings.GetValue(_settingPrefix + "Maximized", false) && _appWindow.Presenter is OverlappedPresenter presenter) presenter.Maximize(); } public void Save() { - _settings.SetValue(_settingPrefix + "Location.X", _appWindow.Position.X); - _settings.SetValue(_settingPrefix + "Location.Y", _appWindow.Position.Y); - _settings.SetValue(_settingPrefix + "Width", _appWindow.Size.Width); - _settings.SetValue(_settingPrefix + "Height", _appWindow.Size.Height); + if (_appWindow.Presenter is OverlappedPresenter presenter) + { + bool maximized = presenter.State == OverlappedPresenterState.Maximized; + _settings.SetValue(_settingPrefix + "Maximized", maximized); + + if (presenter.State == OverlappedPresenterState.Restored) + { + _settings.SetValue(_settingPrefix + "Location.X", _appWindow.Position.X); + _settings.SetValue(_settingPrefix + "Location.Y", _appWindow.Position.Y); + _settings.SetValue(_settingPrefix + "Width", _appWindow.Size.Width); + _settings.SetValue(_settingPrefix + "Height", _appWindow.Size.Height); + } + } } [DllImport("user32.dll")]