SDL icon indicating copy to clipboard operation
SDL copied to clipboard

SDL sys timer Mac OS update proposal

Open SDLBugzilla opened this issue 4 years ago • 0 comments

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1 Reported for operating system, platform: Mac OS X (All), All

Comments on the original bug report:

On 2021-01-15 20:44:06 +0000, David Carlier wrote:

Created attachment 4666 patch

Change of api from 2016 which reduce code complexity a bit.

On 2021-01-15 20:51:18 +0000, Ozkan Sezer wrote:

As far as I can see, this breaks macOS < 10.12

On 2021-01-15 20:53:30 +0000, Ryan C. Gordon wrote:

(In reply to Ozkan Sezer from comment # 1)

As far as I can see, this breaks macOS < 10.12

Fwiw, mach_absolute_time() is apparently deprecated as of 11.0, so we'll probably have to wrap this in preprocessor check in any case. :/

--ryan.

On 2021-01-15 21:09:59 +0000, David Carlier wrote:

Ah older still supported. I can try detection at config time in both autoconf/Cmake fashions.

On 2021-01-15 21:37:18 +0000, David Carlier wrote:

Created attachment 4668 patch 2

On 2021-01-15 22:07:26 +0000, Ozkan Sezer wrote:

Created attachment 4671 patch 3

The second patch didn't apply, I regenerated it by guess-work: Please confirm that it matches your intention.

On 2021-01-15 22:12:30 +0000, David Carlier wrote:

Yes sorry I m not comfortable with mercurial really much better with git :-) but the diff looks good indeed, except maybe those two lines removal

-#else
-#define SDL_MONOTONIC_CLOCK CLOCK_MONOTONIC

On 2021-01-15 22:25:55 +0000, Ozkan Sezer wrote:

Created attachment 4672 patch 4

(In reply to David Carlier from comment # 6)

but the diff looks good indeed, except maybe those two lines removal

Attached v4 patch.

On 2021-01-15 22:27:11 +0000, David Carlier wrote:

+1

On 2021-01-17 18:13:58 +0000, Sam Lantinga wrote:

Is there any benefit for using the newer API? If it's performance, can you attach your test and the timing results you got?

On 2021-01-17 18:41:25 +0000, David Carlier wrote:

Not that much (no more init but that s once) but more reliability, less error prone and easier to use. It s usually advised to move on to this newer api when possible (ideally I would have prefered removing the ancient one but depends on support wanted).

On 2021-01-23 17:51:15 +0000, Sam Lantinga wrote:

Patch added, thanks! https://hg.libsdl.org/SDL/rev/8cad7d7ec50c

On 2021-01-23 22:31:20 +0000, Ozkan Sezer wrote:

Created attachment 4697 better check for clock_gettime_nsec_np()

On 2021-01-23 22:31:50 +0000, Ozkan Sezer wrote:

(In reply to Ozkan Sezer from comment # 12)

Created attachment 4697 [details] better check for clock_gettime_nsec_np()

The attached patch makes sure that the build is targeting macOS >= 10.12 for clock_gettime_nsec_np() availability. It also adds missing HAVE_xxx to SDL_config_macosx.h. Please review.

Notes: I do NOT know how to check IOS version requirements. time.h from 10.12 SDK says: __IOS_AVAILABLE(10.0) __TVOS_AVAILABLE(10.0) __WATCHOS_AVAILABLE(3.0)

If these are already the minimum requirements from SDL, then I can add the availability unconditionally: Tell me what to do for it and I will complete the patch and push to hg.

On 2021-01-23 22:48:57 +0000, David Carlier wrote:

I do NOT know how to check IOS version requirements

I do not know if that answers your question but the attribute available can

as :

if (@available(iOS 10, *)) // It is variadic to be able to check tvOs and so on

On 2021-01-24 00:08:31 +0000, Ozkan Sezer wrote:

(In reply to David Carlier from comment # 14)

I do not know if that answers your question but the attribute available can

as :

if (@available(iOS 10, *)) // It is variadic to be able to check tvOs and so on

I believe that's for detection at run time. We need something for compile-time.

On 2021-01-24 06:22:41 +0000, David Carlier wrote:

ah right I think you have constants such as :

  • __IPHONE_OS_VERSION_MIN_REQUIRED
  • more broadly __OSX_AVAILABLE_STARTING(__MAC_10_2,__IPHONE_10_0)

from the top of my head.

On 2021-01-24 06:28:28 +0000, David Carlier wrote:

otherwise there are these in case :

  • __WATCHOS_AVAILABLE() and __TVOS_AVAILABLE()

On 2021-01-24 14:40:55 +0000, Ozkan Sezer wrote:

Created attachment 4700 better check for clock_gettime_nsec_np()

-- patch rebased to current hg

On 2021-01-24 16:47:00 +0000, Ozkan Sezer wrote:

Created attachment 4701 better check for clock_gettime_nsec_np() - v6

Updated patch, v6:

  • Unconditionally defines HAVE_CLOCK_GETTIME_NSEC_NP for iOS / TVOS assuming iOS >= 10.0. This is similar to SDL_JOYSTICK_MFI being defined for iOS or TVOS which depend on GameController framework which require iOS >=10.0 at the least.

Comments? OK to apply?

On 2021-01-24 19:16:04 +0000, David Carlier wrote:

Seems ok to me

On 2021-01-25 00:29:47 +0000, Ryan C. Gordon wrote:

Latest patch looks good to me.

--ryan.

On 2021-01-25 01:13:40 +0000, Ozkan Sezer wrote:

(In reply to David Carlier from comment # 20)

Seems ok to me

(In reply to Ryan C. Gordon from comment # 21)

Latest patch looks good to me.

--ryan.

OK, applied the latest patch as https://hg.libsdl.org/SDL/rev/d01593bd58d9

Re-closing as fixed.

On 2021-02-02 05:59:03 +0000, Sam Lantinga wrote:

SDL_GetTicks() was broken, so I backed this out: https://hg.libsdl.org/SDL/rev/88c79c6a8457

On 2021-02-02 06:16:00 +0000, David Carlier wrote:

Ah surprising but understood... Sorry for this.

On 2021-02-02 06:41:04 +0000, Sam Lantinga wrote:

It may make sense to use clock_gettime_nsec_np() for the performance timer and leave the original code for SDL_GetTicks().

Thoughts?

On 2021-02-02 06:44:38 +0000, David Carlier wrote:

Could work out indeed.

On 2021-02-02 17:56:46 +0000, Sam Lantinga wrote:

Is there any advantage in performance or resolution?

On 2021-02-02 18:06:39 +0000, David Carlier wrote:

Performance not really, resolution most likely and more consistent across architectures.

On 2021-02-02 22:24:48 +0000, Alex Szpakowski wrote:

As far as I know there's no benefit to using clock_gettime_nsec_np versus the current code aside from mach_absolute_time being marked as deprecated. I suspect Apple only marked it as deprecated in the first place because other code (not SDL) was using it wrong on macOS in a way that will produce correct results on x64 but not Apple Silicon CPUs.

SDLBugzilla avatar Feb 11 '21 02:02 SDLBugzilla