-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Improve handling of rendering startup errors #93706
Conversation
bc19837
to
32d9c93
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Thanks! |
@@ -6261,8 +6261,14 @@ DisplayServerX11::DisplayServerX11(const String &p_rendering_driver, WindowMode | |||
|
|||
#if defined(RD_ENABLED) | |||
if (rendering_context) { | |||
rendering_device = memnew(RenderingDevice); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed one line too many here... not sure why CI didn't complain, but then it broke once merged: https://github.com/godotengine/godot/actions/runs/9723676850/job/26839084653
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed up with 25de53e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure why CI didn't complain
Seems like the CI never ran because GH was broken yesterday: https://www.githubstatus.com/incidents/9vwllhs2w1kj
I wish they'd make that more explicit before merging.
The line was removed by mistake.
The line was removed by mistake.
The line was removed by mistake.
The line was removed by mistake.
The line was removed by mistake.
The line was removed by mistake.
Makes
RenderingDevice
startup fail if staging buffers can't be created. Also allows any error in such initialization to take the engine through the path where it can handle it so it can eventually show the message box that suggests using GL ES 3, etc.Despite this, the engine still crashes after that (at least in my case and with a dev build). However, this is already a step forward so in cases such as the one in #93670 users get at least some friendly error instead of a plain crash with no further info.