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

Add FLAG_FULLSCREEN_DESKTOP #3382

Closed
wants to merge 2 commits into from
Closed

Conversation

M374LX
Copy link
Contributor

@M374LX M374LX commented Oct 6, 2023

This PR implements the third approach suggested in #3378.

Update: not ready to be merged because it has the same problems as #3391 (comment).

Code example

#include <raylib.h>

int main()
{
	SetConfigFlags(FLAG_FULLSCREEN_DESKTOP);
	InitWindow(800, 600, "Test: fullscreen without video mode change");

	while (!WindowShouldClose()) {
		if (IsKeyPressed(KEY_F)) {
			ToggleFullscreen();
		}

		BeginDrawing();
		ClearBackground(GRAY);
		EndDrawing();
	}

	CloseWindow();

	return 0;
}

@blueloveTH
Copy link
Contributor

I think this feature is useful.

@raysan5
Copy link
Owner

raysan5 commented Oct 7, 2023

@ubkp You were working on this feature, please could you take a look?

@ghost
Copy link

ghost commented Oct 7, 2023

@raysan5 Sure thing. Tested (62d3066), some considerations:

  1. When alt-tabbing, the window gets minimized, as the regular fullscreen.

  2. Breaks toggling out of regular fullscreen using ToggleFullscreen if FLAG_FULLSCREEN_DESKTOP is not being used.

  3. Should probably reuse Size previousScreen; (L391) instead of Size restoreSize; (R397)

  4. IMHO, the same could be achieved with:

SetWindowSize(GetMonitorWidth(0), GetMonitorHeight(0));
ToggleFullscreen();
  1. I had hopes this could replace BORDERLESS_WINDOWED_MODE, but, unfortunately, IMHO, it doesn't.

Environment: tested for PLATFORM_DESKTOP on Gnome 44.0 (Wayland), KDE Plasma 5.27.8 (Wayland and X11) and MATE 1.26.2 (X11) on Linux Fedora 38 64-bit.

@raysan5
Copy link
Owner

raysan5 commented Oct 7, 2023

@ubkp thank you very much for taking a look and the fast response!

  1. Yes, that's the expected behaviour.
  2. Mmmh... ToggleFullscreen() in this situation shouldn't probably do anything...
  3. Yes, agree.
  4. Ok, having another way to accomplish it shouldn't be a problem...
  5. Mmmh... I see, I though it was similar to BORDERLESS_WINDOWED_MODE?

@ghost
Copy link

ghost commented Oct 7, 2023

@raysan5 No problem. 👍

