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

Implement render_target_was_used API so that Viewports can properly check if they have been used. #70132

Merged
merged 1 commit into from
Dec 17, 2022

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Dec 16, 2022

Fixes: #55471

In GLES3 we have to iterate over textures to bind them so at the same time we can just check if the textures are render targets and set them to used. In the RD renderer we need to carry a reference to the render targets as we bake the textures into a uniform set and only use that at draw time.

Also fixes a bug in the GLES3 renderer that caused TEXTURE to override the first texture uniform.

Testing in all three renderers and using the Viewport as a TEXTURE, a uniform (in both canvas item and spatial shaders), and as a child of subviewportcontainer

cc @Sauermann

@Sauermann
Copy link
Contributor

I have tested this PR and it solves the following problem for me:
When using a SubViewport with render_target_update_mode = UPDATE_WHEN_VISIBLE as a texture for a TextureRect, then the TextureRect is displayed completely black.

In my setting I was unable to recreate the Uniforms were never supplied for set (3) at the time of drawing, which are required by the pipeline problem of the linked PR (not sure, how to do that).

@clayjohn
Copy link
Member Author

For the RD renderer, this does not work for Viewports used in scene shaders yet as they cache their uniform sets both in the MaterialData struct and in the surface cache. I don't want to pass a reference to the render targets in the surface cache as we need to keep it as small as possible.

I think I have an idea to get around this. Will try it out tomorrow.

…heck if they have been used.

For the RD renderer, this does not work for Viewports used in scene shaders yet
@clayjohn
Copy link
Member Author

Should be ready for review and merging now.

@akien-mga akien-mga merged commit 676f60b into godotengine:master Dec 17, 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.

Vulkan: SubViewports not functioning unless update mode is set to "Always"
3 participants