diff --git a/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs b/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs index cf36cad..cf74a0a 100644 --- a/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs +++ b/src/ParquetViewer/Controls/AudioPlayerDataGridViewCell.cs @@ -20,7 +20,11 @@ internal class AudioPlayerDataGridViewCell : DataGridViewTextBoxCell private Timer _updateTimer = new() { Interval = 100 }; private Timer _initializationTimer = new() { Interval = 100 }; - private bool _isInitialized = false; + //Written by the background initialization task and read by the UI thread while painting. + //volatile gives the release/acquire ordering that makes the writes above it visible: without it + //the UI thread could observe _isInitialized as true while _audioStream was still null, which is + //what the defensive null check in Paint is working around. + private volatile bool _isInitialized = false; private AudioFormat? _audioFormat = AudioFormat.Invalid; private string _errorMessage = "loading..."; private bool _isCellTooSmall = false; @@ -31,7 +35,6 @@ internal class AudioPlayerDataGridViewCell : DataGridViewTextBoxCell private Rectangle _trackBarBounds; private Rectangle _contextMenuButtonBounds; - private string? _debugText = null; private bool _isCursorHoveringPlayPauseButton; private bool _isCursorHoveringStopButton; private bool _isCursorHoveringMenuButton; @@ -56,13 +59,27 @@ public AudioPlayerDataGridViewCell() private void RedrawCell() => DataGridView?.InvalidateCell(this); private Task? _initializationTask = null; + + /// + /// Kicks off audio initialization on a background thread, once per cell. + /// + /// Must be called from the UI thread: it reads the cell's Value before dispatching. private Task InitializePlayerAsync() - => _initializationTask ??= Task.Run(() => + { + if (_initializationTask is not null) + return _initializationTask; + + //Read the cell state here rather than inside the task. Value and ValueType reach into the + //grid's data binding, which is not safe to touch from a background thread. + var cellValue = this.Value; + var cellValueTypeName = this.ValueType?.Name ?? "null"; + + return _initializationTask = Task.Run(() => { try { //Prepare audio stream - if (this.Value is IByteArrayValue byteArray) + if (cellValue is IByteArrayValue byteArray) { this._audioStream = GetAudioStream(byteArray.Data, out var audioFormat); this._audioFormat = audioFormat; @@ -73,13 +90,13 @@ private Task InitializePlayerAsync() this._audioPlayer.Init(this._audioStream); this._audioPlayer.PlaybackStopped += OnPlaybackStopped; } - else if (this.Value == DBNull.Value) + else if (cellValue == DBNull.Value) { this._audioFormat = null; } else { - throw new InvalidDataException($"{this.ValueType.Name} was not the expected type {nameof(IByteArrayValue)}"); + throw new InvalidDataException($"{cellValueTypeName} was not the expected type {nameof(IByteArrayValue)}"); } } catch (Exception ex) @@ -89,9 +106,11 @@ private Task InitializePlayerAsync() } finally { + //Set last: the volatile write publishes every field assigned above to the UI thread. this._isInitialized = true; } }); + } private void OnPlaybackStopped(object? source, StoppedEventArgs args) { @@ -202,15 +221,13 @@ protected override void Paint(Graphics graphics, Rectangle clipBounds, Rectangle timeFormat += this._audioStream.TotalTime.TotalSeconds < 0 ? @"\.fff" : string.Empty; //show milliseconds if the audio is less than 1 second string currentTime = this._audioStream.CurrentTime.ToString(timeFormat) ?? TimeSpan.FromSeconds(0).ToString(timeFormat); string totalTime = this._audioStream.TotalTime.ToString(timeFormat) ?? TimeSpan.FromSeconds(0).ToString(timeFormat); - TextRenderer.DrawText(graphics, _debugText ?? $"{currentTime} / {totalTime}", cellStyle.Font, _trackBarBounds, Theme.LightModeTheme.TextColor, TextFormatFlags.VerticalCenter | TextFormatFlags.HorizontalCenter); + TextRenderer.DrawText(graphics, $"{currentTime} / {totalTime}", cellStyle.Font, _trackBarBounds, Theme.LightModeTheme.TextColor, TextFormatFlags.VerticalCenter | TextFormatFlags.HorizontalCenter); } protected override void OnMouseMove(DataGridViewCellMouseEventArgs e) { base.OnMouseMove(e); - Rectangle translatedBounds = this._playPauseButtonBounds; - translatedBounds.Offset(-this._cellBounds.Location.X, -this._cellBounds.Location.Y); this._isCursorHoveringPlayPauseButton = ContainsCursor(this._playPauseButtonBounds, e.Location); this._isCursorHoveringStopButton = ContainsCursor(this._stopButtonBounds, e.Location); this._isCursorHoveringMenuButton = ContainsCursor(this._contextMenuButtonBounds, e.Location); @@ -322,7 +339,7 @@ private void TogglePlayPause() private void StopPlayback() { - if (this._audioPlayer == null || this._audioPlayer == null) + if (this._audioPlayer == null) return; if (this._audioPlayer.PlaybackState != PlaybackState.Stopped) @@ -411,6 +428,10 @@ private void ShowContextMenu(Point location) } } + //Show() is modeless, so the menu can't be disposed inline. Dispose it once it closes instead, + //otherwise every right click leaks a strip and its items for the lifetime of the grid. + menu.Closed += (_, _) => menu.BeginInvoke(menu.Dispose); + menu.Show(this.DataGridView, location + (Size)this._cellBounds.Location); menu.PerformLayout(); diff --git a/src/ParquetViewer/Controls/ParquetGridView.cs b/src/ParquetViewer/Controls/ParquetGridView.cs index 66bd266..ebf9646 100644 --- a/src/ParquetViewer/Controls/ParquetGridView.cs +++ b/src/ParquetViewer/Controls/ParquetGridView.cs @@ -1440,6 +1440,14 @@ protected override void OnDataBindingComplete(DataGridViewBindingCompleteEventAr //exceptions "seem" to be innocuous so going to keep doing it this way for now. //Only other alternative is to stop using AutoGenerateColumns :/ column.CellTemplate = new AudioPlayerDataGridViewCell(); + + //Assigning the template only affects cells created from this point on, and binding + //has already created every cell in the column by the time this event fires. Without + //replacing them the column keeps its text box cells and renders the raw bytes. + foreach (DataGridViewRow row in this.Rows) + { + row.Cells[column.Index] = new AudioPlayerDataGridViewCell(); + } } } }