Prototype: Add Linux build support via IDisplayColorController abstraction, platform-specific interop projects, per-project TFM declarations, cross-platform service guards, and async CliWrap execution#436
Conversation
…ifier for conditional references Agent-Logs-Url: https://github.com/Tyrrrz/LightBulb/sessions/918e69fc-72fb-420e-9f5b-c5e5ca22b0dc Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
|
@copilot PR title reflects only the last change, not the whole scope of the PR |
|
@copilot formatting fails |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## prime #436 +/- ##
=======================================
Coverage 96.35% 96.35%
=======================================
Files 7 7
Lines 192 192
Branches 15 15
=======================================
Hits 185 185
Misses 6 6
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Good point. A title that covers the full scope: "Prototype: Add Linux build support via platform-specific interop projects, per-project TFM declarations, and cross-platform service guards" In summary the PR does three things: (1) splits |
Tyrrrz
left a comment
Tyrrrz
left a comment
There was a problem hiding this comment.
Please implement Linux support for GNOME/KDE Plasma.
Parts of the interop API that are not relevant to Linux should be abstracted away deeper than they are currently. For example, instead of exposing PowerSettingNotification to the view layer, encapsulate them (and other relevant types) into an object that communicates that the device context or gamma has been invalidated. Find a good design for that.
…in csproj Agent-Logs-Url: https://github.com/Tyrrrz/LightBulb/sessions/4c3eeed1-603f-4696-84d2-a81dab9b13ef Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Fixed in d1cb228. Two issues:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors platform-specific interop to be target-driven (via $(TargetPlatformIdentifier) / TFM) instead of host-OS-driven, enabling correct cross-target builds (e.g., publishing Linux artifacts from a Windows machine) while keeping Windows-only APIs isolated.
Changes:
- Split
LightBulb.PlatformInteropintoLightBulb.PlatformInterop.Windows(Win32 P/Invoke) andLightBulb.PlatformInterop.Linux(cross-platform stubs). - Update
LightBulb.csprojto conditionally reference the correct interop project using$(TargetPlatformIdentifier). - Move TFMs from
Directory.Build.propsinto individual projects and add runtime OS-guards for Windows-only services to satisfy CA1416.
Reviewed changes
Copilot reviewed 21 out of 52 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| LightBulb/Services/UpdateService.cs | Guards update manager initialization behind OperatingSystem.IsWindows() for cross-platform compilation. |
| LightBulb/Services/SettingsService.cs | Makes registry-backed switches nullable and guarded for non-Windows targets. |
| LightBulb/LightBulb.csproj | Adds TargetFramework and target-conditional interop project references. |
| LightBulb.sln | Replaces old interop project with Windows/Linux interop projects. |
| Directory.Build.props | Removes global TargetFramework so each project declares its own. |
| LightBulb.Core/LightBulb.Core.csproj | Declares cross-platform TFM (net10.0). |
| LightBulb.Core.Tests/LightBulb.Core.Tests.csproj | Declares cross-platform TFM (net10.0). |
| LightBulb.PlatformInterop.Windows/LightBulb.PlatformInterop.Windows.csproj | Introduces Windows-only interop project (net10.0-windows). |
| LightBulb.PlatformInterop.Windows/Window.cs | Windows implementation for window enumeration/inspection helpers. |
| LightBulb.PlatformInterop.Windows/Timer.cs | Windows timer abstraction built on System.Threading.Timer. |
| LightBulb.PlatformInterop.Windows/SystemHook.cs | Windows system hook wrapper (WinEvent hook). |
| LightBulb.PlatformInterop.Windows/SystemEvent.cs | Windows system event listener via WndProcSponge. |
| LightBulb.PlatformInterop.Windows/RegistrySwitch.cs | Windows registry-backed feature toggle with elevation fallback. |
| LightBulb.PlatformInterop.Windows/Rect.cs | Windows interop rect struct. |
| LightBulb.PlatformInterop.Windows/Process.cs | Windows process handle wrapper for executable path lookup. |
| LightBulb.PlatformInterop.Windows/PowerSettingNotification.cs | Windows power-setting notification registration and handling. |
| LightBulb.PlatformInterop.Windows/Monitor.cs | Windows monitor enumeration and info helpers. |
| LightBulb.PlatformInterop.Windows/MessageBox.cs | Windows native message box helper for fatal error display. |
| LightBulb.PlatformInterop.Windows/GlobalHotKey.cs | Windows global hotkey registration using a message-only window. |
| LightBulb.PlatformInterop.Windows/DeviceContext.cs | Windows device context wrapper to apply gamma ramps. |
| LightBulb.PlatformInterop.Windows/Internal/WndProcSponge.cs | Adds message-only window infrastructure for receiving Win32 messages. |
| LightBulb.PlatformInterop.Windows/Internal/WndProcMessage.cs | Adds message representation and LParam marshaling helper. |
| LightBulb.PlatformInterop.Windows/Internal/WndProcBroadcaster.cs | Broadcasts Win32 messages to listeners. |
| LightBulb.PlatformInterop.Windows/Internal/WndProc.cs | Declares Win32 window procedure delegate. |
| LightBulb.PlatformInterop.Windows/Internal/WndClassEx.cs | Declares Win32 WNDCLASSEX struct. |
| LightBulb.PlatformInterop.Windows/Internal/WinEventProc.cs | Declares WinEvent hook delegate type. |
| LightBulb.PlatformInterop.Windows/Internal/PowerBroadcastSetting.cs | Declares struct for WM_POWERBROADCAST payload marshaling. |
| LightBulb.PlatformInterop.Windows/Internal/NativeResource.cs | Base class for native resource ownership + finalization. |
| LightBulb.PlatformInterop.Windows/Internal/NativeModule.cs | Provides current module handle for registration APIs. |
| LightBulb.PlatformInterop.Windows/Internal/NativeMethods.User32.cs | User32 P/Invoke declarations used by interop layer. |
| LightBulb.PlatformInterop.Windows/Internal/NativeMethods.Kernel32.cs | Kernel32 P/Invoke declarations used by interop layer. |
| LightBulb.PlatformInterop.Windows/Internal/NativeMethods.Gdi32.cs | Gdi32 P/Invoke declarations used by interop layer. |
| LightBulb.PlatformInterop.Windows/Internal/MonitorInfoEx.cs | Declares monitor info struct for GetMonitorInfo. |
| LightBulb.PlatformInterop.Windows/Internal/GammaRamp.cs | Declares gamma ramp struct used for display adjustments. |
| LightBulb.PlatformInterop.Windows/Internal/EnumWindowsProc.cs | Declares window enumeration callback delegate. |
| LightBulb.PlatformInterop.Windows/Internal/EnumMonitorsProc.cs | Declares monitor enumeration callback delegate. |
| LightBulb.PlatformInterop.Windows/Utils/Reg.cs | Adds helper to elevate registry edits via reg.exe. |
| LightBulb.PlatformInterop.Windows/Utils/Disposable.cs | Adds small helper to create disposable unsubscription actions. |
| LightBulb.PlatformInterop.Windows/Utils/Extensions/RegistryExtensions.cs | Adds registry hive helpers (OpenKey, short monikers). |
| LightBulb.PlatformInterop.Linux/LightBulb.PlatformInterop.Linux.csproj | Declares non-Windows interop stub project (net10.0). |
| LightBulb.PlatformInterop.Linux/Window.cs | Linux stub for Window API surface. |
| LightBulb.PlatformInterop.Linux/Timer.cs | Linux implementation of timer abstraction. |
| LightBulb.PlatformInterop.Linux/SystemHook.cs | Linux stub for SystemHook API surface. |
| LightBulb.PlatformInterop.Linux/SystemEvent.cs | Linux stub for SystemEvent API surface. |
| LightBulb.PlatformInterop.Linux/RegistrySwitch.cs | Linux stub for registry switch API surface. |
| LightBulb.PlatformInterop.Linux/Rect.cs | Linux stub rect struct mirroring Windows interop struct. |
| LightBulb.PlatformInterop.Linux/Process.cs | Linux stub for Process API surface. |
| LightBulb.PlatformInterop.Linux/PowerSettingNotification.cs | Linux stub for power notification API surface. |
| LightBulb.PlatformInterop.Linux/Monitor.cs | Linux stub for monitor API surface. |
| LightBulb.PlatformInterop.Linux/MessageBox.cs | Linux stub for error dialog API surface. |
| LightBulb.PlatformInterop.Linux/GlobalHotKey.cs | Linux stub for global hotkey API surface. |
| LightBulb.PlatformInterop.Linux/DeviceContext.cs | Linux stub for device context / gamma API surface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…fix csproj TargetOS condition Agent-Logs-Url: https://github.com/Tyrrrz/LightBulb/sessions/ce5f00bd-fbeb-4886-85eb-ff22f5a51948 Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in 34f26b6. Here's what was changed: Abstraction: Added Linux gamma (GNOME/KDE on X11/XWayland): Wayland note: Pure Wayland sessions (without XWayland) require compositor-specific protocols (wlr-gamma-control for wlroots, KWin's color management API for KDE, etc.). Those can be added as follow-ups once the basic prototype is validated. Build command: dotnet publish LightBulb -p:TargetFramework=net10.0 --runtime linux-x64 --self-containedWarning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot please handle merge conflicts |
Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in a1c4e22. Merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot can you generate me an architectural diagram of the current project state? |
Here's an architectural diagram of the current project state: |
|
@copilot can we rearchitect the solution so that there's another abstraction layer, called Then, we can have the following implementations:
Essentially, for the non-native controllers, we would delegate to the desktop environment to handle native calls for us. As such, they would only be supported where those environments (of proper versions) are installed. For features with conditional support, such as brightness, we can have the abstraction report e.g. The high-level architecture would then look like: LightBulb UI -> LightBulb Core -> LightBulb Platform Interop (includes I think it would also make sense to give the user the option to choose their controller in settings, assuming more that one is available. |
- Add LightBulb.PlatformInterop project with IDisplayGammaController interface and ColorTemperatureConversion helper - Add WindowsNativeDisplayGammaController and DisplayGammaControllerProvider to Windows platform interop - Add LinuxXorgDisplayGammaController, LinuxGnomeDisplayGammaController, LinuxPlasmaDisplayGammaController and DisplayGammaControllerProvider to Linux platform interop - Refactor GammaService to delegate gamma control to IDisplayGammaController (removing direct DeviceContext/Monitor usage and color math) - Rename InvalidateDeviceContexts to InvalidateDisplayConfiguration throughout - Add DisplayGammaControllerId setting to SettingsService - Add controller selection UI to AdvancedSettingsTabViewModel and AdvancedSettingsTabView - Add GammaControllerLabel/GammaControllerTooltip localization strings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Tyrrrz/LightBulb/sessions/8173ea8e-690a-4f12-b5f7-4ac1d999133d Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in 6cde2b4. Here's the new architecture:
GammaService now holds all available controllers, picks the active one by Settings UI: A "Gamma controller" ComboBox appears in the Advanced tab only when more than one controller is available (single-controller platforms show nothing extra). The The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot can the |
Unfortunately
The correct TFM for the Linux interop project remains |
| /// Returns the <see cref="IDisplayGammaController"/> implementations available on Linux, | ||
| /// ordered from most capable (native) to most integrated (DE-specific). | ||
| /// </summary> | ||
| public static class DisplayGammaControllerProvider |
There was a problem hiding this comment.
Rename this class to GammaController
There was a problem hiding this comment.
Done in 59c6871 — both Windows and Linux DisplayGammaControllerProvider classes have been renamed to GammaController.
| public string Id => "linux-gnome"; | ||
| public string DisplayName => "GNOME Night Light"; |
There was a problem hiding this comment.
If these are only required on the UI, then the UI can implement a converter to resolve these
There was a problem hiding this comment.
Done in 59c6871 — DisplayName has been removed from the IDisplayColorController interface and from all implementations. A new DisplayColorControllerToNameConverter in the UI project resolves display names from the controller Id, and the ComboBox in AdvancedSettingsTabView.axaml now uses DisplayMemberBinding="{Binding Converter={x:Static converters:DisplayColorControllerToNameConverter.Instance}}".
| public bool IsBrightnessSupported => false; | ||
|
|
||
| public static bool IsAvailable() => | ||
| (Environment.GetEnvironmentVariable("XDG_CURRENT_DESKTOP") ?? string.Empty).Contains( |
There was a problem hiding this comment.
| (Environment.GetEnvironmentVariable("XDG_CURRENT_DESKTOP") ?? string.Empty).Contains( | |
| Environment.GetEnvironmentVariable("XDG_CURRENT_DESKTOP")?.Contains( |
There was a problem hiding this comment.
Apply this in other implementatnions too
There was a problem hiding this comment.
Done in 59c6871 — applied ?.Contains(...) pattern in both LinuxGnomeDisplayGammaController and LinuxPlasmaDisplayGammaController.
| public string DisplayName => "GNOME Night Light"; | ||
| public bool IsBrightnessSupported => false; | ||
|
|
||
| public static bool IsAvailable() => |
There was a problem hiding this comment.
Done in 59c6871 — IsAvailable() is now a static property IsAvailable on both LinuxGnomeDisplayGammaController and LinuxPlasmaDisplayGammaController.
|
|
||
| public async ValueTask SetGammaAsync(ColorConfiguration configuration) | ||
| { | ||
| var temperature = (int)Math.Round(configuration.Temperature); |
There was a problem hiding this comment.
Done in 59c6871 — the temperature variable is now inlined as ((int)Math.Round(configuration.Temperature)).ToString() directly in the WithArguments call.
…rollers, add IPlatformSettings abstraction Agent-Logs-Url: https://github.com/Tyrrrz/LightBulb/sessions/bc3bf9e4-443f-4ba6-9143-5d27d09b268b Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Tyrrrz/LightBulb/sessions/bc3bf9e4-443f-4ba6-9143-5d27d09b268b Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Summary
Prototype for Linux build support. Introduces a new
IDisplayColorControllerabstraction layer in a sharedLightBulb.PlatformInteropproject, splits the platform interop layer into separate Windows and Linux projects with concrete controller implementations, makesLightBulb.Coretruly cross-platform by movingTargetFrameworkdeclarations into individual projects, guards Windows-only service calls so the app compiles against a non-Windows TFM, and makes the entire interop API async-aware using CliWrap for external command execution.Changes
New project structure
LightBulb.PlatformInteropnet10.0IDisplayColorController,IPlatformSettings,DisplayStateWatcher, shared typesLightBulb.PlatformInterop.Windowsnet10.0-windowsLightBulb.PlatformInterop.Linuxnet10.0IDisplayColorControllerabstractionA new
IDisplayColorControllerinterface (inLightBulb.PlatformInterop) decouples gamma-setting logic from the service layer. Each controller reports its availability and capabilities (e.g.IsBrightnessSupported) and accepts a color configuration:Display names for controllers are resolved in the UI layer via a
DisplayColorControllerToNameConverterconverter.Implementations
WindowsGdiNativeDisplayGammaControllerSetDeviceGammaRampviaDeviceContextLinuxXorgDisplayGammaControllerxrandr --gammavia CliWrap (X11/XWayland)LinuxGnomeDisplayGammaControllerLinuxPlasmaDisplayGammaControllerThe appropriate controller set is surfaced by
GammaController(previouslyDisplayGammaControllerProvider) in each platform interop project. When more than one controller is available, the user can select their preferred controller in Settings → Advanced.IPlatformSettingsabstractionA new
IPlatformSettingsinterface (inLightBulb.PlatformInterop) replaces the Windows-onlyRegistrySwitchno-op stubs on Linux. Both platform projects provide aPlatformSettingsimplementation:SettingsServicenow injectsIPlatformSettingsinstead of directly using Win32 registry types, eliminating all no-op Linux stubs.ColorTemperaturemoved toLightBulb.CoreColorTemperatureConversion(previously inLightBulb.PlatformInterop) has been renamed toColorTemperatureand moved toLightBulb.Core, making color temperature math available to the core domain layer without a platform interop dependency.Conditional reference in
LightBulb.csprojLightBulbalways referencesLightBulb.PlatformInterop(shared) and conditionally references the platform-specific project via a$(TargetOS)property derived from$(TargetPlatformIdentifier):TFM declarations moved to individual projects
TargetFrameworkremoved fromDirectory.Build.props; each project now declares its own:LightBulb→net10.0-windows(default; override with-p:TargetFramework=net10.0for Linux)LightBulb.Core,LightBulb.Core.Tests→net10.0(cross-platform)LightBulb.PlatformInterop→net10.0(cross-platform)LightBulb.PlatformInterop.Windows→net10.0-windowsLightBulb.PlatformInterop.Linux→net10.0DisplayStateWatcherabstractionA
DisplayStateWatcherclass (inLightBulb.PlatformInterop) encapsulates all platform-specific display/power event registrations. The Windows implementation wrapsPowerSettingNotification,SystemHook, andSystemEvent. The Linux stub is a no-op.GammaServiceuses the high-level API:Controller selection in Settings
When more than one controller is available on the current platform, an Advanced Settings picker allows the user to select their preferred
IDisplayColorController. The setting is persisted asDisplayColorControllerIdinsettings.json. UI elements tied to capabilities (e.g. brightness slider) are enabled/disabled based onIsBrightnessSupportedof the active controller.Async-aware API with CliWrap
All external command execution uses CliWrap instead of
System.Diagnostics.Process:LinuxXorgDisplayGammaController: Callsxrandr --output <name> --gamma r:g:bconcurrently across all connected outputs viaTask.WhenAll.GammaService:SetGammaAsync+EnsureValidDeviceContextsAsyncpropagate async end-to-end;Disposeuses.ContinueWith(OnlyOnFaulted)for safe best-effort gamma reset.DashboardViewModel.UpdateConfiguration:async voidwith a top-level try-catch to prevent unobserved exceptions.Platform guards in services
SettingsService.csandUpdateService.csguard Windows-only API calls withOperatingSystem.IsWindows()to satisfy CA1416 when compiling againstnet10.0.Linux
MessageBoxfallbackMessageBox.ShowErroron Linux writes toConsole.ErrorandTrace.TraceErrorso fatal startup exceptions are never silently discarded.Building for Linux