macOS: hotplugging a monitor marks it as removed #12016
Milestone
Comments
|
I tried it here with an usb-c connected monitor, and I got (with ie display 3 is attached and immediately removed, then display 4 is attached after. (i only attached the monitor once) |
|
@maia-s, can you enable the debug printing code in Cocoa_DisplayReconfigurationCallback() and run that again? |
|
I also added this patch to testsprite, to help with debugging: diff --git a/test/testsprite.c b/test/testsprite.c
index e8d8a7dcb..c49db59bc 100644
--- a/test/testsprite.c
+++ b/test/testsprite.c
@@ -556,9 +556,35 @@ SDL_AppResult SDL_AppInit(void **appstate, int argc, char *argv[])
SDL_AppResult SDL_AppEvent(void *appstate, SDL_Event *event)
{
+ static bool show = true;
if (event->type == SDL_EVENT_RENDER_DEVICE_RESET) {
LoadSprite(icon);
}
+ switch (event->type) {
+ case SDL_EVENT_DISPLAY_ADDED:
+ SDL_Log("SDL_EVENT_DISPLAY_ADDED %d\n", event->display.displayID);
+ show = true;
+ break;
+ case SDL_EVENT_DISPLAY_REMOVED:
+ SDL_Log("SDL_EVENT_DISPLAY_REMOVED %d\n", event->display.displayID);
+ show = true;
+ break;
+ case SDL_EVENT_DISPLAY_MOVED:
+ SDL_Log("SDL_EVENT_DISPLAY_MOVED %d\n", event->display.displayID);
+ show = true;
+ break;
+ }
+ if (show) {
+ SDL_DisplayID *displays = SDL_GetDisplays(NULL);
+ int i;
+ for (i = 0; displays[i]; ++i) {
+ SDL_DisplayID d = displays[i];
+ SDL_Rect rect;
+ SDL_GetDisplayBounds(d, &rect);
+ SDL_Log("Display %d '%s': %dx%d at %d,%d\n", d, SDL_GetDisplayName(d), rect.w, rect.h, rect.x, rect.y);
+ }
+ show = false;
+ }
return SDLTest_CommonEventMainCallbacks(state, event);
}
|
|
Sure thing: Before this, after I added the patches, I also had a run where everything seemed to work alright with only one attach: |
|
This one is more similar to the run in my first post with the detach happening instantly after the first attach |
|
Connecting to a Roku Ultra over the network with AirPlay. First COCOA DISPLAY RECONFIG block is me clicking "disconnect" on the Roku in System Preferences. Second block is me reconnecting to it. |
wtf, it set all of these in one callback? |
Yep! |
|
So it looks like in your case it doesn't set both, so in theory if both are set we can assume it's connected? |
|
Seems like the quickest fix to do that. CGDisplayIsOnline() reports true during this callback for disconnecting devices, infuriatingly. But, add this real fast to the callback: SDL_Log("DISPLAY IS ONLINE: %s", CGDisplayIsOnline(displayid) ? "true" : "false");
SDL_Log("DISPLAY IS %dx%d", (int) CGDisplayPixelsWide(displayid), (int) CGDisplayPixelsHigh(displayid));When I disconnect the Roku, it's still reported as online (probably changes right after this callback), but the display size becomes 0x0. It has a correct size when adding. |
|
Okay, adding: and then removing: Connecting my iPad: Disconnecting my iPad: |
|
Screw it, let's go with the other plan and see what happens. I'm pushing a fix. |
|
Potential fix: diff --git a/src/video/cocoa/SDL_cocoamodes.m b/src/video/cocoa/SDL_cocoamodes.m
index 7de75729c..9a63e779d 100644
--- a/src/video/cocoa/SDL_cocoamodes.m
+++ b/src/video/cocoa/SDL_cocoamodes.m
@@ -405,8 +405,13 @@ static void Cocoa_DisplayReconfigurationCallback(CGDirectDisplayID displayid, CG
}
if ((flags & kCGDisplayAddFlag) && (flags & kCGDisplayRemoveFlag)) {
- // both adding _and_ removing? Treat it as a remove exclusively. This can happen if a display is unmirroring because it's being disabled, etc.
- flags &= ~kCGDisplayAddFlag;
+ if (CGDisplayPixelsWide(displayid) > 1) {
+ // Final state is connected
+ flags &= ~kCGDisplayRemoveFlag;
+ } else {
+ // Final state is disconnected
+ flags &= ~kCGDisplayAddFlag;
+ }
}
if (flags & kCGDisplayAddFlag) { |
|
Oh, connecting a Roku when it defaults to mirrored got this, which is probably where the comment came from: AddFlag (added) and MirrorFlag (removed, because it's not a separate display anymore). |
|
Okay, push the patch in #12016 (comment), but please add a comment that explains why |
|
Actually, that would be a bug in this case. The Roku would be added as a second display since the size would be valid, right? |
|
Ah shoot, yes, it comes up with a real value. - // both adding _and_ removing? Treat it as a remove exclusively. This can happen if a display is unmirroring because it's being disabled, etc.
- flags &= ~kCGDisplayAddFlag;
+ if (((flags & kCGDisplayMirrorFlag) == 0) && (CGDisplayPixelsWide(displayid) > 1)) {
+ // Final state is connected
+ flags &= ~kCGDisplayRemoveFlag;
+ } else {
+ // Final state is disconnected
+ flags &= ~kCGDisplayAddFlag;
+ }
}
Hopefully this isn't going to be a massive pile of hacks by the time we're done. |
|
That seems reasonable to me... push it? |
|
Here was my more complicated rewrite: diff --git a/src/video/cocoa/SDL_cocoamodes.m b/src/video/cocoa/SDL_cocoamodes.m
index 7de75729c..146b08a29 100644
--- a/src/video/cocoa/SDL_cocoamodes.m
+++ b/src/video/cocoa/SDL_cocoamodes.m
@@ -387,29 +387,31 @@ static void Cocoa_DisplayReconfigurationCallback(CGDirectDisplayID displayid, CG
SDL_VideoDevice *_this = (SDL_VideoDevice *) userInfo;
SDL_VideoDisplay *display = Cocoa_FindSDLDisplayByCGDirectDisplayID(_this, displayid); // will be NULL for newly-added (or newly-unmirrored) displays!
+ bool added = false;
+ bool removed = false;
- if (flags & kCGDisplayDisabledFlag) {
- flags |= kCGDisplayRemoveFlag; // treat this like a display leaving, even though it's still plugged in.
+ if (flags & (kCGDisplayRemoveFlag | kCGDisplayDisabledFlag)) {
+ removed = true;
}
- if (flags & kCGDisplayEnabledFlag) {
- flags |= kCGDisplayAddFlag; // treat this like a display leaving, even though it's still plugged in.
+ if (flags & (kCGDisplayAddFlag | kCGDisplayEnabledFlag)) {
+ added = true;
}
if (flags & kCGDisplayMirrorFlag) {
- flags |= kCGDisplayRemoveFlag; // treat this like a display leaving, even though it's still actually here.
+ removed = true; // treat this like a display leaving, even though it's still actually here.
}
if (flags & kCGDisplayUnMirrorFlag) {
- flags |= kCGDisplayAddFlag; // treat this like a new display arriving, even though it was here all along.
+ added = true; // treat this like a new display arriving, even though it was here all along.
}
- if ((flags & kCGDisplayAddFlag) && (flags & kCGDisplayRemoveFlag)) {
- // both adding _and_ removing? Treat it as a remove exclusively. This can happen if a display is unmirroring because it's being disabled, etc.
- flags &= ~kCGDisplayAddFlag;
+ if (added && removed) {
+ // Do we need to see if the display is valid?
+ removed = false;
}
- if (flags & kCGDisplayAddFlag) {
+ if (added) {
if (!display) {
if (!Cocoa_AddDisplay(displayid, true)) {
return; // oh well.
@@ -419,7 +421,7 @@ static void Cocoa_DisplayReconfigurationCallback(CGDirectDisplayID displayid, CG
}
}
- if (flags & kCGDisplayRemoveFlag) {
+ if (removed) {
if (display) {
SDL_DelVideoDisplay(display->id, true);
display = NULL; |
|
This doesn't work here. Every time I disconnect the Roku when mirrored and reconnect it, I get a new display with a number for a name and the dead one remains: Let's go with the smaller patch for now. |
Sounds good! |
|
Okay, it's in. Everyone try the latest in revision control and make sure it still works with your configuration, please! |
|
(Also, with testsprite, you don't need the patch, you can just run it with |
|
I still get the display removed event with the latest git |
|
but not always (this is with just |
|
It does seem a lot more rare now. I've run it 10 times in a row (detach monitor and wait for that to finish, run testsprite, attach monitor, exit testsprite), and it only got the detach event the first time |
|
It happened again after I attached and detached the monitor while testsprite was running. I let it rest a few seconds after each detach seemed complete so I don't think it's a delayed event. It did work a few times before it happened again Final detach before bug: Attach after the above: |
|
In your latest logs the device really is removed for a bit before it reattaches. I think this might be an OS quirk, or just how the underlying device is attached and moved around. I think this is okay, as it ends up in the correct state at the end. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When I plug a second monitor into my Mac Mini M1 running macOS 15.1.1, I get an event that the display was added, then immediately removed.
The debug output shows:
For reference, when the display is removed, the sequence is:
When I connect an iPad as a second display, the sequence is:
and when I remove it, the sequence is:
The text was updated successfully, but these errors were encountered: