add new property DisplayInfo.RefreshRate #1505

janusw · 2020-11-06T13:24:51Z

Description of Change

This enhancement was proposed in #1443. It extends the DisplayInfo class with information about the display's refresh rate.

The new property has an implementation on Android, iOS and UWP. On MacOS it is always zero at present, because the corresponding Xamarin API is missing on MacOS (see also xamarin/xamarin-macios#9958).

I have tested the implementation on real hardware with Android 10 and iOS14.

Bugs Fixed

Fixes [Enhancement] DeviceDisplay/DisplayInfo should report refresh rate #1443

API Changes

List all API changes here (or just put None), example:

Added:

public float DisplayInfo.RefreshRate { get; }
constructor: DisplayInfo(double width, double height, double density, DisplayOrientation orientation, DisplayRotation rotation, float rate)

Behavioral Changes

None.

PR Checklist

Has tests (if omitted, state reason in description)
Has samples (if omitted, state reason in description)
Rebased on top of main at time of PR
Changes adhere to coding standard
Updated documentation (see walkthrough)

mattleibow

Looking really good. We just can't break API.

Xamarin.Essentials/Types/DisplayInfo.shared.cs

* with a proper implementation on Android and iOS * MacOS and UWP always give a rate of zero

Based on xamarin-macios issue: xamarin/xamarin-macios#9958 Copied code from: https://gist.github.com/chamons/1eae64a2e5e10cbaf18b140e92ec4ca3

mattleibow · 2021-01-19T03:42:26Z

/azp run

azure-pipelines · 2021-01-19T03:42:39Z

Azure Pipelines successfully started running 1 pipeline(s).

mattleibow

I tested on my Razer Phone and got that sweet extra Hz

mattleibow · 2021-01-19T04:51:01Z

CI seems unhappy on some devices. Can't seem to repro it locally :(

…m/janusw/Essentials into pr/1505

mattleibow · 2021-01-19T05:22:45Z

Xamarin.Essentials/DeviceDisplay/DeviceDisplay.android.cs

-            using var display = GetDefaultDisplay();
+            var display = GetDefaultDisplay();


Without this, it appears that Android 21-24 throws a ObjectDisposedException for the Display object

janusw · 2021-01-19T12:35:22Z

I tested on my Razer Phone and got that sweet extra Hz

Cool :)
I only tested it on standard 60 Hz devices so far ...

janusw · 2021-01-19T12:38:58Z

Thanks a lot for adding a MacOS implementation, @mattleibow! I just ran Samples.Mac on a MacBook with Big Sur, but unfortunately I see a Refresh Rate of zero :/
Did you test it and could you observe a more meaningful value?

mattleibow · 2021-01-19T13:08:20Z

I got 60 on my Mac mini. I saw in the docs that sometimes it will be 0, but let me see if there is more info or another way.

Docs: https://developer.apple.com/documentation/coregraphics/1454661-cgdisplaymodegetrefreshrate

Not sure if this is the best now that you say it isn't working...

mattleibow · 2021-01-19T16:26:08Z

@janusw I think I may have found a solution... But it doesn't work for my screen...

I saw this issue: glfw/glfw#137
And this commit: glfw/glfw@aab0871

Maybe if you tweak some things it works for you?

janusw · 2021-01-20T11:32:10Z

@janusw I think I may have found a solution... But it doesn't work for my screen...

I saw this issue: glfw/glfw#137
And this commit: glfw/glfw@aab0871

Maybe if you tweak some things it works for you?

Thanks for your efforts here, Matt! I tried your solution, but I still only get a RefreshRate of 0 on the MacBook.

I see CVDisplayLinkInterop.GetRefreshRate being called twice. The first time, all components of period are zero. The second time, period.Flags and period.TimeScale have rather odd non-zero values, but period.TimeValue is still zero. I have no idea why this is the case.

This is not the most correct way, but is probably good enough. If there are any better ways in future, we can just add them in or replace this logic.

janusw · 2021-01-25T20:11:25Z

Awesome! Thanks for merging 😃

