Fix blur theme backdrop, corners, margin issues & simplify AutoDropShadow#4513
Conversation
9b5603b to
6f6d5fc
Compare
There was a problem hiding this comment.
2 issues found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughExtends DWM backdrop into the full client area, always applies blur before drop-shadow during frame refresh, and adjusts theme resource/effect handling to avoid double-corner rendering and prevent repeated margin shrinking. ChangesBlur Theme Double-Corner Fix
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
65-78:⚠️ Potential issue | 🟠 MajorRetract DWM client-area frame on
BackdropTypes.Noneand includeDwmExtendFrameIntoClientAreasuccess in the return value
DWMSetBackdropForWindow(...)callsDwmExtendFrameIntoClientArea(...)only whenbackdrop is not BackdropTypes.None(using-1margins), so switching toBackdropTypes.Nonenever retracts the previous client-area extension. Also, the method’sboolreturn currently reflects onlyDwmSetWindowAttribute(...); theDwmExtendFrameIntoClientArea(...)result is ignored.Suggested fix
public static unsafe bool DWMSetBackdropForWindow(Window window, BackdropTypes backdrop) { + var hwnd = GetWindowHandle(window); var backdropType = backdrop switch { BackdropTypes.Acrylic => DWM_SYSTEMBACKDROP_TYPE.DWMSBT_TRANSIENTWINDOW, BackdropTypes.Mica => DWM_SYSTEMBACKDROP_TYPE.DWMSBT_MAINWINDOW, BackdropTypes.MicaAlt => DWM_SYSTEMBACKDROP_TYPE.DWMSBT_TABBEDWINDOW, _ => DWM_SYSTEMBACKDROP_TYPE.DWMSBT_AUTO }; - // The backdrop renders in the non-client frame area. WindowStyle=None - // removes that area, so we extend it across the entire client area. - // https://learn.microsoft.com/en-us/windows/win32/api/dwmapi/nf-dwmapi-dwmextendframeintoclientarea - if (backdrop is not BackdropTypes.None) - { - var margins = new MARGINS { cxLeftWidth = -1, cxRightWidth = -1, cyTopHeight = -1, cyBottomHeight = -1 }; - PInvoke.DwmExtendFrameIntoClientArea(GetWindowHandle(window), in margins); - } + var margins = backdrop is BackdropTypes.None + ? new MARGINS() + : new MARGINS { cxLeftWidth = -1, cxRightWidth = -1, cyTopHeight = -1, cyBottomHeight = -1 }; + + var extendResult = PInvoke.DwmExtendFrameIntoClientArea(hwnd, in margins); - return PInvoke.DwmSetWindowAttribute( - GetWindowHandle(window), + return extendResult.Succeeded && PInvoke.DwmSetWindowAttribute( + hwnd, DWMWINDOWATTRIBUTE.DWMWA_SYSTEMBACKDROP_TYPE, &backdropType, (uint)Marshal.SizeOf<int>()).Succeeded; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher.Infrastructure/Win32Helper.cs` around lines 65 - 78, The method currently only extends the frame when backdrop != BackdropTypes.None and ignores that call's result; update the logic in DWMSetBackdropForWindow so that when backdrop == BackdropTypes.None you call PInvoke.DwmExtendFrameIntoClientArea(GetWindowHandle(window), in margins) with margins set to zeros to retract the extension, capture the HRESULT/return value of that call, and then return a combined success boolean that requires both the DwmExtendFrameIntoClientArea call and the subsequent PInvoke.DwmSetWindowAttribute(GetWindowHandle(window), DWMWINDOWATTRIBUTE.DWMWA_SYSTEMBACKDROP_TYPE, &backdropType, (uint)Marshal.SizeOf<int>()) to have Succeeded; use the existing GetWindowHandle(window), backdropType and DWMWINDOWATTRIBUTE symbols to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Flow.Launcher.Core/Resource/Theme.cs`:
- Around line 741-745: The blur-theme branch removed the WindowBorderStyle
override but forgot to restore the resize border cleanup that
RemoveDropShadowEffectFromCurrentTheme() used to perform; update the same block
that calls SetWindowCornerPreference("Round") and removes "WindowBorderStyle" to
also call SetResizeBoarderThickness(null) (or invoke
RemoveDropShadowEffectFromCurrentTheme() if that function contains the correct
cleanup) so the enlarged resize border is cleared when switching to blur themes.
---
Outside diff comments:
In `@Flow.Launcher.Infrastructure/Win32Helper.cs`:
- Around line 65-78: The method currently only extends the frame when backdrop
!= BackdropTypes.None and ignores that call's result; update the logic in
DWMSetBackdropForWindow so that when backdrop == BackdropTypes.None you call
PInvoke.DwmExtendFrameIntoClientArea(GetWindowHandle(window), in margins) with
margins set to zeros to retract the extension, capture the HRESULT/return value
of that call, and then return a combined success boolean that requires both the
DwmExtendFrameIntoClientArea call and the subsequent
PInvoke.DwmSetWindowAttribute(GetWindowHandle(window),
DWMWINDOWATTRIBUTE.DWMWA_SYSTEMBACKDROP_TYPE, &backdropType,
(uint)Marshal.SizeOf<int>()) to have Succeeded; use the existing
GetWindowHandle(window), backdropType and DWMWINDOWATTRIBUTE symbols to locate
the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fd603f2-aef7-4031-9480-c557f3d25b91
📒 Files selected for processing (3)
Flow.Launcher.Core/Resource/Theme.csFlow.Launcher.Infrastructure/NativeMethods.txtFlow.Launcher.Infrastructure/Win32Helper.cs
64a50d8 to
68b9169
Compare
…opShadowEffectFromCurrentTheme
The method was loading raw theme XAML and directly setting WindowBorderStyle, always overriding SetBlurForWindow's transparent background and CornerRadius=0 Should fix #4457
…der reset from AutoDropShadow to SetBlurForWindow - AutoDropShadow is now simplified to only handle adding or removing the drop shadow effect. - The WindowBorderStyle removal, and resize border reset were just cleanup for SetBlurForWindow so make more sense to be there. - The Corner prefernce was only really a binary choice between blur enabled or not - so SetBlurForWindow can also handle that.
06389fe to
7cf0b97
Compare
Closes #4457 and #4512
Potentially also closes #3624
Bug Fixes
RemoveDropShadowEffectFromCurrentThemefor blur themes as they both made conflicting modifications to the style - instead just clear all window border styles in theSetBlurForWindowmethodRemoveDropShadowEffectFromCurrentThemeso that it only removes the drop shadow margin if necessary - avoiding shrinking of the main windowDwmExtendFrameIntoClientAreawhen using blur themes so that removal of non client area byWindowStyle=Noneis not an issueMinor Refactor
Simplified
AutoDropShadowby moving any non drop shadow related code toSetBlurForWindow, including the cleanup for it in the now removed BlurEnabled branch but also the DWM corner prefernce as that only needs to consider if the theme has blur or not.Summary by cubic
Fixes blur/backdrop and drop shadow interactions so Windows themes render correctly. Mica/Acrylic now cover the full window with
WindowStyle=None; corners, margins, and resize borders are correct.Summary of changes
AutoDropShadowafterSetBlurForWindow; it only adds/removes the effect and no-ops on blur.SetBlurForWindownow sets corner preference (Round for blur, Default otherwise), clears any directly setWindowBorderStyle, and resets resize border to system default.RemoveDropShadowEffectFromCurrentThemesubtracts shadow margin only when anEffectsetter exists to avoid cumulative shrink.DwmExtendFrameIntoClientArea(MARGINS = -1) so backdrops render across the entire window.RemoveDropShadowEffectFromCurrentThemefor blur themes; removed stale commented code inRefreshFrameAsync.Release Note
Windows blur themes now render full-window Mica/Acrylic with correct corners, shadows, margins, and resize borders.
Written for commit 7cf0b97. Summary will update on new commits.