Fix inconsistencies in progress reporting (#2129)

Signal and setup events correctly
Eliminate possible races
Use a single event
Mark volatiles and reduce scope of waithandles
Common handler
100ms -> 50ms

ARMeilleure/Translation/PTC/Ptc.cs

@@ -52,14 +52,10 @@ namespace ARMeilleure.Translation.PTC
 
         private static readonly ManualResetEvent _waitEvent;
 
-        private static readonly AutoResetEvent _loggerEvent;
-
         private static readonly object _lock;
 
         private static bool _disposed;
 
-        private static volatile int _translateCount;
-
         internal static PtcJumpTable PtcJumpTable { get; private set; }
 
         internal static string TitleIdText { get; private set; }
@@ -70,9 +66,10 @@ namespace ARMeilleure.Translation.PTC
 
         internal static PtcState State { get; private set; }
 
-        // Progress update events
+        // Progress reporting helpers
-        public static event Action<bool> PtcTranslationStateChanged;
+        private static volatile int _translateCount;
-        public static event Action<int, int> PtcTranslationProgressChanged;
+        private static volatile int _translateTotalCount;
+        public static event Action<PtcLoadingState, int, int> PtcStateChanged;
 
         static Ptc()
         {
@@ -82,8 +79,6 @@ namespace ARMeilleure.Translation.PTC
 
             _waitEvent = new ManualResetEvent(true);
 
-            _loggerEvent = new AutoResetEvent(false);
-
             _lock = new object();
 
             _disposed = false;
@@ -773,10 +768,20 @@ namespace ARMeilleure.Translation.PTC
             }
 
             _translateCount = 0;
+            _translateTotalCount = profiledFuncsToTranslate.Count;
 
-            ThreadPool.QueueUserWorkItem(TranslationLogger, profiledFuncsToTranslate.Count);
+            PtcStateChanged?.Invoke(PtcLoadingState.Start, _translateCount, _translateTotalCount);
 
-            PtcTranslationStateChanged?.Invoke(true);
+            using AutoResetEvent progressReportEvent = new AutoResetEvent(false);
+
+            Thread progressReportThread = new Thread(ReportProgress)
+            {
+                Name = "Ptc.ProgressReporter",
+                Priority = ThreadPriority.Lowest,
+                IsBackground = true
+            };
+
+            progressReportThread.Start(progressReportEvent);
 
             void TranslateFuncs()
             {
@@ -825,8 +830,12 @@ namespace ARMeilleure.Translation.PTC
 
             threads.Clear();
 
-            _loggerEvent.Set();
+            progressReportEvent.Set();
-            PtcTranslationStateChanged?.Invoke(false);
+            progressReportThread.Join();
+
+            PtcStateChanged?.Invoke(PtcLoadingState.Loaded, _translateCount, _translateTotalCount);
+
+            Logger.Info?.Print(LogClass.Ptc, $"{_translateCount} of {_translateTotalCount} functions translated");
 
             PtcJumpTable.Initialize(jumpTable);
 
@@ -838,19 +847,25 @@ namespace ARMeilleure.Translation.PTC
             preSaveThread.Start();
         }
 
-        private static void TranslationLogger(object state)
+        private static void ReportProgress(object state)
         {
-            const int refreshRate = 100; // ms
+            const int refreshRate = 50; // ms
 
-            int profiledFuncsToTranslateCount = (int)state;
+            AutoResetEvent endEvent = (AutoResetEvent)state;
+
+            int count = 0;
 
             do
             {
-                PtcTranslationProgressChanged?.Invoke(_translateCount, profiledFuncsToTranslateCount);
+                int newCount = _translateCount;
-            }
-            while (!_loggerEvent.WaitOne(refreshRate));
 
-            Logger.Info?.Print(LogClass.Ptc, $"{_translateCount} of {profiledFuncsToTranslateCount} functions translated");
+                if (count != newCount)
+                {
+                    PtcStateChanged?.Invoke(PtcLoadingState.Loading, newCount, _translateTotalCount);
+                    count = newCount;
+                }
+            }
+            while (!endEvent.WaitOne(refreshRate));
         }
 
         internal static void WriteInfoCodeRelocUnwindInfo(ulong address, ulong guestSize, bool highCq, PtcInfo ptcInfo)
