Screens API refactor #16295

maxkatz6 · 2024-07-12T09:55:14Z

What does the pull request do?

Screens API now consistently returns the same instances of Screen class for the same physical screen. Previously it would return new instances each time.
On physical screen changes, previously retrieved Screen instances now also get updated. No need to re-read all screens each time.
New event added that is raised on any screen changed or added (no fine tuning here, sorry, maybe in the future).
Adds an API to retrieve screen platform handle, where possible.
+DisplayName and +CurrentOrientation properties on screen.
Screen is accessible from the TopLevel, not only Window.
Adds implementations for all platforms (in this PR - only Windows and MacOS to keep it simpler, Linux is skipped for now)

I skipped Linux because I have no idea how it works and can't properly test it locally.
Which also means, Linux still returns new Screen instance on each read, basically an old behavior.

See #15901 for details

Updated API:

public class WindowBase : TopLevel
{
-    public Screens Screens { get; }
+    public new Screens Screens => base.Screens!;
}
public class TopLevel
{
+    public Screens? Screens { get; }
}

public class Screen
{
+    public string? DisplayName { get; }
+    public ScreenOrientation CurrentOrientation { get; }

+    public IPlatformHandle? TryGetPlatformHandle();
}

public class Screens
{
+    public event EventHandler? Changed;
+    public Screen? ScreenFromTopLevel(TopLevel topLevel);
+    Task<bool> RequestScreenDetails();
}

+public enum ScreenOrientation
+{
+    None,
+    Landscape,
+    Portrait,
+    LandscapeFlipped,
+    PortraitFlipped
+}

Checklist

Added unit tests (if possible)?
Added XML documentation to any related classes?
Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Breaking changes

ctor.Screens(IScreenImpl) is now PrivateAPI. It should not break any app, as IScreenImpl by itself was a private API since 11.0.

Fixed issues

#15382
#11512
#15637
#15901

…es of backend, use Microsoft.Windows.CsWin32 for interop

…ayID

maxkatz6 · 2024-07-12T09:59:15Z

One breaking change that I would love to change, but can't - make Bounds and WorkArea nullable, and instead add non-nullable Size. Because we can get Screen.Size pretty much on any platform, but screen position and work area are only available on desktop + desktop chromium.

public class Screen
{
+     public PixelSize Size { get; }
-     public PixelRect Bounds { get; }
+     public PixelRect? Bounds { get; }
-     public PixelRect WorkArea { get; }
+     public PixelRect? WorkArea { get; }
}

We might move this specific change to 12.0, cc @kekekeks

src/Windows/Avalonia.Win32/Avalonia.Win32.csproj

native/Avalonia.Native/src/OSX/Screens.mm

maxkatz6 · 2024-07-12T10:05:17Z

src/Avalonia.Base/Compatibility/UnmanagedCallersOnlyAttribute.cs

