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

RenderingServer::create_local_rendering_device - Add null check and update docs #68884

Conversation

dzil123
Copy link
Contributor

@dzil123 dzil123 commented Nov 19, 2022

Fixes #66372

@dzil123 dzil123 requested review from a team as code owners November 19, 2022 11:26
@@ -838,6 +838,8 @@
<method name="create_local_rendering_device" qualifiers="const">
<return type="RenderingDevice" />
<description>
Creates a RenderingDevice to do all local drawing and compute on any thread. Cannot draw to the screen nor share data with the global RenderingDevice.
[b]Note:[/b] When using the OpenGL backend or when running in headless mode, this function always returns [code]null[/code].
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 this is intentional for headless, or just that the implementation is missing? I could imagine users wanting to run headless on a system with a GPU to do compute stuff.

Still, if that's the current state the documentation is correct, we should just open an issue if we think that headless RenderingDevice ought to be supported.

Copy link
Member

@Calinou Calinou Nov 19, 2022

Choose a reason for hiding this comment

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

--headless does not have a way to use the GPU, as it doesn't spawn a window. This would require implementing off-screen rendering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think there would be interest in a feature to make a compute-only local rendering device, while the rest of the engine is headless? There would be no window created and no off-screen rendering.

I have a toy example capable of running the compute demo on Linux without a display: master...dzil123:godot:headless_local_renderingdevice.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think that would make sense. This implementation would need more work of course as there shouldn't be X11 includes in servers, platform-specific code should stay self-contained within in platform folder.

Copy link
Contributor Author

@dzil123 dzil123 Nov 21, 2022

Choose a reason for hiding this comment

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

platform-specific code should stay self-contained within in platform folder

Right, of course. The specific implementation difficulty for that is the need to get the platform specific VulkanContext impl. Currently only the platform specific DisplayServers reference the specific VulkanContext, but I can't use the DisplayServer singleton to construct the VulkanContext because on headless the singleton is DisplayServerHeadless. This may need a mechanism like DisplayServer::create_func.

There is also the question of whether creating a VulkanContext should be allowed if using the OpenGL renderer, how to pick between RenderingDeviceVulkan and other apis like D3D12, and in general how this feature should be controlled through project settings or the CLI.

Co-authored-by: Clay John <claynjohn@gmail.com>
@dzil123 dzil123 force-pushed the create_local_rendering_device_null_check_docs branch from 250b811 to 9ce9c95 Compare November 20, 2022 00:32
@akien-mga akien-mga added this to the 4.0 milestone Nov 20, 2022
@akien-mga akien-mga merged commit 4a52fb8 into godotengine:master Nov 20, 2022
@akien-mga
Copy link
Member

Thanks!

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.

Executing RenderingServer.create_local_rendering_device crashes Godot
4 participants