* main: Update docs Add new property DisplayInfo.RefreshRate (xamarin#1505)

mattleibow reviewed Nov 6, 2020

View reviewed changes

Xamarin.Essentials/Types/DisplayInfo.shared.cs Show resolved Hide resolved

janusw force-pushed the issue1443_displayinfo_refreshrate branch from affa970 to 0994f1f Compare November 6, 2020 19:58

janusw added 4 commits December 1, 2020 08:48

DisplayInfo: add a property 'RefreshRate' (xamarin#1443)

43761b1

* with a proper implementation on Android and iOS * MacOS and UWP always give a rate of zero

DisplayInfo: implement 'RefreshRate' on UWP

5522db4

Samples: show RefreshRate on DeviceInfoPage (xamarin#1443)

da20bae

add documentation for DisplayInfo.RefreshRate (xamarin#1443)

46a0f8b

janusw force-pushed the issue1443_displayinfo_refreshrate branch from 0994f1f to 46a0f8b Compare December 1, 2020 07:48

mattleibow added 2 commits December 6, 2020 16:24

Merge branch 'main' into issue1443_displayinfo_refreshrate

ecdfae9

Merge branch 'main' into issue1443_displayinfo_refreshrate

fb9775d

mattleibow previously approved these changes Jan 19, 2021

View reviewed changes

mattleibow added this to the 1.6.1 milestone Jan 19, 2021

Add the logic to read the macos refresh rate

3805d7a

Based on xamarin-macios issue: xamarin/xamarin-macios#9958 Copied code from: https://gist.github.com/chamons/1eae64a2e5e10cbaf18b140e92ec4ca3

mattleibow dismissed their stale review via 3805d7a January 19, 2021 03:36

mattleibow previously approved these changes Jan 19, 2021

View reviewed changes

Merge branch 'main' into issue1443_displayinfo_refreshrate

fb28804

mattleibow added 2 commits January 19, 2021 07:20

Older Android versions don't like disposal

69000ea

Merge branch 'issue1443_displayinfo_refreshrate' of https://github.co…

c145524

…m/janusw/Essentials into pr/1505

mattleibow dismissed their stale review via c145524 January 19, 2021 05:21

mattleibow reviewed Jan 19, 2021

View reviewed changes

Try using CV as well

70ba644

Merge branch 'main' into issue1443_displayinfo_refreshrate

67432ac

Merge branch 'main' into issue1443_displayinfo_refreshrate

452c41d

mattleibow added 2 commits January 22, 2021 00:10

Merge branch 'main' into issue1443_displayinfo_refreshrate

d694891

Try to guess the Hz, but fall back to 60

e958c01

This is not the most correct way, but is probably good enough. If there are any better ways in future, we can just add them in or replace this logic.

mattleibow merged commit 90a40a3 into xamarin:main Jan 25, 2021

janusw deleted the issue1443_displayinfo_refreshrate branch January 25, 2021 20:11

dimonovdd added a commit to dimonovdd/Essentials that referenced this pull request Jan 26, 2021

Merge branch 'main' into featureSaveToGalery

eaa70f2

* main: Update docs Add new property DisplayInfo.RefreshRate (xamarin#1505)

dependabot bot mentioned this pull request Mar 18, 2021

Bump Xamarin.Essentials from 1.5.1 to 1.6.1 aleks-ivanov/maui#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add new property DisplayInfo.RefreshRate #1505

add new property DisplayInfo.RefreshRate #1505

janusw commented Nov 6, 2020 •

edited by mattleibow

Loading

mattleibow left a comment

mattleibow commented Jan 19, 2021

azure-pipelines bot commented Jan 19, 2021

mattleibow left a comment

mattleibow commented Jan 19, 2021

mattleibow Jan 19, 2021

janusw commented Jan 19, 2021

janusw commented Jan 19, 2021

mattleibow commented Jan 19, 2021 •

edited

Loading

mattleibow commented Jan 19, 2021

janusw commented Jan 20, 2021

janusw commented Jan 25, 2021

		using var display = GetDefaultDisplay();
		var display = GetDefaultDisplay();

add new property DisplayInfo.RefreshRate #1505

add new property DisplayInfo.RefreshRate #1505

Conversation

janusw commented Nov 6, 2020 • edited by mattleibow Loading

Description of Change

Bugs Fixed

API Changes

Behavioral Changes

PR Checklist

mattleibow left a comment

Choose a reason for hiding this comment

mattleibow commented Jan 19, 2021

azure-pipelines bot commented Jan 19, 2021

mattleibow left a comment

Choose a reason for hiding this comment

mattleibow commented Jan 19, 2021

mattleibow Jan 19, 2021

Choose a reason for hiding this comment

janusw commented Jan 19, 2021

janusw commented Jan 19, 2021

mattleibow commented Jan 19, 2021 • edited Loading

mattleibow commented Jan 19, 2021

janusw commented Jan 20, 2021

janusw commented Jan 25, 2021

janusw commented Nov 6, 2020 •

edited by mattleibow

Loading

mattleibow commented Jan 19, 2021 •

edited

Loading