+namespace System.Runtime.InteropServices
+{
+    [global::System.AttributeUsage(global::System.AttributeTargets.Method, Inherited = false)]
+    internal sealed class UnmanagedCallersOnlyAttribute : global::System.Attribute


FYI: I used function pointers in netstandard2.0, and it even works on net framework.

Have you checked with x86 arch? It's the one with weird calling conventions.

IIRC one of the reasons for that attribute was to ensure that the function has a proper native->managed transition and is even callable with the corresponding calling convention.

.NET Framework isn't aware of the attribute and isn't likely to setup a proper native->managed transition.

So I believe that it's not safe to do.

maxkatz6 · 2024-07-13T01:32:33Z

native/Avalonia.Native/src/OSX/Screens.mm

@@ -43,14 +66,44 @@ virtual HRESULT GetScreen (int index, AvnScreen* ret) override

            ret->Scaling = 1;


Does anybody remember why we set scaling to 1? Instead of using an actual value (more likely between 2 and 3 for most main displays). MacOS.

Our Screen API reports layout scaling, not DPI. macOS always uses logical coordinates for window positioning, so the reported value is 1.
We could extend the API to report both.

Our Screen API reports layout scaling

Is it used anywhere like that? It's also not intuitive.
Normally people would use Window.DesktopScaling for that.

Other implementations like qtbase and chromium just return dpi scaling, not layout.

Will keep it as is for now

maxkatz6 · 2024-07-13T01:32:58Z

Added some screen integration tests as well. PR is ready.

avaloniaui-bot · 2024-07-13T01:50:30Z

You can test this PR using the following package version. 11.2.999-cibuild0050061-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

avaloniaui-bot · 2024-07-13T02:26:27Z

You can test this PR using the following package version. 11.2.999-cibuild0050079-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

native/Avalonia.Native/src/OSX/common.h

native/Avalonia.Native/src/OSX/Screens.mm

kekekeks · 2024-07-13T09:46:28Z

native/Avalonia.Native/src/OSX/Screens.mm

@@ -43,14 +66,44 @@ virtual HRESULT GetScreen (int index, AvnScreen* ret) override

            ret->Scaling = 1;


Our Screen API reports layout scaling, not DPI. macOS always uses logical coordinates for window positioning, so the reported value is 1.
We could extend the API to report both.

native/Avalonia.Native/src/OSX/Screens.mm

kekekeks · 2024-07-13T09:52:16Z

src/Avalonia.Base/Compatibility/UnmanagedCallersOnlyAttribute.cs

+namespace System.Runtime.InteropServices
+{
+    [global::System.AttributeUsage(global::System.AttributeTargets.Method, Inherited = false)]
+    internal sealed class UnmanagedCallersOnlyAttribute : global::System.Attribute


Have you checked with x86 arch? It's the one with weird calling conventions.

kekekeks · 2024-07-13T10:07:16Z

src/Avalonia.Base/Compatibility/UnmanagedCallersOnlyAttribute.cs

+namespace System.Runtime.InteropServices
+{
+    [global::System.AttributeUsage(global::System.AttributeTargets.Method, Inherited = false)]
+    internal sealed class UnmanagedCallersOnlyAttribute : global::System.Attribute


So I believe that it's not safe to do.

src/Avalonia.Controls/Platform/Screen.cs

src/Avalonia.Controls/Primitives/PopupPositioning/ManagedPopupPositionerPopupImplHelper.cs

src/Avalonia.Native/ScreenImpl.cs

src/Windows/Avalonia.Win32/Interop/UnmanagedMethods.cs

avaloniaui-bot · 2024-07-16T05:15:36Z

You can test this PR using the following package version. 11.2.999-cibuild0050132-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

…onitors

…hout changing base Screen

maxkatz6 · 2024-07-16T07:09:36Z

@kekekeks removed incorrect UnmanagedCallersOnly usage, changed macOS impl to use com pointer, and moved GetHashCode/Equals impl to another class that inherits Screen.

avaloniaui-bot · 2024-07-16T07:39:27Z

You can test this PR using the following package version. 11.2.999-cibuild0050150-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

avaloniaui-bot · 2024-07-16T19:19:11Z

You can test this PR using the following package version. 11.2.999-cibuild0050178-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

* Draft new API * Push reusable ScreensBaseImpl implementation * Fix tests and stubs * Update ScreensPage sample to work on mobile + show new APIs * Reimplement Windows ScreensImpl, reuse existing screens in other places of backend, use Microsoft.Windows.CsWin32 for interop * Make X11 project buildable, don't utilize new APIs yet * Reimplement macOS Screens API, differenciate screens by CGDirectDisplayID * Fix build * Adjust breaking changes file (none affect users) * Fix missing macOS Screen.DisplayName * Add more tests + fix screen removal * Add screens integration tests * Use hash set with comparer when removing screens * Make screenimpl safer on macOS as per review * Replace UnmanagedCallersOnly usage with source generated EnumDisplayMonitors * Remove unused dllimport * Only implement GetHashCode and Equals on PlatformScreen subclass, without changing base Screen # Conflicts: # api/Avalonia.nupkg.xml

maxkatz6 added 9 commits July 12, 2024 02:28

Draft new API

a260fb4

Push reusable ScreensBaseImpl implementation

0f20b9e

Fix tests and stubs

2457e5f

Update ScreensPage sample to work on mobile + show new APIs

441857b

Reimplement Windows ScreensImpl, reuse existing screens in other plac…

11cab6b

…es of backend, use Microsoft.Windows.CsWin32 for interop

Make X11 project buildable, don't utilize new APIs yet

8a1f555

Reimplement macOS Screens API, differenciate screens by CGDirectDispl…

de8e877

…ayID

Fix build

a14f375

Adjust breaking changes file (none affect users)

62d5058

maxkatz6 added enhancement feature os-macos os-windows labels Jul 12, 2024

maxkatz6 requested a review from kekekeks July 12, 2024 09:55

Merge branch 'master' into screens-api-refactor

557ca54

maxkatz6 commented Jul 12, 2024

View reviewed changes

src/Windows/Avalonia.Win32/Avalonia.Win32.csproj Show resolved Hide resolved

maxkatz6 commented Jul 12, 2024

View reviewed changes

native/Avalonia.Native/src/OSX/Screens.mm Outdated Show resolved Hide resolved

maxkatz6 commented Jul 12, 2024

View reviewed changes

maxkatz6 added 4 commits July 12, 2024 16:01

Fix missing macOS Screen.DisplayName

4f7b8d8

Add more tests + fix screen removal

fd1b5c2

Add screens integration tests

c16f5ef

Use hash set with comparer when removing screens

4b11e53

maxkatz6 commented Jul 13, 2024

View reviewed changes

Merge branch 'master' into screens-api-refactor

c683f10

kekekeks requested changes Jul 13, 2024

View reviewed changes

Make screenimpl safer on macOS as per review

5e4d96d

maxkatz6 requested a review from kekekeks July 16, 2024 06:06

maxkatz6 added 3 commits July 15, 2024 23:45

Replace UnmanagedCallersOnly usage with source generated EnumDisplayM…

cf1c184

…onitors

Remove unused dllimport

4e9e8ff

Only implement GetHashCode and Equals on PlatformScreen subclass, wit…

c04c15b

…hout changing base Screen

kekekeks approved these changes Jul 16, 2024

View reviewed changes

Merge branch 'master' into screens-api-refactor

ebe1e5c

maxkatz6 merged commit 05ac6d2 into master Jul 16, 2024
9 of 11 checks passed

maxkatz6 deleted the screens-api-refactor branch July 16, 2024 22:48

This was referenced Jul 17, 2024

Fix headless screens #16348

Merged

Function pointers and LibraryImport mono/SkiaSharp#2917

Merged

maxkatz6 mentioned this pull request Sep 24, 2024

Add missing Screen.Equals/GetHashCode overrides #17112

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Screens API refactor #16295

Screens API refactor #16295

maxkatz6 commented Jul 12, 2024 •

edited

Loading

maxkatz6 commented Jul 12, 2024

maxkatz6 Jul 12, 2024

kekekeks Jul 13, 2024

kekekeks Jul 13, 2024

kekekeks Jul 13, 2024

maxkatz6 Jul 13, 2024 •

edited

Loading

kekekeks Jul 13, 2024

maxkatz6 Jul 13, 2024

maxkatz6 Jul 13, 2024

maxkatz6 Jul 16, 2024

maxkatz6 commented Jul 13, 2024

avaloniaui-bot commented Jul 13, 2024

avaloniaui-bot commented Jul 13, 2024

kekekeks Jul 13, 2024

kekekeks Jul 13, 2024

kekekeks Jul 13, 2024

avaloniaui-bot commented Jul 16, 2024

maxkatz6 commented Jul 16, 2024

avaloniaui-bot commented Jul 16, 2024

avaloniaui-bot commented Jul 16, 2024

		@@ -43,14 +66,44 @@ virtual HRESULT GetScreen (int index, AvnScreen* ret) override

		ret->Scaling = 1;

Screens API refactor #16295

Screens API refactor #16295

Conversation

maxkatz6 commented Jul 12, 2024 • edited Loading

What does the pull request do?

Checklist

Breaking changes

Fixed issues

maxkatz6 commented Jul 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxkatz6 Jul 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxkatz6 commented Jul 13, 2024

avaloniaui-bot commented Jul 13, 2024

avaloniaui-bot commented Jul 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avaloniaui-bot commented Jul 16, 2024

maxkatz6 commented Jul 16, 2024

avaloniaui-bot commented Jul 16, 2024

avaloniaui-bot commented Jul 16, 2024

maxkatz6 commented Jul 12, 2024 •

edited

Loading

maxkatz6 Jul 13, 2024 •

edited

Loading