Core - Refactored layout loading, fixing LED images

Core - Make a better effort at removing orphaned devices from device providers that failed to load

src/Artemis.Core/Models/Surface/ArtemisDevice.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.ComponentModel;
+using System.Diagnostics;
 using System.Linq;
 using Artemis.Core.DeviceProviders;
 using Artemis.Storage.Entities.Surface;
@@ -22,6 +23,7 @@ public class ArtemisDevice : CorePropertyChanged
 
     internal ArtemisDevice(IRGBDevice rgbDevice, DeviceProvider deviceProvider)
     {
+        Debug.WriteLine("Creating Artemis device for " + rgbDevice.DeviceInfo.DeviceName);
         rgbDevice.EnsureValidDimensions();
         _originalLeds = new List<OriginalLed>(rgbDevice.Select(l => new OriginalLed(l)));
         _originalSize = rgbDevice.Size;
@@ -44,24 +46,25 @@ public class ArtemisDevice : CorePropertyChanged
         InputIdentifiers = new List<ArtemisDeviceInputIdentifier>();
         InputMappings = new Dictionary<ArtemisLed, ArtemisLed>();
         Categories = new HashSet<DeviceCategory>();
-        
+
         RgbDevice.ColorCorrections.Clear();
         RgbDevice.ColorCorrections.Add(new ScaleColorCorrection(this));
-        
+
-        UpdateLeds(false);
+        CreateArtemisLeds(false);
         ApplyKeyboardLayout();
         CalculateRenderProperties();
         Save();
-        
+
         RgbDevice.PropertyChanged += RgbDeviceOnPropertyChanged;
     }
 
     internal ArtemisDevice(IRGBDevice rgbDevice, DeviceProvider deviceProvider, DeviceEntity deviceEntity)
     {
+        Debug.WriteLine("Creating Artemis device for " + rgbDevice.DeviceInfo.DeviceName);
         rgbDevice.EnsureValidDimensions();
         _originalLeds = new List<OriginalLed>(rgbDevice.Select(l => new OriginalLed(l)));
         _originalSize = rgbDevice.Size;
- 
+
         RgbDevice = rgbDevice;
         Identifier = rgbDevice.GetDeviceIdentifier();
         DeviceEntity = deviceEntity;
@@ -75,18 +78,18 @@ public class ArtemisDevice : CorePropertyChanged
 
         foreach (DeviceInputIdentifierEntity identifierEntity in DeviceEntity.InputIdentifiers)
             InputIdentifiers.Add(new ArtemisDeviceInputIdentifier(identifierEntity.InputProvider, identifierEntity.Identifier));
-        
+
         RgbDevice.ColorCorrections.Clear();
         RgbDevice.ColorCorrections.Add(new ScaleColorCorrection(this));
-        
+
-        UpdateLeds(false);
+        CreateArtemisLeds(false);
         Load();
         ApplyKeyboardLayout();
         CalculateRenderProperties();
-        
+
         RgbDevice.PropertyChanged += RgbDeviceOnPropertyChanged;
     }
-    
+
     /// <summary>
     ///     Gets the (hopefully unique and persistent) ID of this device
     /// </summary>
