Align Acrylic10 tint behavior with official dark/light acrylic palette - #1915
Conversation
Agent-Logs-Url: https://github.com/QL-Win/QuickLook/sessions/7856deeb-578c-4683-879b-e2d924eb4224 Co-authored-by: emako <24737061+emako@users.noreply.github.com>
Agent-Logs-Url: https://github.com/QL-Win/QuickLook/sessions/7856deeb-578c-4683-879b-e2d924eb4224 Co-authored-by: emako <24737061+emako@users.noreply.github.com>
Agent-Logs-Url: https://github.com/QL-Win/QuickLook/sessions/7856deeb-578c-4683-879b-e2d924eb4224 Co-authored-by: emako <24737061+emako@users.noreply.github.com>
Reviewer's GuideAligns the Acrylic10 backdrop’s tint color and opacity with official dark/light acrylic palette values while preserving existing behavior for other backdrop modes and maintaining custom background overrides. Sequence diagram for Acrylic10 backdrop application with new tint palette and opacitysequenceDiagram
participant VW as ViewerWindow
participant SH as SettingHelper
participant WH as WindowHelper
VW->>VW: ApplyBackdrop(backdrop)
alt backdrop is Acrylic10 and OS is Win10 or Win11
VW->>VW: GetAcrylic10TintColor()
VW->>SH: Get(WindowBackgroundColor)
alt custom color present and valid
VW-->>VW: use custom color as acrylicTint
else custom color missing or invalid
VW-->>VW: use Acrylic10DarkTintColor or Acrylic10LightTintColor based on CurrentTheme
end
VW->>WH: EnableAcrylicBlur(this, acrylicTint, isDarkTheme, Acrylic10TintOpacity)
VW-->>VW: Background = Transparent
else other backdrop types
VW-->>VW: use existing backdrop logic
end
Class diagram for updated ViewerWindow and WindowHelper acrylic handlingclassDiagram
class ViewerWindow {
- double Acrylic10TintOpacity
- Color Acrylic10DarkTintColor
- Color Acrylic10LightTintColor
- Size _customWindowSize
- bool _ignoreNextWindowSizeChange
- string _path
- Themes CurrentTheme
+ void ApplyBackdrop(SystembackdropType backdrop)
- Color GetAcrylicTintColor()
- Color GetAcrylic10TintColor()
- void ApplyWindowBackgroundEffects()
}
class WindowHelper {
+ static void EnableBlur(Window window)
+ static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme, double tintOpacity)
+ static void DisableDwmBlur(Window window)
}
class SystembackdropType {
<<enum>>
Auto
None
Mica
Acrylic
Acrylic10
Acrylic11
Tabbed
}
class Themes {
<<enum>>
Light
Dark
}
ViewerWindow --> WindowHelper : uses
ViewerWindow ..> SystembackdropType : selects
ViewerWindow ..> Themes : reads
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The custom color retrieval and parsing logic in
GetAcrylicTintColor()andGetAcrylic10TintColor()is duplicated; consider extracting a shared helper (e.g.,TryGetCustomWindowBackgroundColor()) so future changes to this behavior stay consistent in one place. - In
WindowHelper.EnableAcrylicBlur, you might want to clamp or validate thetintOpacityparameter to the expected 0–1 range so that accidental out-of-range values do not lead to unexpected visual behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The custom color retrieval and parsing logic in `GetAcrylicTintColor()` and `GetAcrylic10TintColor()` is duplicated; consider extracting a shared helper (e.g., `TryGetCustomWindowBackgroundColor()`) so future changes to this behavior stay consistent in one place.
- In `WindowHelper.EnableAcrylicBlur`, you might want to clamp or validate the `tintOpacity` parameter to the expected 0–1 range so that accidental out-of-range values do not lead to unexpected visual behavior.
## Individual Comments
### Comment 1
<location path="QuickLook.Common/Helpers/WindowHelper.cs" line_range="171-174" />
<code_context>
Marshal.FreeHGlobal(accentPtr);
}
- public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme)
+ public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme, double tintOpacity = 0.8)
{
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider validating or clamping the `tintOpacity` parameter before passing it to `ToAbgr`.
Since `tintOpacity` is now a public parameter, callers can pass values outside the expected [0, 1] range. If `ToAbgr` assumes a normalized value, consider clamping or validating `tintOpacity` in `EnableAcrylicBlur` (and falling back to the default) to avoid invalid alpha values or unexpected visuals for inputs like -0.1 or 1.5.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme) | ||
| public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme, double tintOpacity = 0.8) | ||
| { | ||
| window.Background = Brushes.Transparent; | ||
|
|
There was a problem hiding this comment.
suggestion (bug_risk): Consider validating or clamping the tintOpacity parameter before passing it to ToAbgr.
Since tintOpacity is now a public parameter, callers can pass values outside the expected [0, 1] range. If ToAbgr assumes a normalized value, consider clamping or validating tintOpacity in EnableAcrylicBlur (and falling back to the default) to avoid invalid alpha values or unexpected visuals for inputs like -0.1 or 1.5.
There was a problem hiding this comment.
Pull request overview
Aligns the Win10 Acrylic (“Acrylic10”) backdrop tint and opacity with the specified dark/light palette values while keeping other backdrop modes’ behavior unchanged.
Changes:
- Adds Acrylic10-specific tint colors (dark/light) and uses them when no custom
WindowBackgroundColoroverride is provided. - Introduces an optional
tintOpacityparameter toWindowHelper.EnableAcrylicBlur(...)and uses0.7specifically for Acrylic10. - Narrows invalid color parsing catches to specific exception types (via exception filters).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| QuickLook/ViewerWindow.xaml.cs | Adds Acrylic10 tint palette + opacity constant and routes Acrylic10 through a dedicated tint-color resolver. |
| QuickLook.Common/Helpers/WindowHelper.cs | Extends EnableAcrylicBlur to accept a tint opacity and applies it when building the ABGR gradient color. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| accent.AccentState = AccentState.AccentEnableAcrylicblurbehind; | ||
| accent.GradientColor = ToAbgr(tintColor, 0.8); | ||
| accent.GradientColor = ToAbgr(tintColor, tintOpacity); | ||
|
|
There was a problem hiding this comment.
The new tintOpacity parameter is used directly in ToAbgr(tintColor, tintOpacity) without validation. Values outside [0,1] can overflow/truncate the alpha channel during the << 24 shift in ToAbgr, producing incorrect gradient colors. Consider clamping tintOpacity to [0, 1] (or clamping the computed alpha to byte range) before calling ToAbgr.
| catch (Exception ex) when (ex is FormatException || ex is NotSupportedException) | ||
| { | ||
| // Ignore invalid color | ||
| } |
There was a problem hiding this comment.
This catch (Exception ex) when (...) declares ex but never uses it. To avoid unused-variable analyzer warnings and keep the intent clearer, consider either catch (Exception) when (...) or catching FormatException / NotSupportedException explicitly.
| catch (Exception ex) when (ex is FormatException || ex is NotSupportedException) | |
| { | |
| // Ignore invalid color | |
| } | |
| catch (FormatException) | |
| { | |
| // Ignore invalid color | |
| } | |
| catch (NotSupportedException) | |
| { | |
| // Ignore invalid color | |
| } |
| catch (Exception ex) when (ex is FormatException || ex is NotSupportedException) | ||
| { |
There was a problem hiding this comment.
This catch (Exception ex) when (...) declares ex but never uses it. To avoid unused-variable analyzer warnings and keep the intent clearer, consider either catch (Exception) when (...) or catching FormatException / NotSupportedException explicitly.
| catch (Exception ex) when (ex is FormatException || ex is NotSupportedException) | ||
| { |
There was a problem hiding this comment.
This catch (Exception ex) when (...) declares ex but never uses it. To avoid unused-variable analyzer warnings and keep the intent clearer, consider either catch (Exception) when (...) or catching FormatException / NotSupportedException explicitly.
| private Color GetAcrylic10TintColor() | ||
| { | ||
| var customColor = SettingHelper.Get("WindowBackgroundColor", string.Empty, "QuickLook"); | ||
|
|
||
| if (!string.IsNullOrEmpty(customColor)) | ||
| { | ||
| try | ||
| { | ||
| return ((SolidColorBrush)new BrushConverter().ConvertFromString(customColor)).Color; | ||
| } | ||
| catch (Exception ex) when (ex is FormatException || ex is NotSupportedException) | ||
| { | ||
| // Ignore invalid color | ||
| } | ||
| } |
There was a problem hiding this comment.
GetAcrylic10TintColor() duplicates the custom color override parsing logic from GetAcrylicTintColor(). Since both read the same setting and have identical parsing/exception handling, consider factoring this into a shared helper (e.g., a method that tries to parse WindowBackgroundColor and returns a nullable Color) to avoid future divergence between the two code paths.
| public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme, double tintOpacity = 0.8) | ||
| { |
There was a problem hiding this comment.
WindowHelper.EnableAcrylicBlur is a public method in QuickLook.Common; changing its signature by adding a parameter is a binary breaking change for any existing plugins/binaries compiled against the old 3-arg method. To preserve backward compatibility, keep the original overload EnableAcrylicBlur(Window, Color, bool) and have it call the new overload with the default opacity (0.8).
| public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme, double tintOpacity = 0.8) | |
| { | |
| public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme) | |
| { | |
| EnableAcrylicBlur(window, tintColor, isDarkTheme, 0.8); | |
| } | |
| public static void EnableAcrylicBlur(Window window, Color tintColor, bool isDarkTheme, double tintOpacity) | |
| { |
Acrylic10 in QuickLook did not match the issue’s target visual parameters for dark/light mode. This change aligns Acrylic10 tint color and tint opacity with the specified official values while keeping non-Acrylic10 backdrop behavior unchanged.
Acrylic10 tint now follows official dark/light values
ViewerWindow:#171717#F2F2F2SystembackdropType.Acrylic10now uses this palette viaGetAcrylic10TintColor().Acrylic10 tint opacity set to 0.70
WindowHelper.EnableAcrylicBlur(...)with an optionaltintOpacityparameter (default remains0.8to preserve existing callers).EnableAcrylicBlur(..., tintOpacity: 0.7).No functional drift for other backdrop modes
WindowBackgroundColoroverride remains supported and still takes precedence when valid.Summary by Sourcery
Align Acrylic10 backdrop appearance with the official dark/light acrylic palette while preserving existing behavior for other backdrops.
New Features:
Enhancements: