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

[rcore][RGFW] bug fixes #4798

Merged
merged 7 commits into from
Mar 10, 2025
Merged

[rcore][RGFW] bug fixes #4798

merged 7 commits into from
Mar 10, 2025

Conversation

ColleagueRiley
Copy link
Contributor

@ColleagueRiley ColleagueRiley commented Feb 25, 2025

  • ensure functions that use platform.window check platform.window is valid first
  • ensure fullscreen toggling is working properly
  • ensure raylib responds to RGFW_quit properly
  • update RGFW
  • handle RGFW_windowMaximized, RGFW_windowMinimized and RGFW_windowRestored events
  • properly handle raylib's FLAG_WINDOW_UNFOCUSED flag with RGFW_windowFocusOnShow
  • remove out-of-date code

@sleeptightAnsiC
Copy link
Contributor

ensure functions that use platform.window check platform.window is valid first

This is something similar that rcore_desktop_glfw.c does inside of WindowShouldClose() but no other function has those kind of checks. Other backends also don't have them. If this gets accepted, can we also port it to SDL, GLFW, etc? @raysan5

(also linking this to #4751 )

@raysan5
Copy link
Owner

raysan5 commented Feb 28, 2025

@ColleagueRiley @sleeptightAnsiC In raylib I tried to avoid/minimize this kind of checks, spreaded all around the code on all functions. Can it be checked only on InitWindow()?

@sleeptightAnsiC
Copy link
Contributor

sleeptightAnsiC commented Feb 28, 2025

Can it be checked only on InitWindow()?

It should be possible to early return from InitWindow() and then someone could simply check for the errors with IsWindowReady() and abort/recover accordingly.

Currently, there is a problem with raylib's initialization where, if a platform cannot provide Window and/or OpenGL context, the whole application will crash deep inside of rcore backend code, without any possibility to recover. I wanted to report/patch this, but then I noticed a similar PR #4164 where you rejected the fix @raysan5 Would it be fine, if I try to make another Issue and PR for this problem? (EDIT: I just reported the mentioned problem here: #4801 )

@raysan5
Copy link
Owner

raysan5 commented Feb 28, 2025

Currently, there is a problem with raylib's initialization where, if a platform cannot provide Window and/or OpenGL context, the whole application will crash deep inside of rcore backend code, without any possibility to recover.

@sleeptightAnsiC Could it be just validated if the process (window+context creation) worked properly and just show a TraceLog() message showing the issue (like most raylib functions approach) if not initialized and then just set CORE.Window.ready to false for further user check?

@sleeptightAnsiC
Copy link
Contributor

@sleeptightAnsiC Could it be ...

Replied under linked issue.

@ColleagueRiley
Copy link
Contributor Author

If the window is not valid or is invalidated, there will be a confusing crash.

This check is there to avoid the crash, but I agree that this is not how it should be done. Instead of the if-statement, there should be an assert on each function platform.window so they crash in an obvious way if they are used before initialization or after deinitialization.

If you think there shouldn't be any asserts or checks, I will remove them. But they could be useful if something goes wrong.

That other issue is not relevant for RGFW internally, RGFW fallbacks to a basic OpenGL context if it fails to load the accelerated context. The crash will still happen because RLGL does not use a fallback, but that could be easily added.

As for SetWindowState: It assumes that the user might try setting the state before the window is created and modifies flags that can only take effect when creating the window. that may not be a correct assumption.

@@ -303,8 +308,7 @@ void ToggleFullscreen(void)
// Toggle borderless windowed mode
void ToggleBorderlessWindowed(void)
{
if (platform.window == NULL)
return;
if (platform.window == NULL) return;
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer to avoid adding that check to all the functions, if that check is required I think it should be implemented in RGFW library side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, they exist on the RGFW side as asserts.

@raysan5 raysan5 changed the title PLATFORM_DESKTOP_RGFW bug fixes [rcore][RGFW] bug fixes Mar 2, 2025
@raysan5 raysan5 merged commit 0853c5b into raysan5:master Mar 10, 2025
15 checks passed
@raysan5
Copy link
Owner

raysan5 commented Mar 10, 2025

@ColleagueRiley thanks for the review!

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