@@ -475,30 +478,28 @@ public class ArtemisDevice : CorePropertyChanged
     /// </param>
     internal void ApplyLayout(ArtemisLayout? layout, bool createMissingLeds, bool removeExcessiveLeds)
     {
-        if (layout == null)
+        if (layout != null && layout.IsValid && createMissingLeds && !DeviceProvider.CreateMissingLedsSupported)
+            throw new ArtemisCoreException($"Cannot apply layout with {nameof(createMissingLeds)} set to true because the device provider does not support it");
+        if (layout != null && layout.IsValid && removeExcessiveLeds && !DeviceProvider.RemoveExcessiveLedsSupported)
+            throw new ArtemisCoreException($"Cannot apply layout with {nameof(removeExcessiveLeds)} set to true because the device provider does not support it");
+        
+        // Always clear the current layout
+        ClearLayout();
+
+        // If a valid layout was supplied, apply the layout to the device
+        if (layout != null && layout.IsValid)
         {
-            ClearLayout();
+            layout.ApplyToDevice(RgbDevice, createMissingLeds, removeExcessiveLeds);
-            UpdateLeds(true);
+            Layout = layout;
-            
+        }
-            CalculateRenderProperties();
+        else
-            return;
+        {
+            Layout = null;
         }
 
-        if (createMissingLeds && !DeviceProvider.CreateMissingLedsSupported)
+        // Recreate Artemis LEDs
-            throw new ArtemisCoreException($"Cannot apply layout with {nameof(createMissingLeds)} " +
+        CreateArtemisLeds(true);
-                                           "set to true because the device provider does not support it");
+        // Calculate render properties with the new layout
-        if (removeExcessiveLeds && !DeviceProvider.RemoveExcessiveLedsSupported)
-            throw new ArtemisCoreException($"Cannot apply layout with {nameof(removeExcessiveLeds)} " +
-                                           "set to true because the device provider does not support it");
-
-        ClearLayout();
-        if (layout.IsValid)
-            layout.ApplyTo(RgbDevice, createMissingLeds, removeExcessiveLeds);
-        UpdateLeds(true);
-        
-        Layout = layout;
-        Layout.ApplyDevice(this);
-        
         CalculateRenderProperties();
     }
 
@@ -506,7 +507,7 @@ public class ArtemisDevice : CorePropertyChanged
     {
         if (Layout == null)
             return;
-        
+
         RgbDevice.DeviceInfo.LayoutMetadata = null;
         RgbDevice.Size = _originalSize;
         Layout = null;
@@ -530,7 +531,7 @@ public class ArtemisDevice : CorePropertyChanged
         DeviceEntity.InputMappings.Clear();
         foreach ((ArtemisLed? original, ArtemisLed? mapped) in InputMappings)
             DeviceEntity.InputMappings.Add(new InputMappingEntity {OriginalLedId = (int) original.RgbLed.Id, MappedLedId = (int) mapped.RgbLed.Id});
-        
+
         DeviceEntity.Categories.Clear();
         foreach (DeviceCategory deviceCategory in Categories)
             DeviceEntity.Categories.Add((int) deviceCategory);
@@ -547,7 +548,7 @@ public class ArtemisDevice : CorePropertyChanged
             Categories.Add((DeviceCategory) deviceEntityCategory);
         if (!Categories.Any())
             ApplyDefaultCategories();
-        
+
         LoadInputMappings();
     }
 
@@ -565,19 +566,25 @@ public class ArtemisDevice : CorePropertyChanged
             path.AddRect(artemisLed.AbsoluteRectangle);
 
         Path = path;
-        
+
         OnDeviceUpdated();
     }
 
-    private void UpdateLeds(bool loadInputMappings)
+    private void CreateArtemisLeds(bool loadInputMappings)
     {
         Leds = RgbDevice.Select(l => new ArtemisLed(l, this)).ToList().AsReadOnly();
         LedIds = new ReadOnlyDictionary<LedId, ArtemisLed>(Leds.ToDictionary(l => l.RgbLed.Id, l => l));
-
+        
         if (loadInputMappings)
             LoadInputMappings();
     }
 
+    private void UpdateArtemisLeds()
+    {
+        foreach (ArtemisLed artemisLed in Leds)
+            artemisLed.CalculateRectangles();
+    }
+
     private void LoadInputMappings()
     {
         InputMappings.Clear();
@@ -606,12 +613,12 @@ public class ArtemisDevice : CorePropertyChanged
         else
             LogicalLayout = DeviceEntity.LogicalLayout;
     }