1 - Yes, but is expected for an actual fullscreen, not for a "desktop fullscreen". This was one of the main problems that BORDERLESS_WINDOWED_MODE aimed to solve (reference: #3198).

2 - I mean, if I compile 62d3066 and use it on an example that just doesn't use FLAG_FULLSCREEN_DESKTOP at all, calling ToggleFullscreen while on fullscreen won't leave fullscreen, and it should (and it will) on current master branch.

4 - Personal opinion: IMHO that's a lot of change for something that can be done with 2 calls on the user side.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 7, 2023

e47bb70 replaces restoreSize with previousScreen and also fixes the breakage of leaving regular fullscreen.

@ghost
Copy link

ghost commented Oct 7, 2023

Sorry, but:

  1. This adds something that can already be easily achieved with 2 calls;
  2. It changes stable working fullscreen code;
  3. It does not implement a real "desktop fullscreen" like SDL;
  4. I don't understand what's the issue with GLFW_DONT_CARE (doc) since:
    "GLFW_REFRESH_RATE specifies the desired refresh rate for full screen windows. A value of GLFW_DONT_CARE means the highest available refresh rate will be used. This hint is ignored for windowed mode windows."
  5. In summary, I have voiced my concerns, I want nothing to do with this change.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 7, 2023

Actually, these two calls do not work as expected:

SetWindowSize(GetMonitorWidth(0), GetMonitorHeight(0));
ToggleFullscreen();

The reason is because SetWindowSize() simply calls glfwSetWindowSize() without updating CORE.Window.screen.width and CORE.Window.screen.height. This causes ToggleFulscreen() to actually use the previous window size.

The wrong video mode problem when entering fullscreen mode can be solved by doing:

if (IsKeyPressed(KEY_F)) {
	SetWindowSize(GetMonitorWidth(0), GetMonitorHeight(0));
	fullscreen_toggled = true;
}


BeginDrawing();
EndDrawing();

if (fullscreen_toggled) {
	ToggleFullscreen();
	fullscreen_toggled = false;
}

However, this still causes a video mode change, which involves the annoying screen blackout for about a second.

When running the code example 1 below, we see "800x600" on the screen instead of the actual monitor size even in fullscreen.

Code example 1

#include <raylib.h>
#include <stdio.h>

int main()
{
	int win_width;
	int win_height;
	char str[64];

	InitWindow(800, 600, "Test");

	while (!WindowShouldClose()) {
		win_width = GetScreenWidth();
		win_height = GetScreenHeight();

		sprintf(str, "Screen size: %dx%d", win_width, win_height);

		if (IsKeyPressed(KEY_F)) {
			SetWindowSize(GetMonitorWidth(0), GetMonitorHeight(0));
			ToggleFullscreen();
		}

		BeginDrawing();
		ClearBackground(GRAY);
		DrawText(str, 0, 64, 32, BLACK);
		EndDrawing();
	}

	CloseWindow();

	return 0;
}

Code example 2

#include <raylib.h>
#include <stdio.h>
#include <stdbool.h>

int main()
{
	int win_width;
	int win_height;
	bool fullscreen_toggled = false;
	char str[64];

	InitWindow(800, 600, "Test");

	while (!WindowShouldClose()) {
		win_width = GetScreenWidth();
		win_height = GetScreenHeight();

		sprintf(str, "Screen size: %dx%d", win_width, win_height);

		if (IsKeyPressed(KEY_F)) {
			SetWindowSize(GetMonitorWidth(0), GetMonitorHeight(0));
			fullscreen_toggled = true;
		}

		BeginDrawing();
		ClearBackground(GRAY);
		DrawText(str, 0, 64, 32, BLACK);
		EndDrawing();

		if (fullscreen_toggled) {
			ToggleFullscreen();
			fullscreen_toggled = false;
		}
	}

	CloseWindow();

	return 0;
}

@ghost
Copy link

ghost commented Oct 7, 2023

Then SetWindowSize should set CORE.Window.screen.* not the other way around.
Also how GLFW_DONT_CARE won't resolve to the same frequency already being used?

@M374LX
Copy link
Contributor Author

M374LX commented Oct 7, 2023

For some reason, the mode change still happens in my system when using GLFW_DONT_CARE, but not when the current refresh rate is passed.

If everyone agrees, we can implement the first approach suggested in #3378, in which the screen size is checked and, if it is the same as the monitor, the refresh rate is passed toglfwSetWindowMonitor()instead of GLFW_DONT_CARE. In this case, there would be no need to add FLAG_FULLSCREEN_DESKTOP.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 7, 2023

Also note that the code example 2 in #3382 (comment) does not automatically restore the window size when leaving fullscreen mode. In this case, the responsibility of storing and then restoring the window size is left to the programmer, not done automatically by raylib.

@ghost
Copy link

ghost commented Oct 7, 2023

The only thing that should be fixed at this point is SetWindowSize to update CORE.

@ghost
Copy link

ghost commented Oct 7, 2023

Any fullscreen call will cause the window to be minimized if alt-tabbing. GLFW is not SDL.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 7, 2023

The only thing that should be fixed at this point is SetWindowSize to update CORE.

This and also the undesirable video mode change when the screen (or window) size is set to the same as the monitor.

Update: This is what issue #3390 is about.

Any fullscreen call will cause the window to be minimized if alt-tabbing. GLFW is not SDL.

If the user does not want this behavior, using FLAG_BORDERLESS_WINDOWED_MODE is an option, but the window remains movable, which is undesirable (maybe it is a topic for another issue). Nothing prevents FLAG_FULLSCREEN_DESKTOP and FLAG_BORDERLESS_WINDOWED_MODE from coexisting.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 8, 2023

I have done some tests with the first approach suggested in #3378, but found a considerable increase in complexity for the program's code due to it needing to manage the window restore size (which would not be managed by raylib) and also that a mode change happened when toggling between fullscreen and windowed mode multiple times too quickly.

It looks like the third approach, as implemented in this PR, allows the program's code to be simpler while adding just a few more lines of code to raylib.

Here is an example of the increased code complexity:

#include <raylib.h>
#include <stdio.h>
#include <stdbool.h>

int main()
{
	int restore_width = 0;
	int restore_height = 0;
	bool fullscreen_toggled = false;
	bool leave_fullscreen = false;
	char str[32];

	InitWindow(800, 600, "Test: fullscreen without video mode change");

	while (!WindowShouldClose()) {
		if (IsKeyPressed(KEY_F)) {
			fullscreen_toggled = true;

			if (!IsWindowFullscreen()) {
				//Change to fullscreen
				restore_width = GetScreenWidth();
				restore_height = GetScreenHeight();
				SetWindowSize(GetMonitorWidth(GetCurrentMonitor()), GetMonitorHeight(GetCurrentMonitor()));
			} else {
				leave_fullscreen = true;
			}
		}

		sprintf(str, "Screen size: %dx%d %c", GetScreenWidth(), GetScreenHeight(), IsWindowFullscreen() ? 'F' : 'W');

		BeginDrawing();
		ClearBackground(GRAY);
		DrawText(str, 0, 64, 32, BLACK);
		EndDrawing();

		if (fullscreen_toggled) {
			fullscreen_toggled = false;

			ToggleFullscreen();

			if (leave_fullscreen) {
				SetWindowSize(restore_width, restore_height);
				leave_fullscreen = false;
			}
		}
	}

	CloseWindow();

	return 0;
}

This would be simpler, but did not work as expected (probably related to SetWindowSize() not updating CORE):

#include <raylib.h>
#include <stdio.h>
#include <stdbool.h>

int main()
{
	int restore_width = 0;
	int restore_height = 0;
	bool fullscreen_toggled = false;
	char str[32];

	InitWindow(800, 600, "Test: fullscreen without video mode change");

	while (!WindowShouldClose()) {
		if (IsKeyPressed(KEY_F)) {
			fullscreen_toggled = true;

			if (!IsWindowFullscreen()) {
				//Change to fullscreen
				restore_width = GetScreenWidth();
				restore_height = GetScreenHeight();
				SetWindowSize(GetMonitorWidth(GetCurrentMonitor()), GetMonitorHeight(GetCurrentMonitor()));
			} else {
				SetWindowSize(restore_width, restore_height);
			}
		}

		sprintf(str, "Screen size: %dx%d %c", GetScreenWidth(), GetScreenHeight(), IsWindowFullscreen() ? 'F' : 'W');

		BeginDrawing();
		ClearBackground(GRAY);
		DrawText(str, 0, 64, 32, BLACK);
		EndDrawing();

		if (fullscreen_toggled) {
			fullscreen_toggled = false;
			ToggleFullscreen();
		}
	}

	CloseWindow();

	return 0;
}

@M374LX M374LX marked this pull request as ready for review October 8, 2023 18:15
@M374LX
Copy link
Contributor Author

M374LX commented Oct 8, 2023

Marking it as ready for review, as it seems to work fine on two monitors.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 8, 2023

This still does not work as expected:

SetConfigFlags(FLAG_FULLSCREEN_DESKTOP | FLAG_FULLSCREEN_MODE);

Should raylib handle this case or can we just instruct the programmer to create the window initially without FLAG_FULLSCREEN_MODE and change to fullscreen later?

Also, if the program starts in fullscreen mode, there will be no size to restore the window to when changing to windowed mode. A possible solution is to change SetWindowSize() so, while in fullscreen mode, it stores the size to set the window to when changing to windowed mode.

@ghost
Copy link

ghost commented Oct 8, 2023

Repeating myself: the only thing that should be fixed is SetWindowSize to update CORE.

@raysan5 I'm sorry, but I'm no longer wasting time reviewing or testing this.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 8, 2023

#3391 proposes a fix to SetWindowSize() not updating CORE.

@M374LX
Copy link
Contributor Author

M374LX commented Oct 9, 2023

For some reason, the mode change still happens in my system when using GLFW_DONT_CARE, but not when the current refresh rate is passed.

Turns out that the refresh rate on my system was not set to the highest available. By changing to the highest available refresh rate, a mode change did not happen with GLFW_DONT_CARE.

@raysan5
Copy link
Owner

raysan5 commented Oct 26, 2023

@M374LX I'm not implementing this change FLAG_FULLSCREEN_DESKTOP, as @ubkp commented current implementation looks solid enough for most situations and this PR looks to me as an overengineering effort to solve some very side cases, adding extra complexity to raylib.

@raysan5 raysan5 closed this Oct 26, 2023
@M374LX M374LX deleted the fullscreen-desktop branch February 29, 2024 22:36
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.

3 participants