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

Direct3D 12: Enhance management of texture data life cycle #87872

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Feb 2, 2024

  1. When available, graphics memory is asked not to be zero-initialized.

    That brings better performance since in most cases those bytes will be overwritten anyway.

2. Texture memory initialization is tracked across the limbo, garbage and done states.

- Limbo: resource just allocated, contents are dangerous unless overwritten by certain operations (like a clear), or discarded.
- Garbage: the memory has been discarded (i.e., D3D12 knows it has to ignore previous contents), but it's still not guaranteed to be black, orthogonally to the non-zero-mem feature.
- Done: the texture has known contents, either because it was updated with some image data or because it was cleared, worst case by this code, to black, to ensure garbage data is never used.

UPDATE: Approach simplified. It turns out that texture with many many subresources would add a lot of strain.

  1. Texture memory initialization is guaranteed by clearing in black every texture that can be a UAV or RTV.

@akien-mga
Copy link
Member

Needs a rebase, and a re-review.

@RandomShaper
Copy link
Member Author

Rebased!

@DarioSamo
Copy link
Contributor

PR currently does not handle D3D12_SRV_DIMENSION_TEXTURECUBEARRAY in _make_rtv_for_texture, so it crashes in dev mode with the following call stack due to the assert.

image
image
image

I could reproduce this on the 3D Platformer project, probably due to the use of reflection probes.

Copy link
Contributor

@DarioSamo DarioSamo left a comment

Choose a reason for hiding this comment

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

After fixing the error I mentioned above at least everything seems to still work, but the behavior regarding the excessive amount of clears likely needs to be reviewed, as the comment indicates it shouldn't happen often but it triggers often enough to go above the default limit.

@DarioSamo
Copy link
Contributor

Approved. The new approach to clearing the textures seems a lot cleaner and also requires far less tracking that can hurt performance.

@akien-mga akien-mga merged commit cfe344f into godotengine:master Feb 27, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@RandomShaper RandomShaper deleted the d3d12_tex_mem_is_life branch February 27, 2024 16:03
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.

3 participants