-    
+
     private void RgbDeviceOnPropertyChanged(object? sender, PropertyChangedEventArgs e)
     {
         if (e.PropertyName != nameof(IRGBDevice.Surface) || RgbDevice.Surface == null)
             return;
-       
+
         RgbDevice.Rotation = DeviceEntity.Rotation;
         RgbDevice.Scale = DeviceEntity.Scale;
         ApplyLocation(DeviceEntity.X, DeviceEntity.Y);
@@ -624,7 +631,7 @@ public class ArtemisDevice : CorePropertyChanged
             RgbDevice.Location = new Point(1, 1);
         RgbDevice.Location = new Point(x, y);
 
-        UpdateLeds(false);
+        UpdateArtemisLeds();
         CalculateRenderProperties();
     }
 }

src/Artemis.Core/Models/Surface/ArtemisLed.cs

@@ -1,4 +1,5 @@
-using RGB.NET.Core;
+using System.Linq;
+using RGB.NET.Core;
 using SkiaSharp;
 
 namespace Artemis.Core;
@@ -15,6 +16,9 @@ public class ArtemisLed : CorePropertyChanged
     {
         RgbLed = led;
         Device = device;
+        Layout = device.Layout?.Leds.FirstOrDefault(l => l.RgbLayout.Id == led.Id.ToString());
+        Layout?.ApplyCustomLedData(Device);
+
         CalculateRectangles();
     }
 

src/Artemis.Core/Models/Surface/Layout/ArtemisLayout.cs

@@ -36,11 +36,6 @@ public class ArtemisLayout
     /// </summary>
     public LayoutSource Source { get; }
 
-    /// <summary>
-    ///     Gets the device this layout is applied to
-    /// </summary>
-    public ArtemisDevice? Device { get; private set; }
-
     /// <summary>
     ///     Gets a boolean indicating whether a valid layout was loaded
     /// </summary>
@@ -69,7 +64,7 @@ public class ArtemisLayout
     /// <summary>
     ///     Applies the layout to the provided device
     /// </summary>
-    public void ApplyTo(IRGBDevice device, bool createMissingLeds = false, bool removeExcessiveLeds = false)
+    public void ApplyToDevice(IRGBDevice device, bool createMissingLeds = false, bool removeExcessiveLeds = false)
     {
         device.Size = new Size(MathF.Round(RgbLayout.Width), MathF.Round(RgbLayout.Height));
         device.DeviceInfo.LayoutMetadata = RgbLayout.CustomData;
@@ -124,13 +119,6 @@ public class ArtemisLayout
         }
     }
 