@@ -968,8 +983,6 @@ namespace ARMeilleure.Translation.PTC
                 Wait();
                 _waitEvent.Dispose();
 
-                _loggerEvent.Dispose();
-
                 DisposeMemoryStreams();
             }
         }

ARMeilleure/Translation/PTC/PtcLoadingState.cs

@@ -0,0 +1,9 @@
+namespace ARMeilleure.Translation.PTC
+{
+    public enum PtcLoadingState
+    {
+        Start,
+        Loading,
+        Loaded
+    }
+}

Ryujinx.Graphics.Gpu/GpuContext.cs

@@ -80,25 +80,14 @@ namespace Ryujinx.Graphics.Gpu
         internal Capabilities Capabilities => _caps.Value;
 
         /// <summary>
-        /// Signaled when shader cache begins and ends loading.
+        /// Event for signalling shader cache loading progress.
-        /// Signals true when loading has started, false when ended.
         /// </summary>
-        public event Action<bool> ShaderCacheStateChanged
+        public event Action<Shader.ShaderCacheState, int, int> ShaderCacheStateChanged
         {
             add => Methods.ShaderCache.ShaderCacheStateChanged += value;
             remove => Methods.ShaderCache.ShaderCacheStateChanged -= value;
         }
 
-        /// <summary>
-        /// Signaled while shader cache is loading to indicate current progress.
-        /// Provides current and total number of shaders loaded.
-        /// </summary>
-        public event Action<int, int> ShaderCacheProgressChanged
-        {
-            add => Methods.ShaderCache.ShaderCacheProgressChanged += value;
-            remove => Methods.ShaderCache.ShaderCacheProgressChanged -= value;
-        }
-
         /// <summary>
         /// Creates a new instance of the GPU emulation context.
         /// </summary>

Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs

@@ -38,10 +38,9 @@ namespace Ryujinx.Graphics.Gpu.Shader
         private const ulong ShaderCodeGenVersion = 2088;
 
         // Progress reporting helpers
-        private int _shaderCount;
+        private volatile int _shaderCount;
-        private readonly AutoResetEvent _progressReportEvent;
+        private volatile int _totalShaderCount;
-        public event Action<bool> ShaderCacheStateChanged;
+        public event Action<ShaderCacheState, int, int> ShaderCacheStateChanged;
-        public event Action<int, int> ShaderCacheProgressChanged;
 
         /// <summary>
         /// Creates a new instance of the shader cache.
@@ -57,8 +56,6 @@ namespace Ryujinx.Graphics.Gpu.Shader
             _gpPrograms = new Dictionary<ShaderAddresses, List<ShaderBundle>>();
             _gpProgramsDiskCache = new Dictionary<Hash128, ShaderBundle>();
             _cpProgramsDiskCache = new Dictionary<Hash128, ShaderBundle>();
-
-            _progressReportEvent = new AutoResetEvent(false);
         }
 
         /// <summary>
@@ -85,11 +82,25 @@ namespace Ryujinx.Graphics.Gpu.Shader
 
                 ReadOnlySpan<Hash128> guestProgramList = _cacheManager.GetGuestProgramList();
 
-                _progressReportEvent.Reset();
+                using AutoResetEvent progressReportEvent = new AutoResetEvent(false);
-                _shaderCount = 0;
 
