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

Fix LightmapGI crash when using --headless #93483

Closed
wants to merge 1 commit into from

Conversation

jamie-pate
Copy link
Contributor

@jamie-pate jamie-pate commented Jun 22, 2024

When headless lightmaps rid is 0 causing it to crash when assigning the lightmap.

Fixes #89119

@jamie-pate jamie-pate requested a review from a team as a code owner June 22, 2024 19:36
@jamie-pate
Copy link
Contributor Author

Tested with the MRP and no crashing, also fixed the crash in my project while running tests with --headless

@jamie-pate jamie-pate changed the title Fixes #89119 LightmapGI causes crash when using --headless Fixes LightmapGI crash when using --headless Jun 22, 2024
@AThousandShips AThousandShips changed the title Fixes LightmapGI crash when using --headless Fix LightmapGI crash when using --headless Jun 23, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Jun 23, 2024
@AThousandShips AThousandShips added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jun 23, 2024
@RandomShaper
Copy link
Member

I'm not very sure if this is the right way to handle this. It may be better than headless provides non-zero RIDs, so everything can work transparently. Otherwise, we need to add that sort of checks here and there. Those checks are easy to forget to add
because one is generally debugging on a rendering-enabled session. Also, such checks can be confusing for maintainers as one has to wonder why is there a null check somewhere, which can lead to wrong assumptions.

@jamie-pate
Copy link
Contributor Author

there are other places that do a null check on this property, but I agree that providing a dummy rid would be more robust..
That approach is a bit over my head though.

Fixes godotengine#73374

As of godot 4 On windows/osx the game window will be frozen and will not
be updated.

In the debugger loop it calls

OS::get_singleton()->process_and_drop_events();
which allows windows/osx to handle system events. If the window doesn't
handle these events then both systems will judge the window to be 'not
responding' (osx beachball cursor)

When the event processing code was migrated from OS to DisplayServer the
process_and_drop_events() logic was moved to DisplayServer, but the call
inside the remote debugger pause loop was not updated to call the
DisplayServer version, there are currently no implementations of
OS::process_and_drop_events() so i removed it and switched to the new
DisplayServer::force_process_and_drop_events() method.
@jamie-pate jamie-pate requested review from a team as code owners July 10, 2024 20:44
@jamie-pate jamie-pate closed this Jul 10, 2024
@Calinou Calinou added archived and removed cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jul 10, 2024
@AThousandShips AThousandShips removed this from the 4.3 milestone Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LightmapGI causes crash when using --headless
4 participants