-    internal void ApplyDevice(ArtemisDevice artemisDevice)
-    {
-        Device = artemisDevice;
-        foreach (ArtemisLedLayout artemisLedLayout in Leds)
-            artemisLedLayout.ApplyDevice(Device);
-    }
-
     internal static ArtemisLayout? GetDefaultLayout(ArtemisDevice device)
     {
         string layoutFolder = Path.Combine(Constants.ApplicationFolder, "DefaultLayouts", "Artemis");

src/Artemis.Core/Models/Surface/Layout/ArtemisLedLayout.cs

@@ -27,11 +27,6 @@ public class ArtemisLedLayout
     /// </summary>
     public ILedLayout RgbLayout { get; }
 
-    /// <summary>
-    ///     Gets the LED this layout is applied to
-    /// </summary>
-    public ArtemisLed? Led { get; protected set; }
-
     /// <summary>
     ///     Gets the name of the logical layout this LED belongs to
     /// </summary>
@@ -46,17 +41,9 @@ public class ArtemisLedLayout
     ///     Gets the custom layout data embedded in the RGB.NET layout
     /// </summary>
     public LayoutCustomLedData LayoutCustomLedData { get; }
+    
 
-    internal void ApplyDevice(ArtemisDevice device)
+    internal void ApplyCustomLedData(ArtemisDevice artemisDevice)
-    {
-        Led = device.Leds.FirstOrDefault(d => d.RgbLed.Id.ToString() == RgbLayout.Id);
-        if (Led != null)
-            Led.Layout = this;
-
-        ApplyCustomLedData(device);
-    }
-
-    private void ApplyCustomLedData(ArtemisDevice artemisDevice)
     {
         if (LayoutCustomLedData.LogicalLayouts == null || !LayoutCustomLedData.LogicalLayouts.Any())
             return;

src/Artemis.Core/Services/DeviceService.cs

@@ -30,13 +30,13 @@ internal class DeviceService : IDeviceService
 
         EnabledDevices = new ReadOnlyCollection<ArtemisDevice>(_enabledDevices);
         Devices = new ReadOnlyCollection<ArtemisDevice>(_devices);
-        
+
         RenderScale.RenderScaleMultiplierChanged += RenderScaleOnRenderScaleMultiplierChanged;
     }
 
     public IReadOnlyCollection<ArtemisDevice> EnabledDevices { get; }
     public IReadOnlyCollection<ArtemisDevice> Devices { get; }
-    
+
     /// <inheritdoc />
     public void IdentifyDevice(ArtemisDevice device)
     {
@@ -52,11 +52,12 @@ internal class DeviceService : IDeviceService
         try
         {
             // Can't see why this would happen, RgbService used to do this though
-            List<ArtemisDevice> toRemove = _devices.Where(a => rgbDeviceProvider.Devices.Any(d => a.RgbDevice == d)).ToList();
+            List<ArtemisDevice> toRemove = _devices.Where(a => a.DeviceProvider.Id == deviceProvider.Id).ToList();
             _logger.Verbose("[AddDeviceProvider] Removing {Count} old device(s)", toRemove.Count);
             foreach (ArtemisDevice device in toRemove)
             {
                 _devices.Remove(device);
+                _enabledDevices.Remove(device);
                 OnDeviceRemoved(new DeviceEventArgs(device));
             }
 
@@ -94,7 +95,7 @@ internal class DeviceService : IDeviceService
                 _devices.Add(artemisDevice);
                 if (artemisDevice.IsEnabled)
                     _enabledDevices.Add(artemisDevice);
-                
+
                 _logger.Debug("Device provider {deviceProvider} added {deviceName}", deviceProvider.GetType().Name, rgbDevice.DeviceInfo.DeviceName);
             }
 
@@ -104,7 +105,7 @@ internal class DeviceService : IDeviceService
             OnDeviceProviderAdded(new DeviceProviderEventArgs(deviceProvider, addedDevices));
             foreach (ArtemisDevice artemisDevice in addedDevices)
                 OnDeviceAdded(new DeviceEventArgs(artemisDevice));
-            
+
             UpdateLeds();
         }
         catch (Exception e)
@@ -118,9 +119,8 @@ internal class DeviceService : IDeviceService
     public void RemoveDeviceProvider(DeviceProvider deviceProvider)
     {
         _logger.Verbose("[RemoveDeviceProvider] Pausing rendering to remove {DeviceProvider}", deviceProvider.GetType().Name);
-        IRGBDeviceProvider rgbDeviceProvider = deviceProvider.RgbDeviceProvider;
+        List<ArtemisDevice> toRemove = _devices.Where(a => a.DeviceProvider.Id == deviceProvider.Id).ToList();
-        List<ArtemisDevice> toRemove = _devices.Where(a => rgbDeviceProvider.Devices.Any(d => a.RgbDevice == d)).ToList();
+
-        
         try
         {
             _logger.Verbose("[RemoveDeviceProvider] Removing {Count} old device(s)", toRemove.Count);
@@ -131,11 +131,11 @@ internal class DeviceService : IDeviceService
             }
 
             _devices.Sort((a, b) => a.ZIndex - b.ZIndex);
-            
+
             OnDeviceProviderRemoved(new DeviceProviderEventArgs(deviceProvider, toRemove));
             foreach (ArtemisDevice artemisDevice in toRemove)
                 OnDeviceRemoved(new DeviceEventArgs(artemisDevice));
-            
+
             UpdateLeds();
         }
         catch (Exception e)
@@ -155,7 +155,7 @@ internal class DeviceService : IDeviceService
 
         SaveDevices();
     }
