Skip to content
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

Introduces Cocoa_GetWindowDisplayIndex. This enable a proper management for dpi when switching between retina and non-retina displays. #5573

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

misl6
Copy link
Contributor

@misl6 misl6 commented Apr 25, 2022

Description

The generic SDL_GetWindowDisplayIndex doesn't provide the real window positioning on macOS.

On macOS, displays may have separate Spaces (by default, it can be changed via System Preferences > Mission Control > Displays have separate Spaces)

This means that even if the window is placed by 99% on "SCREEN B", the focus is kept on "SCREEN A", the window belongs to the "SCREEN A".

These changes introduce Cocoa_GetWindowDisplayIndex, which takes into account NSWindow.screen.

That makes possible to select the right display density only when the window really switches the display is attached to. (As an example when moving the window from a non-retina display and a retina one or vice-versa)

Existing Issue(s)

…nt for dpi when switching between retina and non-retina displays.
@slouken slouken self-assigned this Apr 25, 2022
@slouken slouken added this to the 2.0.24 milestone Apr 25, 2022
@slouken slouken merged commit 76afb85 into libsdl-org:main Apr 26, 2022
@sezero
Copy link
Contributor

sezero commented Apr 26, 2022

I got the following error when building against 10.8 SDM from an old clang version:

src/video/cocoa/SDL_cocoawindow.m:2213:12: error: initializing 'CGRect' (aka 'struct CGRect') with an expression of incompatible type 'NSRect' (aka 'struct _NSRect')
    CGRect displayframe = data->nswindow.screen.frame;
           ^              ~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [build/SDL_cocoawindow.lo] Error 1
make: *** Waiting for unfinished jobs....

@misl6
Copy link
Contributor Author

misl6 commented Apr 26, 2022

I got the following error when building against 10.8 SDM from an old clang version:

src/video/cocoa/SDL_cocoawindow.m:2213:12: error: initializing 'CGRect' (aka 'struct CGRect') with an expression of incompatible type 'NSRect' (aka 'struct _NSRect')
    CGRect displayframe = data->nswindow.screen.frame;
           ^              ~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [build/SDL_cocoawindow.lo] Error 1
make: *** Waiting for unfinished jobs....

I guess this is due to: When building for 64 bit systems, or building 32 bit like 64 bit, NSRect is typedef’d to CGRect. Reference: https://developer.apple.com/documentation/foundation/nsrect?language=objc

Will submit a patch ASAP.

@sezero
Copy link
Contributor

sezero commented Apr 26, 2022

I was indeed building for i686

@misl6
Copy link
Contributor Author

misl6 commented Apr 26, 2022

I was indeed building for i686

Can you confirm that https://github.com/libsdl-org/SDL/pull/5576/files fixes the issue?

@sezero
Copy link
Contributor

sezero commented Apr 26, 2022

Yes, it does. Thanks!

icculus pushed a commit to icculus/SDL that referenced this pull request Apr 27, 2022
peppy added a commit to ppy/SDL that referenced this pull request Jul 13, 2022
…crash

With the introduction of this function, it is possible that for certain
monitor and window configurations, creating an SDL window will cause a
native crash.

```
Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000050
Exception Codes:       0x0000000000000001, 0x0000000000000050
Exception Note:        EXC_CORPSE_NOTIFY

Termination Reason:    Namespace SIGNAL, Code 11 Segmentation fault: 11
Terminating Process:   exc handler [56627]

VM Region Info: 0x50 is not in any region.  Bytes before following region: 140737486737328
      REGION TYPE                    START - END         [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      UNUSED SPACE AT START
--->
      VM_ALLOCATE              7fffffe75000-7fffffe76000 [    4K] r-x/r-x SM=ALI

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libSDL2.dylib                            0x10247f665 SDL_UpdateFullscreenMode + 357
1   libSDL2.dylib                            0x10247ec70 SDL_CreateWindow_REAL + 1504
2   ???                                      0x111262de8 ???
3   ???                                      0x110c39fff ???
4   libcoreclr.dylib                         0x101fdf2a9 CallDescrWorkerInternal + 124
```

Tracking thread from our end: ppy/osu-framework#5190
Regressed with: libsdl-org#5573

In testing, the window would not find a valid screen if created
"hanging" off a primary display with a secondary display below it. In
checking why this was the case, the `display_centre` was being
calculated with a negative y origin, causing a final negative value
falling outside all display bounds:

```
SDL error log [debug]: display_centre.y = -1296 + 1296 / 2

SDL error log [debug]: Display rect 0: 0 0 2560 1440
SDL error log [debug]: Display rect 1: 2560 -625 1080 2560
SDL error log [debug]: Display rect 2: 0 1440 1728 1296
```

