-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add WindowCornerHintsProperty #17354
base: master
Are you sure you want to change the base?
Conversation
You can test this PR using the following package version. |
This API makes sense to me as part of Win32Properties class: https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.Controls/Platform/Win32Properties.cs#L17. Not xplat. |
ae30832
to
a996eda
Compare
Oh, I didn't know about |
a996eda
to
265fe2e
Compare
You can test this PR using the following package version. |
{ | ||
public static readonly AttachedProperty<Win32WindowCornerHints> WindowCornerHintProperty = | ||
AvaloniaProperty.RegisterAttached<Win32Properties, Window, Win32WindowCornerHints>("WindowCornerHint"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we just make this a 3-state bool value? This property is ONLY controlling a window corner radius. This means we need to control whether it is ON or OFF. Then it's probably useful to have a "default" condition as well.
bool? could communicate:
- null : Platform default
- true : Round corners
- false : Don't round corners
Will there ever be a reason to have more values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's a windows specific property, as it is now, it should mirror windows API.
If we assume it shouldn't be windows specific one day, it can either be made a 3-state bool later. But it's unlikely to be supported on wider range of platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's a windows specific property, as it is now, it should mirror windows API.
Good point. That's the right way to look at it I think so your probably right.
I also notice there are more values supported: https://learn.microsoft.com/en-us/windows/win32/api/dwmapi/ne-dwmapi-dwm_window_corner_preference. Namely RoundSmall
. So an enum is the way to go.
/// <summary> | ||
/// Prevents corners from being rounded. | ||
/// </summary> | ||
DoNotRound |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of this name. I think the whole enum should go away but "NotRounded" is probably better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, changed it for consistency, thanks!
NoHint, | ||
|
||
/// <summary> | ||
/// The platform's default corner style. | ||
/// </summary> | ||
PlatformDefault, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NoHint
is exactly the same as PlatformDefault
. We shouldn't have two values here. These should be combined into a Default
value. Default
means no hint is given so the platform defaults will be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the current behavior:
Avalonia/src/Windows/Avalonia.Win32/WindowImpl.cs
Lines 1150 to 1174 in 0bd90d8
if (_isClientAreaExtended && WindowState != WindowState.FullScreen) | |
{ | |
var margins = UpdateExtendMargins(); | |
DwmExtendFrameIntoClientArea(_hwnd, ref margins); | |
unsafe | |
{ | |
int cornerPreference = (int)DwmWindowCornerPreference.DWMWCP_ROUND; | |
DwmSetWindowAttribute(_hwnd, (int)DwmWindowAttribute.DWMWA_WINDOW_CORNER_PREFERENCE, &cornerPreference, sizeof(int)); | |
} | |
} | |
else | |
{ | |
var margins = new MARGINS(); | |
DwmExtendFrameIntoClientArea(_hwnd, ref margins); | |
_offScreenMargin = new Thickness(); | |
_extendedMargins = new Thickness(); | |
unsafe | |
{ | |
int cornerPreference = (int)DwmWindowCornerPreference.DWMWCP_DEFAULT; | |
DwmSetWindowAttribute(_hwnd, (int)DwmWindowAttribute.DWMWA_WINDOW_CORNER_PREFERENCE, &cornerPreference, sizeof(int)); | |
} | |
} |
So it is DWMWCP_ROUND
when _isClientAreaExtended
is true and window state is not full screen. Otherwise it uses DWMWCP_DEFAULT
.
This is because DWMWCP_DEFAULT with client area extended results in not rounded corners.
This is also why I've split the enum into NoHint
(aka avalonia default) and Default
(aka platform default). However, I could agree than maybe Default
aka "Platform Default" is not useful in practice. Avalonia default is good for 99.9% programmers and for the small percentage the choice between "rounded" and "not rounded" it probably enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading, I think we need the following values: Default, Rounded, RoundedSmall, NotRounded (Alternative: Round, RoundSmall, DoNotRound following upstream API)
You can test this PR using the following package version. |
[Flags] | ||
public enum Win32WindowCornerHints |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this flags? And plural naming?
Reading the docs I don't think multiple values can be set at once. So it's isn't treated as flags.
https://learn.microsoft.com/en-us/windows/win32/api/dwmapi/ne-dwmapi-dwmwindowattribute
https://learn.microsoft.com/en-us/windows/win32/api/dwmapi/ne-dwmapi-dwm_window_corner_preference
Perhaps the name should be Win32WindowCornerPreference
as well. Although, I think it's supported only on Windows 11+ so calling it "Hints" might be appropriate as its ignored on platforms that don't support it.
What does the pull request do?
This PR adds a new
StyledProperty
to theWindow
class:WindowCornerHintsProperty
, which provides hints for window corner appearance. The hint is only used on Windows, as only this platform supports controlling corner appearance.This is required for Classic.Avalonia theme to look good.
What is the current behavior?
Currently, there is no way to control window corners; it is up to Avalonia to decide which corner style to use.
What is the updated/expected behavior with this PR?
When
WindowCornerHintsProperty
is set todefault platform
,round
, ordo not round
, the appropriate corner preference is used. However, by default,WindowCornerHintsProperty
has the valueNoHint
, which indicates that it is up to Avalonia to decide which corner style should be used. This way, the default behavior is the same as it is now.How was the solution implemented (if it's not obvious)?
This PR follows the implementation of
ExtendClientAreaChromeHintsProperty
.Checklist
Breaking changes
Obsoletions / Deprecations
Fixed issues
Fixes #17294