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

Remove GPU readback from NoiseTexture3D.get_format() #80407

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Aug 8, 2023

I caught this issue while reviewing another PR. When accessing the format of the NoiseTexture3D it was requesting a GPU->CPU copy of the texture data, then throwing away the texture data and only using the format. This is not only an expensive operation, it also causes a full GPU->CPU sync making it very expensive to call at runtime.

This is a very low risk bug fix

Bugsquad edit:

@clayjohn clayjohn added bug topic:rendering performance cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Aug 8, 2023
@clayjohn clayjohn added this to the 4.2 milestone Aug 8, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good.

@akien-mga akien-mga merged commit ff1f50f into godotengine:master Aug 8, 2023
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov added the crash label Aug 8, 2023
@clayjohn clayjohn deleted the NoiseTexture3D-format branch August 8, 2023 15:59
@akien-mga akien-mga changed the title Remove GPU readback from NoiseTexture3D.get_format() Remove GPU readback from NoiseTexture3D.get_format() Aug 11, 2023
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 20, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.2.

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