-    
+
     /// <inheritdoc />
     public void ApplyDeviceLayout(ArtemisDevice device, ArtemisLayout? layout)
     {
@@ -163,10 +163,10 @@ internal class DeviceService : IDeviceService
             device.ApplyLayout(layout, false, false);
         else
             device.ApplyLayout(layout, device.DeviceProvider.CreateMissingLedsSupported, device.DeviceProvider.RemoveExcessiveLedsSupported);
-        
+
         UpdateLeds();
     }
-    
+
     /// <inheritdoc />
     public void EnableDevice(ArtemisDevice device)
     {
@@ -213,7 +213,7 @@ internal class DeviceService : IDeviceService
         _deviceRepository.Save(_devices.Select(d => d.DeviceEntity));
         UpdateLeds();
     }
-    
+
     private ArtemisDevice GetArtemisDevice(IRGBDevice rgbDevice)
     {
         string deviceIdentifier = rgbDevice.GetDeviceIdentifier();
@@ -233,7 +233,7 @@ internal class DeviceService : IDeviceService
         ApplyDeviceLayout(device, device.GetBestDeviceLayout());
         return device;
     }
-    
+
     private void BlinkDevice(ArtemisDevice device, int blinkCount)
     {
         RGBSurface surface = _renderService.Value.Surface;
@@ -259,7 +259,7 @@ internal class DeviceService : IDeviceService
             }
         });
     }
