godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.71k stars 20.82k forks source link

Compute shaders execute before calling submit() #92706

Open emberlightstudios opened 4 months ago

emberlightstudios commented 4 months ago

Tested versions

Reproducible in 4.2.2

System information

Fedora 40 - Vulkan - AMD 6800s - amdgpu driver

Issue description

I'm writing an addon that wraps the compute shader API.

https://github.com/matt-seaton/compute_companion

This is a fork of ComputeWorker here: https://github.com/wardensdev/ComputeWorker

Just discovered the shader is being executed after dispatching the compute list. It seems to run without calling submit/sync at all. I believe this is the expected behavior if using the global RenderingDevice but I am not. I also checked rd == RenderingServer.get_rendering_device() and it returned false. I don't understand it at all. Why does it run if I'm not calling submit? Is it related to the fact that the code is in a @tool script? It is the same behavior if I run the scene though.

Steps to reproduce

Load the MRP and in the addon code, in the examples folder, load the color inversion scene. Press the checkbox in the inspector and the texture will update. In the script I have commented out the call to compute.execute() which is the only place rd.submit() is called.

Minimal reproduction project (MRP)

bug.zip

clayjohn commented 3 months ago

Do you mind putting together an MRP for this? I've taken a look at your project and it is huge and complex. I can see a few places where rd.submit() may be called, but I don't want to debug your entire application to figure out if the bug is coming from your application or from Godot.

If the problem is coming from Godot you should be able to reproduce the issue with a single GDScript file without dependencies.

Normally I would try running the project anyway just to confirm that I can see the same thing you do (without trying to analyze where the problem comes from) but this project requires running a 20 mb tscn. Naturally, that is a huge security risk. So just for that reason I ask that you create an MRP that is truly minimal.

emberlightstudios commented 3 months ago

I agree the project is somewhat complex for an MRP, but there is only one place where submit() is called as is easily confirmed by grep -ir --include='*.gd' submit

So it's actually quite simple to see the problem.

You are correct that there are multiple code paths that can end up there, but that is the only place it can be called from. And I can assure you that I am not going down any of those code paths. The only function I call that would end up there is compute.execute.

But you don't have to take my word for it.

You can even comment out the line rd.submit() (which again is the only place that call exists in the addon) and the shader still runs for me. I also put a print statement in that function to see if I ever ended up there. I did not, but the shader executes anyway.

emberlightstudios commented 3 months ago

I'm seeing a warning about Invalid pipelines cache header. Not sure what that's about.

emberlightstudios commented 3 months ago

The reason the tscn is so large is because the shader outputs an image in RGBAF format with 32 bits per channel. Here's a simplified MRP with the same scene with just the original noise texture before going through the shader. That should reduce the scene size. bug.zip

Calinou commented 3 months ago

I'm seeing a warning about Invalid pipelines cache header. Not sure what that's about.

You can ignore that warning; it's been removed in 4.3 for the reasons described in https://github.com/godotengine/godot/pull/80232.

DarioSamo commented 3 months ago

There's a few situations where RenderingDevice will submit all the work it has pending, flush and wait for its execution.

It should be easy to determine when you're hitting one of these by setting a breakpoint when RenderingDevice requires this.