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

2D: Fix CanvasTexture rendering when updating channels #101931

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

stuartcarnie
Copy link
Contributor

@stuartcarnie stuartcarnie commented Jan 22, 2025

Closes #101702
Closes #101865

This PR introduces an invalidation callback API to TextureStorageRD for CanvasTexture, so the 2D renderer can invalidate associated uniform sets when a texture channel, such as diffuse, is modified.

CleanShot 2025-01-23 at 06 45 06

stuartcarnie added a commit to stuartcarnie/godot that referenced this pull request Jan 23, 2025
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected. Code looks good to me.

To test this, try changing the texture of the Background node in https://github.com/Calinou/godot-rendering-tests' tests/2d/test_point_light_2d.tscn to a CanvasTexture with normal and specular maps.

@clayjohn
Copy link
Member

Looks good to me! Did you test if this closes #101865 as well?

@stuartcarnie
Copy link
Contributor Author

@clayjohn thanks for the tip – I was funnily enough going to try some code, but great there is an MRP already. Will report back.

TightLocalVector<RID> *vec = static_cast<TightLocalVector<RID> *>(p_userdata);
RD *rd = RD::get_singleton();
for (RID rid : *vec) {
// the invalidation callback will take care of clearing rid_set_to_uniform_set cache also
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// the invalidation callback will take care of clearing rid_set_to_uniform_set cache also
// The invalidation callback will also take care of clearing rid_set_to_uniform_set cache.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shoot, how'd I miss these. Ehh, we'll just get 'em in a formatting pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aye, sorry about that!

@@ -2992,6 +3007,19 @@ void RendererCanvasRenderRD::_render_batch(RD::DrawListID p_draw_list, CanvasSha
const RIDCache::Pair *iter = rid_set_to_uniform_set.insert(key, rid);
uniform_set = &iter->data;
RD::get_singleton()->uniform_set_set_invalidation_callback(rid, RendererCanvasRenderRD::_uniform_set_invalidation_callback, (void *)&iter->key);

// If this is a CanvasTexture, it must be tracked so that any changes to the diffuse, normal
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// If this is a CanvasTexture, it must be tracked so that any changes to the diffuse, normal
// If this is a CanvasTexture, it must be tracked so that any changes to the diffuse, normal,

@Repiteo Repiteo merged commit 21e1115 into godotengine:master Jan 24, 2025
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 24, 2025

Thanks!

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