-                ShaderCacheStateChanged?.Invoke(true);
+                _shaderCount = 0;
-                ThreadPool.QueueUserWorkItem(ProgressLogger, guestProgramList.Length);
+                _totalShaderCount = guestProgramList.Length;
+
+                ShaderCacheStateChanged?.Invoke(ShaderCacheState.Start, _shaderCount, _totalShaderCount);
+                Thread progressReportThread = null;
+
+                if (guestProgramList.Length > 0)
+                {
+                    progressReportThread = new Thread(ReportProgress)
+                    {
+                        Name = "ShaderCache.ProgressReporter",
+                        Priority = ThreadPriority.Lowest,
+                        IsBackground = true
+                    };
+
+                    progressReportThread.Start(progressReportEvent);
+                }
 
                 for (int programIndex = 0; programIndex < guestProgramList.Length; programIndex++)
                 {
@@ -318,7 +329,7 @@ namespace Ryujinx.Graphics.Gpu.Shader
                         _gpProgramsDiskCache.Add(key, new ShaderBundle(hostProgram, shaders));
                     }
 
-                    _shaderCount = programIndex;
+                    _shaderCount = programIndex + 1;
                 }
 
                 if (!isReadOnly)
@@ -329,26 +340,37 @@ namespace Ryujinx.Graphics.Gpu.Shader
                     _cacheManager.Synchronize();
                 }
 
-                _progressReportEvent.Set();
+                progressReportEvent.Set();
-                ShaderCacheStateChanged?.Invoke(false);
+                progressReportThread?.Join();
+
+                ShaderCacheStateChanged?.Invoke(ShaderCacheState.Loaded, _shaderCount, _totalShaderCount);
 
                 Logger.Info?.Print(LogClass.Gpu, $"Shader cache loaded {_shaderCount} entries.");
             }
         }
 
         /// <summary>
-        /// Raises ShaderCacheProgressChanged events periodically.
+        /// Raises ShaderCacheStateChanged events periodically.
         /// </summary>
-        private void ProgressLogger(object state)
+        private void ReportProgress(object state)
         {
-            const int refreshRate = 100; // ms
+            const int refreshRate = 50; // ms
+
+            AutoResetEvent endEvent = (AutoResetEvent)state;
+
+            int count = 0;
 
-            int totalCount = (int)state;
             do
             {
-                ShaderCacheProgressChanged?.Invoke(_shaderCount, totalCount);
+                int newCount = _shaderCount;
+
+                if (count != newCount)
+                {
+                    ShaderCacheStateChanged?.Invoke(ShaderCacheState.Loading, newCount, _totalShaderCount);
+                    count = newCount;
                 }
-            while (!_progressReportEvent.WaitOne(refreshRate));
+            }
+            while (!endEvent.WaitOne(refreshRate));
         }
 
         /// <summary>
@@ -833,7 +855,6 @@ namespace Ryujinx.Graphics.Gpu.Shader
                 }
             }
 
-            _progressReportEvent?.Dispose();
             _cacheManager?.Dispose();
         }
     }

Ryujinx.Graphics.Gpu/Shader/ShaderCacheState.cs

@@ -0,0 +1,13 @@
+namespace Ryujinx.Graphics.Gpu.Shader
+{
+    /// <summary>Shader cache loading states</summary>
+    public enum ShaderCacheState
+    {
+        /// <summary>Shader cache started loading</summary>
+        Start,
+        /// <summary>Shader cache is loading</summary>
+        Loading,
+        /// <summary>Shader cache finished loading</summary>
+        Loaded
+    }
+}

Ryujinx/Ui/MainWindow.cs

@@ -33,6 +33,9 @@ using System.Threading.Tasks;
 
 using GUI = Gtk.Builder.ObjectAttribute;
 
+using PtcLoadingState = ARMeilleure.Translation.PTC.PtcLoadingState;
+using ShaderCacheLoadingState = Ryujinx.Graphics.Gpu.Shader.ShaderCacheState;
+
 namespace Ryujinx.Ui
 {
     public class MainWindow : Window
@@ -105,8 +108,6 @@ namespace Ryujinx.Ui
         [GUI] Label           _loadingStatusLabel;
         [GUI] ProgressBar     _loadingStatusBar;
 
-        private string        _loadingStatusTitle = "";
-
 #pragma warning restore CS0649, IDE0044, CS0169
 
         public MainWindow() : this(new Builder("Ryujinx.Ui.MainWindow.glade")) { }
@@ -346,43 +347,38 @@ namespace Ryujinx.Ui
 
         private void SetupProgressUiHandlers()
         {
-            Ptc.PtcTranslationStateChanged -= PtcStatusChanged;
+            Ptc.PtcStateChanged -= ProgressHandler;
-            Ptc.PtcTranslationStateChanged += PtcStatusChanged;
+            Ptc.PtcStateChanged += ProgressHandler;
 
-            Ptc.PtcTranslationProgressChanged -= LoadingProgressChanged;
+            _emulationContext.Gpu.ShaderCacheStateChanged -= ProgressHandler;
-            Ptc.PtcTranslationProgressChanged += LoadingProgressChanged;
+            _emulationContext.Gpu.ShaderCacheStateChanged += ProgressHandler;
-
-            _emulationContext.Gpu.ShaderCacheStateChanged -= ShaderCacheStatusChanged;
-            _emulationContext.Gpu.ShaderCacheStateChanged += ShaderCacheStatusChanged;
-
-            _emulationContext.Gpu.ShaderCacheProgressChanged -= LoadingProgressChanged;
-            _emulationContext.Gpu.ShaderCacheProgressChanged += LoadingProgressChanged;
         }
 
-        private void ShaderCacheStatusChanged(bool state)
+        private void ProgressHandler<T>(T state, int current, int total) where T : Enum
         {
-            _loadingStatusTitle = "Shaders";
+            bool visible;
-            Application.Invoke(delegate
+            string label;
+
+            switch (state)
             {
-                _loadingStatusBar.Visible = _loadingStatusLabel.Visible = state;
+                case PtcLoadingState ptcState:
-            });
+                    visible = ptcState != PtcLoadingState.Loaded;
+                    label = $"PTC : {current}/{total}";
+                    break;
+                case ShaderCacheLoadingState shaderCacheState:
+                    visible = shaderCacheState != ShaderCacheLoadingState.Loaded;
+                    label = $"Shaders : {current}/{total}";
+                    break;
+                default:
+                    throw new ArgumentException($"Unknown Progress Handler type {typeof(T)}");
             }
 
-        private void PtcStatusChanged(bool state)
-        {
-            _loadingStatusTitle = "PTC";
             Application.Invoke(delegate
             {
-                _loadingStatusBar.Visible = _loadingStatusLabel.Visible = state;
+                _loadingStatusLabel.Text = label;
-            });
+                _loadingStatusBar.Fraction = total > 0 ? (double)current / total : 0;
-        }
+                _loadingStatusBar.Visible = visible;
-
+                _loadingStatusLabel.Visible = visible;
-        private void LoadingProgressChanged(int value, int total)
-        {
-            Application.Invoke(delegate
-            {
-                _loadingStatusBar.Fraction = (double)value / total;
-                _loadingStatusLabel.Text = $"{_loadingStatusTitle} : {value}/{total}";
             });
         }
 
@@ -464,6 +460,8 @@ namespace Ryujinx.Ui
 
                 UpdateGraphicsConfig();
 
+                SetupProgressUiHandlers();
+
                 SystemVersion firmwareVersion = _contentManager.GetCurrentFirmwareVersion();
 
                 bool isDirectory = Directory.Exists(path);
@@ -584,8 +582,6 @@ namespace Ryujinx.Ui
 
                 _deviceExitStatus.Reset();
 
-                SetupProgressUiHandlers();
-
                 Translator.IsReadyForTranslation.Reset();
 #if MACOS_BUILD
                 CreateGameWindow();