The method that was being used to find the current window using the frame
origin/size seems unreliable, so I have opted to replace it with with a
tried method (https://stackoverflow.com/a/40891902).

Initial testing shows that this works with non-standard DPI screens, but
further testing would be appreciated (cc @sezero / @misl6 from the
original PR thread).
slouken pushed a commit that referenced this pull request Jul 24, 2022
…crash

With the introduction of this function, it is possible that for certain
monitor and window configurations, creating an SDL window will cause a
native crash.

```
Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000050
Exception Codes:       0x0000000000000001, 0x0000000000000050
Exception Note:        EXC_CORPSE_NOTIFY

Termination Reason:    Namespace SIGNAL, Code 11 Segmentation fault: 11
Terminating Process:   exc handler [56627]

VM Region Info: 0x50 is not in any region.  Bytes before following region: 140737486737328
      REGION TYPE                    START - END         [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      UNUSED SPACE AT START
--->
      VM_ALLOCATE              7fffffe75000-7fffffe76000 [    4K] r-x/r-x SM=ALI

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libSDL2.dylib                            0x10247f665 SDL_UpdateFullscreenMode + 357
1   libSDL2.dylib                            0x10247ec70 SDL_CreateWindow_REAL + 1504
2   ???                                      0x111262de8 ???
3   ???                                      0x110c39fff ???
4   libcoreclr.dylib                         0x101fdf2a9 CallDescrWorkerInternal + 124
```

Tracking thread from our end: ppy/osu-framework#5190
Regressed with: #5573

In testing, the window would not find a valid screen if created
"hanging" off a primary display with a secondary display below it. In
checking why this was the case, the `display_centre` was being
calculated with a negative y origin, causing a final negative value
falling outside all display bounds:

```
SDL error log [debug]: display_centre.y = -1296 + 1296 / 2

SDL error log [debug]: Display rect 0: 0 0 2560 1440
SDL error log [debug]: Display rect 1: 2560 -625 1080 2560
SDL error log [debug]: Display rect 2: 0 1440 1728 1296
```

The method that was being used to find the current window using the frame
origin/size seems unreliable, so I have opted to replace it with with a
tried method (https://stackoverflow.com/a/40891902).

Initial testing shows that this works with non-standard DPI screens, but
further testing would be appreciated (cc @sezero / @misl6 from the
original PR thread).
PJB3005 pushed a commit to PJB3005/SDL that referenced this pull request Oct 5, 2022
…crash

With the introduction of this function, it is possible that for certain
monitor and window configurations, creating an SDL window will cause a
native crash.

```
Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000050
Exception Codes:       0x0000000000000001, 0x0000000000000050
Exception Note:        EXC_CORPSE_NOTIFY

Termination Reason:    Namespace SIGNAL, Code 11 Segmentation fault: 11
Terminating Process:   exc handler [56627]

VM Region Info: 0x50 is not in any region.  Bytes before following region: 140737486737328
      REGION TYPE                    START - END         [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      UNUSED SPACE AT START
--->
      VM_ALLOCATE              7fffffe75000-7fffffe76000 [    4K] r-x/r-x SM=ALI

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libSDL2.dylib                            0x10247f665 SDL_UpdateFullscreenMode + 357
1   libSDL2.dylib                            0x10247ec70 SDL_CreateWindow_REAL + 1504
2   ???                                      0x111262de8 ???
3   ???                                      0x110c39fff ???
4   libcoreclr.dylib                         0x101fdf2a9 CallDescrWorkerInternal + 124
```

Tracking thread from our end: ppy/osu-framework#5190
Regressed with: libsdl-org#5573

In testing, the window would not find a valid screen if created
"hanging" off a primary display with a secondary display below it. In
checking why this was the case, the `display_centre` was being
calculated with a negative y origin, causing a final negative value
falling outside all display bounds:

```
SDL error log [debug]: display_centre.y = -1296 + 1296 / 2

SDL error log [debug]: Display rect 0: 0 0 2560 1440
SDL error log [debug]: Display rect 1: 2560 -625 1080 2560
SDL error log [debug]: Display rect 2: 0 1440 1728 1296
```

The method that was being used to find the current window using the frame
origin/size seems unreliable, so I have opted to replace it with with a
tried method (https://stackoverflow.com/a/40891902).

Initial testing shows that this works with non-standard DPI screens, but
further testing would be appreciated (cc @sezero / @misl6 from the
original PR thread).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants