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

SCREEN_TEXTURE is not being updated for subsequent shaders even when BackBufferCopy is used for each of them #56873

Closed
saki7 opened this issue Jan 17, 2022 · 2 comments · Fixed by #65794

Comments

@saki7
Copy link
Contributor

saki7 commented Jan 17, 2022

Godot version

master (8958e1b)

System information

Windows 10, GTX1080Ti (511.23), Vulkan

Issue description

SCREEN_TEXTURE is not being updated for subsequent shaders even when BackBufferCopy is used for each of them. In other words, the first captured buffer is always returned when multiple shaders (each BackBufferCopy-parented) exist as siblings.

If I understand correctly, on 2D, backbuffer is fetched only once when SCREEN_TEXTURE appears for the first time. However, when BackBufferCopy is used, backbuffer should be copied every time.

Quoting Godot Contributors Chat (link):

<mux213> saki7: if I recall correctly the back buffer copy is only executed once per frame, right before the very first shader that requires SCREEN_TEXTURE is run. Could that be your issues?

<reduz> mux213: for 2D its a bit more fine grained
for 3D it only does once before drawing transparent objects

Expected Result Actual Result
godot_test_expected godot_test_actual

Shader

shader_type canvas_item;

void fragment() {
    vec3 c = texture(SCREEN_TEXTURE, SCREEN_UV).rgb;
    COLOR = vec4(c * 0.7, 1.0);
}

Steps to reproduce

Scene Tree

  • PanelContainer
    • BackBufferCopy
      • Sprite2D (icon.png)
    • BackBufferCopy
      • Sprite2D (arbitrary image + shader with SCREEN_TEXTURE)
    • BackBufferCopy
      • Sprite2D (arbitrary image + shader with SCREEN_TEXTURE)

Steps

  1. Clone master, build it, run it
  2. Setup a scene tree like above
  3. Run the scene
  4. You will see the "Actual Result" I posted above
  5. On editor's FileSystem panel, click on the .gdshader file which is attached to your node
  6. You will see multiple errors like below

This one is tracked in #56511:

ERROR: Height must be equal or greater than 1 for 2D and 3D textures
   at: (drivers/vulkan/rendering_device_vulkan.cpp:1756)

Other errors related to images, uniforms and textures:
(See #50296 for similar reports)

ERROR: Image (binding: 0, index 0) is not a valid texture.
   at: (drivers/vulkan/rendering_device_vulkan.cpp:5712)
ERROR: Condition "!uniform_set" is true.
   at: compute_list_bind_uniform_set (drivers/vulkan/rendering_device_vulkan.cpp:7888)
ERROR: Uniforms were never supplied for set (3) at the time of drawing, which are required by the pipeline
   at: (drivers/vulkan/rendering_device_vulkan.cpp:8087)
ERROR: Texture (binding: 0, index 0) is not a valid texture.
   at: (drivers/vulkan/rendering_device_vulkan.cpp:5604)
ERROR: Condition "!uniform_set" is true.
   at: compute_list_bind_uniform_set (drivers/vulkan/rendering_device_vulkan.cpp:7888)
...
... (repeated infinitely)
...
  1. Choose one PR from below, apply it on master, rebuild godot
    Warning: These are three independent approaches to fix the similar errors, and we only need ONE of them to fix the errors here. That's for you to decide.
    7-a. Trigger material update on texture update #52528 (note that you need to resolve git conflict manually as of current master)
    7-b. Fix 3D materials not updating on texture invalidation #54370
    7-c. Fix materials not updating when texture replaced/deleted (v2) #54489
  2. Run the scene again
  3. All errors disappear
  4. You will still be seeing the "Actual Result", not the "Expected Result"

Minimal reproduction project

n/a

Workaround

--- a/servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp
+++ b/servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp
@@ -1365,7 +1365,7 @@ void RendererCanvasRenderRD::canvas_render_items(RID p_to_render_target, Item *p
                        MaterialData *md = (MaterialData *)storage->material_get_data(material, RendererStorageRD::SHADER_TYPE_2D);
                        if (md && md->shader_data->valid) {
                                if (md->shader_data->uses_screen_texture && canvas_group_owner == nullptr) {
-                                       if (!material_screen_texture_found) {
+                                       if (true || !material_screen_texture_found) { // FIXME: Workaround BackBufferCopy not being refreshed
                                                backbuffer_copy = true;
                                                back_buffer_rect = Rect2();
                                        }

By applying this workaround, you will see the "Expected Result".

Note: This workaround force-updates all backbuffer when SCREEN_TEXTURE is used in a shader, regardless of the existence of BackBufferCopy. It will destroy the concept of caching SCREEN_TEXTURE in the first place, so this patch should not be used in a production build.

Related Issues

#50296
#52505
#47504 -- This was the most similar issue, but it was on 3.x branch, and it was correctly resolved by using BackBufferCopy for each shader nodes. My issue, however, is on 4.x branch, and it can not be resolved by same approach.

Other stuff that might be affected

The Screen-reading shaders example on official documentation will not work until this issue is fixed.

@akien-mga
Copy link
Member

akien-mga commented Jan 19, 2022

Some of the related issues you linked have been fixed by #54489, might be worth retesting your use case to see if anything changed.

Edit: I see you tested that already and documented the outcome.

@markdibarry
Copy link
Contributor

markdibarry commented Apr 15, 2022

Confirmed, the BackBufferCopy node does nothing at the moment. Here's a scene in 3.4.4
image
and the same scene in a build made today:
image

and a minimal repro:
BackBufferCopyBug.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants