Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

BZFlag 2.4.21
-------------
* Fix many issues with SDL 2 window management - Joshua Bodine, Scott Wichser


BZFlag 2.4.20 "Do You See What I See?" (2020-04-24)
Expand Down
10 changes: 8 additions & 2 deletions src/bzflag/MainWindow.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ MainWindow::MainWindow(BzfWindow* _window, BzfJoystick* _joystick) :
minHeight(MinY),
faulting(false)
{
if (!window->create())
faulting = true;

window->addResizeCallback(resizeCB, this);
resize();
}

MainWindow::~MainWindow()
Expand All @@ -49,6 +53,7 @@ void MainWindow::setMinSize(int _minWidth, int _minHeight)
minWidth = _minWidth;
minHeight = _minHeight;
window->setMinSize(minWidth, minHeight);
resize();
}

void MainWindow::setPosition(int x, int y)
Expand All @@ -59,6 +64,7 @@ void MainWindow::setPosition(int x, int y)
void MainWindow::setSize(int _width, int _height)
{
window->setSize(_width, _height);
resize();
}

void MainWindow::showWindow(bool on)
Expand Down Expand Up @@ -154,6 +160,8 @@ void MainWindow::toggleFullscreen()
{
isFullscreen = !isFullscreen;
window->setFullscreen(isFullscreen);
window->create();
resize();
}

void MainWindow::setFullView(bool _isFullView)
Expand Down Expand Up @@ -279,8 +287,6 @@ void MainWindow::resize()
{
window->getSize(trueWidth, trueHeight);
window->makeCurrent();
if (!window->create())
faulting = true;
setQuadrant(quadrant);
}

Expand Down
3 changes: 0 additions & 3 deletions src/bzflag/bzflag.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1169,9 +1169,6 @@ int main(int argc, char** argv)
// enable vsync if needed
pmainWindow->getWindow()->setVerticalSync(BZDB.evalInt("saveEnergy") == 2);

// Make sure the window is created
pmainWindow->getWindow()->callResizeCallbacks();

// get sound files. must do this after creating the window because
// DirectSound is a bonehead API.
if (!noAudio)
Expand Down
9 changes: 4 additions & 5 deletions src/bzflag/playing.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ static void doEvent(BzfDisplay *disply)
}

// ungrab the mouse if we're running full screen
if (mainWindow->getFullscreen())
if (mainWindow->getFullscreen() && !unmapped) // skip if already unmapped to avoid losing previous resolution
{
preUnmapFormat = -1;
if (disply->getNumResolutions() > 1)
Expand Down Expand Up @@ -7530,11 +7530,7 @@ void startPlaying(BzfDisplay* _display,
{
videoFormat = BZDB.get("resolution");
if (videoFormat.length() != 0)
{
format = display->findResolution(videoFormat.c_str());
if (format >= 0)
mainWindow->getWindow()->callResizeCallbacks();
}
};
// set the resolution (only if in full screen mode)
if (!BZDB.isSet("_window") && BZDB.isSet("resolution"))
Expand Down Expand Up @@ -7572,6 +7568,9 @@ void startPlaying(BzfDisplay* _display,
}
}
}
// otherwise, use the default resolution if we do switch to fullscreen
else
display->setDefaultResolution();

// grab mouse if we should
if (shouldGrabMouse())
Expand Down
151 changes: 138 additions & 13 deletions src/platform/SDL2Window.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ SDLWindow::~SDLWindow()
{
// Restore the original gamma when we exit the client
setGamma(origGamma);

if (windowId != NULL)
SDL_DestroyWindow(windowId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you destroy the window, shouldn't you set it to null? (or better, nullptr)

}

void SDLWindow::setTitle(const char *_title)
Expand Down Expand Up @@ -98,16 +101,27 @@ void SDLWindow::getMouse(int& _x, int& _y) const

void SDLWindow::setSize(int _width, int _height)
{
// workaround for two issues on Linux, where resizing by dragging the window corner causes glitching, and where
// iconifying or switching applications while using a scaled fullscreen resolution causes the non-fullscreen
// window resolution to assume the fullscreen resolution
#ifdef __linux__
if(!fullScreen)
{
base_width = _width;
base_height = _height;
}
#else
base_width = _width;
base_height = _height;
if (!fullScreen && windowId)
SDL_SetWindowSize(windowId, base_width, base_height);
#endif // __linux__
}

void SDLWindow::getSize(int& width, int& height) const
{
if (fullScreen)
const_cast<SDLDisplay *>((const SDLDisplay *)getDisplay())->getWindowSize(width, height);
const_cast<SDLDisplay *>(static_cast<const SDLDisplay *>(getDisplay()))->getWindowSize(width, height);
else
{
width = base_width;
Expand Down Expand Up @@ -148,7 +162,10 @@ void SDLWindow::swapBuffers()
if (! SDL_GL_GetSwapInterval())
return;

const int maxRunawayFPS = 65;
int maxRunawayFPS = 65;
SDL_DisplayMode desktopDisplayMode;
if (SDL_GetDesktopDisplayMode(0, &desktopDisplayMode) == 0)
maxRunawayFPS = desktopDisplayMode.refresh_rate + 5;

static TimeKeeper lastFrame = TimeKeeper::getSunGenesisTime();
const TimeKeeper now = TimeKeeper::getCurrent();
Expand All @@ -165,6 +182,24 @@ void SDLWindow::swapBuffers()
#endif //__APPLE__
}

// For some reason, when creating a new fullscreen window on Linux with a different resolution than before, SDL throws
// a resize event with the old window resolution, which is not what we want. This function is called to filter SDL
// window resize events right after the resolution change and adjust the resolution to the correct one.
#ifdef __linux__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it need to be restricted to Linux?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. In general, I would like to keep platform-specific workarounds within #ifdef guards, because we have a tendency to break other platforms while fixing an issue on one of them. Keeping the unknowns to a minimum is a plus in my opinion.

int SDLWindowEventFilter(void *resolution, SDL_Event *event)
{
if(event->type == SDL_WINDOWEVENT && (event->window.event == SDL_WINDOWEVENT_RESIZED ||
event->window.event == SDL_WINDOWEVENT_SIZE_CHANGED))
{
// adjust the window resolution to match the values passed to us
event->window.data1 = static_cast<int *>(resolution)[0];
event->window.data2 = static_cast<int *>(resolution)[1];
}

return 1; // allow the event
}
#endif // __linux__

bool SDLWindow::create(void)
{
int targetWidth, targetHeight;
Expand Down Expand Up @@ -196,17 +231,107 @@ bool SDLWindow::create(void)
}

// (re)create the window
const Uint32 flags = SDL_WINDOW_OPENGL |
(fullScreen ? SDL_WINDOW_FULLSCREEN : SDL_WINDOW_RESIZABLE) |
(windowWasGrabbed ? SDL_WINDOW_INPUT_GRABBED : 0);

windowId = SDL_CreateWindow(
title.c_str(),
SDL_WINDOWPOS_UNDEFINED,
SDL_WINDOWPOS_UNDEFINED,
targetWidth,
targetHeight,
flags);

// workaround for an SDL 2 bug on Linux with the GNOME Window List extension enabled, where attempting to create a
// fullscreen window on a lower-resolution primary display while a higher-resolution secondary display is plugged in
// causes an infinite loop of window creation on the secondary display
// bug report: https://bugzilla.libsdl.org/show_bug.cgi?id=4990
#ifdef __linux__
if(! fullScreen || SDL_GetNumVideoDisplays() < 2) // create the window with the standard logic
{
#endif // __linux__
windowId = SDL_CreateWindow(
title.c_str(),
SDL_WINDOWPOS_UNDEFINED,
SDL_WINDOWPOS_UNDEFINED,
targetWidth,
targetHeight,
SDL_WINDOW_OPENGL |
(fullScreen ? SDL_WINDOW_FULLSCREEN : SDL_WINDOW_RESIZABLE) |
(windowWasGrabbed ? SDL_WINDOW_INPUT_GRABBED : 0));

// continuation of above workaround
#ifdef __linux__
}
else // create the window in windowed mode first and then switch to fullscreen
{
windowId = SDL_CreateWindow(
title.c_str(),
SDL_WINDOWPOS_UNDEFINED,
SDL_WINDOWPOS_UNDEFINED,
base_width,
base_height,
SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE | (windowWasGrabbed ? SDL_WINDOW_INPUT_GRABBED : 0));

SDL_DisplayMode displayMode;
if(SDL_GetDesktopDisplayMode(0, &displayMode) < 0)
{
printf("Unable to get desktop display mode: %s", SDL_GetError());
return false;
}
displayMode.w = targetWidth;
displayMode.h = targetHeight;
if(SDL_SetWindowDisplayMode(windowId, &displayMode))
{
printf("Unable to set display mode: %s", SDL_GetError());
return false;
}
if(SDL_SetWindowFullscreen(windowId, SDL_WINDOW_FULLSCREEN) < 0)
{
printf("Unable to set window to fullscreen mode: %s", SDL_GetError());
return false;
}
}

// Depending on the distribution (or possibly the window manager), SDL will not recognize the new window resolution
// and will keep returning the previous resolution when SDL_GetWindowSize() is called, until a period of time has
// passed and events have been pumped. Wait up to two seconds for the correct resolution to start being returned,
// checking every quarter second, to avoid repeating the window destruction/re-creation process based on bad data.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a way to flush the queue? This seems like an exceptionally awkward work-around. Or maybe we don't understand the resize event logic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely agreed that the workaround is awkward. SDL_PumpEvents() is flushing the queue in this case. After extensive testing, it appeared to me that both a time delay and pumping events are required in order to reach the point where the correct window resolution is returned. My best guess is that the window creation process in SDL returns before the modeset is complete. It took extensive testing to get to this point. However, if anyone else has a better solution, I'd be all for it.

int currentWidth, currentHeight, resCheckLoops = 0;

do
{
SDL_PumpEvents();
SDL_GetWindowSize(windowId, &currentWidth, &currentHeight);

if(currentWidth == targetWidth && currentHeight == targetHeight)
break;

TimeKeeper::sleep(0.25f);
}
while (resCheckLoops++ < 8);
#endif // __linux__

// Apply filters due to resize event issues on Linux (see the explanation above for SDLWindowEventFilter())
#ifdef __linux__
SDL_PumpEvents();
int windowResolution[] = { targetWidth, targetHeight };
SDL_FilterEvents(&SDLWindowEventFilter, windowResolution);
#endif // __linux__

// Work around an issue with SDL on macOS where a window that gets resized by the operating system for various
// reasons (e.g., creating a window that doesn't fit between the dock and menu bar, or switching from a maximized
// window to native fullscreen then back to windowed mode) doesn't always correctly throw a resize event
#ifdef __APPLE__
SDL_PumpEvents();

int currentWidth, currentHeight;
SDL_GetWindowSize(windowId, &currentWidth, &currentHeight);

if(! fullScreen && (currentWidth != targetWidth || currentHeight != targetHeight))
{
SDL_Event fakeResizeEvent;
SDL_zero(fakeResizeEvent);

fakeResizeEvent.window.type = SDL_WINDOWEVENT;
fakeResizeEvent.window.windowID = 0; // deliberately not matching SDL_GetWindowID() so SDL doesn't purge event
fakeResizeEvent.window.event = SDL_WINDOWEVENT_RESIZED;
fakeResizeEvent.window.data1 = currentWidth;
fakeResizeEvent.window.data2 = currentHeight;

SDL_PushEvent(&fakeResizeEvent);
}
#endif // __APPLE__

// Store the gamma immediately after creating the first window
if (origGamma < 0)
Expand Down