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 ability to call code on rendering thread #79696

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

reduz
Copy link
Member

@reduz reduz commented Jul 20, 2023

As more users use compute in Godot 4, the way they do is most likely incompatible when running on separate threads and will start erroring soon as we improve the thread safety of the render thread.

To properly run code on the render thread, this function was added. Use like this:

func initialize_compute_code():
	....

func update_compute_code(custom_data):
	...

func _ready():
	RenderingServer.call_on_render_thread( initialize_compute_code )

func _process():
	RenderingServer.call_on_render_thread( update_compute_code.bind(with_data) )

@reduz reduz requested review from a team as code owners July 20, 2023 09:54
@clayjohn
Copy link
Member

For clarity, this is only needed for compute code that needs to synchronize with the main RenderingDevice. Compute code running on a local rendering device can be managed from any thread

@yankscally
Copy link

I just ran into this issue earlier, to me it was behaving unexpectedly because I assumed the RenderingServer was not on a seperate thread and calling code in order did not produce the results I expected. Also I had a bad case of 'didn't read docs' today.

here is my report:
#79691

This change would definitely be appreciated now that I understand the problem!

@lyuma
Copy link
Contributor

lyuma commented Jul 20, 2023

@yankscally to be clear, the problem you are trying to solve is synchronizing texture resources for CPU readback?
As in, get_viewport().get_texture().get_image()

I just want to clarify the issue of not being able to directly access the node from another thread. Note that Node apis may not be called from another thread. Let's for the moment assume you have converted the ViewportTexture into a RID that can be safely accessed from the render thread.

I'm not sure about the performance of doing a synchronous CPU readback of texture data from the rendering thread. I'd definitely be worried about calling get_image() regularly, but in your case you are specifically doing this to get a screenshot, so I assume a stall / stutter is expected? (though ideally there would be a way to do this without a stutter)
I'll wait for someone more involved with rendering to respond on the question of synchronous readback of Image data

@reduz
Copy link
Member Author

reduz commented Jul 20, 2023

@lyuma That is a different use case unrelated to this PR, how to do it it is documented in the function itself.

@BastiaanOlij
Copy link
Contributor

@reduz I was wondering about something, and this may not be possible, but seeing the thread group logic you added to nodes, would it be possible to mark a node as being run on the rendering thread?

I guess this would add a lot more difficulty in supporting functions like _ready and require a whole lot more implementation than a "simple" call queue on the rendering thread, but just thought I'd mention it as a discussion topic.

Other than that, fully approve of this approach.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks fine to me!

Note for merging, this should not be cherrypicked. But can be merged any time for 4.2

As more users use compute in Godot 4, the way they do is most likely incompatible when running
on separate threads and will start erroring soon as we improve the thread safety of the render thread.

To properly run code on the render thread, this function was added. Use like this:

```GDScript

func initialize_compute_code():
	....

func update_compute_code(custom_data):
	...

func _ready():
	RenderingServer.call_on_render_thread( initialize_compute_code )

func _process():
	RenderingServer.call_on_render_thread( update_compute_code.bind(with_data) )

```
@YuriSizov YuriSizov merged commit 79d3468 into godotengine:master Jul 31, 2023
@YuriSizov
Copy link
Contributor

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.

6 participants