Conversation
|
Let's move away from using words related to "dimming" here, since it's really just adjusting the screen brightness. Please update all references to use "brightness" or "display brightness" instead. Otherwise, I just have one comment inline. Thanks for taking this on! |
| const fragmentMain = ` | ||
| cogl_color_out.rgb *= u_display_dimming; | ||
| `; |
There was a problem hiding this comment.
Would it be more appropriate to adjust the alpha layer?
There was a problem hiding this comment.
Pull request overview
This pull request adds a screen dimming feature to address the issue where the minimum brightness setting was too bright for nighttime use. The implementation adds a brightness control slider (0-100%) that dims the display by scaling RGB values in the shader, providing a software-based dimming solution as an alternative to the problematic AlphaTint extension.
Key changes:
- Added display brightness slider controls to both GTK and Qt user interfaces
- Implemented shader-based RGB dimming in both KWin (Qt/QML) and GNOME (JavaScript) rendering pipelines
- Added settings storage and synchronization across desktop environments
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ui/src/gtk/connected-device.ui | Added display brightness slider widget with 0-100% range and visual markers |
| ui/src/connecteddevice.py | Implemented settings binding with UI (0-100) to storage (0.0-1.0) conversion |
| ui/data/com.xronlinux.BreezyDesktop.gschema.xml | Added GSettings schema for display-dimming as double (0.0-1.0) with default 1.0 |
| kwin/src/qml/cursorOverlay.frag | Applied dimming by multiplying RGB values while preserving alpha |
| kwin/src/qml/BreezyDesktopDisplay.qml | Added displayDimming property binding to shader |
| kwin/src/kcm/breezydesktopeffectkcm.ui | Added Qt display brightness slider to KDE control module, reindexed subsequent rows |
| kwin/src/breezydesktopeffect.h | Added displayDimming property declaration and member variable |
| kwin/src/breezydesktopeffect.cpp | Implemented displayDimming configuration loading and change notification |
| kwin/src/breezydesktopconfig.kcfg | Added DisplayDimming integer setting (0-100) for KDE configuration |
| gnome/src/virtualdisplaysactor.js | Added display-dimming property and bound it to virtual display effects |
| gnome/src/virtualdisplayeffect.js | Implemented fragment shader dimming and uniform handling for GNOME |
| gnome/src/extension.js | Added display-dimming to settings bindings array |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.settings.bind('monitor-spacing', self.monitor_spacing_adjustment, 'value', Gio.SettingsBindFlags.DEFAULT) | ||
| self.settings.bind('viewport-offset-x', self.viewport_offset_x_adjustment, 'value', Gio.SettingsBindFlags.DEFAULT) | ||
| self.settings.bind('viewport-offset-y', self.viewport_offset_y_adjustment, 'value', Gio.SettingsBindFlags.DEFAULT) | ||
|
|
There was a problem hiding this comment.
There is inconsistent whitespace formatting - a trailing whitespace exists on this otherwise empty line. This should be removed to maintain code cleanliness and consistency with project standards.
In my application, I found the minimum brightness on my glasses to be too bright at night.
I tried the gnome extension AlphaTint, but it completely wigged out and caused flickering windows when there were multiple screens.