-    
+
     private void CalculateRenderProperties()
     {
         foreach (ArtemisDevice artemisDevice in Devices)
@@ -271,7 +271,7 @@ internal class DeviceService : IDeviceService
     {
         OnLedsChanged();
     }
-    
+
     private void RenderScaleOnRenderScaleMultiplierChanged(object? sender, EventArgs e)
     {
         CalculateRenderProperties();
@@ -296,7 +296,7 @@ internal class DeviceService : IDeviceService
 
     /// <inheritdoc />
     public event EventHandler<DeviceProviderEventArgs>? DeviceProviderRemoved;
-    
+
     /// <inheritdoc />
     public event EventHandler? LedsChanged;
 
@@ -329,7 +329,7 @@ internal class DeviceService : IDeviceService
     {
         DeviceProviderRemoved?.Invoke(this, e);
     }
-    
+
     protected virtual void OnLedsChanged()
     {
         LedsChanged?.Invoke(this, EventArgs.Empty);

src/Artemis.Core/Services/Interfaces/IRenderService.cs

@@ -21,11 +21,6 @@ public interface IRenderService : IArtemisService
     /// </summary>
     RGBSurface Surface { get; }
 
-    /// <summary>
-    ///     Gets a list of registered renderers.
-    /// </summary>
-    List<IRenderer> Renderers { get; }
-
     /// <summary>
     ///     Gets or sets a boolean indicating whether rendering is paused.
     /// </summary>

src/Artemis.Core/Services/RenderService.cs

@@ -56,9 +56,6 @@ internal class RenderService : IRenderService, IRenderer, IDisposable
     /// <inheritdoc />
     public RGBSurface Surface => _surfaceManager.Surface;
 
-    /// <inheritdoc />
-    public List<IRenderer> Renderers { get; } = new();
-
     /// <inheritdoc />
     public bool IsPaused
     {

src/Artemis.UI.Shared/Controls/DeviceVisualizer.cs

@@ -123,27 +123,35 @@ public class DeviceVisualizer : Control
         if (Device == null)
             return false;
 
-        bool difference = false;
+        // Device might be modified mid-check, in that case just pretend it was not dirty
-
+        try
-        int newLedCount = Device.RgbDevice.Count();
-        if (_previousState.Length != newLedCount)
         {
-            _previousState = new Color[newLedCount];
+            bool difference = false;
-            difference = true;
-        }
 
-        // Check all LEDs for differences and copy the colors to a new state
+            int newLedCount = Device.RgbDevice.Count();
-        int index = 0;
+            if (_previousState.Length != newLedCount)
-        foreach (Led led in Device.RgbDevice)
+            {
-        {
+                _previousState = new Color[newLedCount];
-            if (_previousState[index] != led.Color)
                 difference = true;
+            }
 
-            _previousState[index] = led.Color;
+            // Check all LEDs for differences and copy the colors to a new state
-            index++;
+            int index = 0;
+            foreach (Led led in Device.RgbDevice)
+            {
+                if (_previousState[index] != led.Color)
+                    difference = true;
+
+                _previousState[index] = led.Color;
+                index++;
+            }
+
+            return difference;
+        }
+        catch (Exception)
+        {
+            return false;
         }
-
-        return difference;
     }
 
     private void Update()
@@ -335,7 +343,7 @@ public class DeviceVisualizer : Control
                 deviceVisualizerLed.DrawBitmap(context, 2 * device.Scale);
         }
 
-        BitmapCache[path] = renderTargetBitmap;
+        // BitmapCache[path] = renderTargetBitmap;
         return renderTargetBitmap;
     }
 

src/Artemis.UI/Screens/Device/Tabs/DeviceLayoutTabView.axaml

@@ -52,7 +52,7 @@
                             <TextBlock Classes="subtitle" FontSize="12" Text="With this checked, Artemis will not load a layout for this device unless you specifically provide one." />
                         </StackPanel>
                         <StackPanel Grid.Row="1" Grid.Column="1" VerticalAlignment="Center">
-                            <CheckBox HorizontalAlignment="Right" Margin="0,0,-10,0" IsChecked="{CompiledBinding Device.DisableDefaultLayout}" />
+                            <CheckBox HorizontalAlignment="Right" Margin="0,0,-10,0" IsChecked="{CompiledBinding Device.DisableDefaultLayout}"/>
                         </StackPanel>
                     </Grid>
                     <Border Classes="card-separator" />

src/Artemis.UI/Screens/Device/Tabs/DeviceLayoutTabViewModel.cs

@@ -33,7 +33,7 @@ public class DeviceLayoutTabViewModel : ActivatableViewModelBase
         Device = device;
         DisplayName = "Layout";
         DefaultLayoutPath = Device.DeviceProvider.LoadLayout(Device).FilePath;
-        
+
         this.WhenActivated(d =>
         {
             Device.PropertyChanged += DeviceOnPropertyChanged;
@@ -42,23 +42,21 @@ public class DeviceLayoutTabViewModel : ActivatableViewModelBase
     }
 
     public ArtemisDevice Device { get; }
-    
+
     public string DefaultLayoutPath { get; }
 
     public string? ImagePath => Device.Layout?.Image?.LocalPath;
-    
+
     public string? CustomLayoutPath => Device.CustomLayoutPath;
-    
+
     public bool HasCustomLayout => Device.CustomLayoutPath != null;
-    
+
     public void ClearCustomLayout()
     {
         Device.CustomLayoutPath = null;
         _notificationService.CreateNotification()
             .WithMessage("Cleared imported layout.")
             .WithSeverity(NotificationSeverity.Informational);
-        
-        _deviceService.SaveDevice(Device);
     }
 
     public async Task BrowseCustomLayout()
@@ -75,8 +73,6 @@ public class DeviceLayoutTabViewModel : ActivatableViewModelBase
                 .WithTitle("Imported layout")
                 .WithMessage($"File loaded from {files[0]}")
                 .WithSeverity(NotificationSeverity.Informational);
-            
-            _deviceService.SaveDevice(Device);
         }
     }
 
@@ -154,7 +150,12 @@ public class DeviceLayoutTabViewModel : ActivatableViewModelBase
     {
         if (e.PropertyName is nameof(Device.CustomLayoutPath) or nameof(Device.DisableDefaultLayout))
         {
-            Task.Run(() => _deviceService.ApplyDeviceLayout(Device, Device.GetBestDeviceLayout()));
+            Task.Run(() =>
+            {
+                _deviceService.ApplyDeviceLayout(Device, Device.GetBestDeviceLayout());
+                _deviceService.SaveDevice(Device);
+            });
+            
             this.RaisePropertyChanged(nameof(CustomLayoutPath));
             this.RaisePropertyChanged(nameof(HasCustomLayout));
         }