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

[re_renderer] Uniform buffer utility using CpuWriteGpuReadBelt #1400

Merged
merged 10 commits into from
Feb 27, 2023

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Feb 24, 2023

Used for all uniform buffers except meshes - need some more plumbing there, will be done in a follow-up Pr!

This PR introduces a new utility for easy uniform buffer creation & filling. It enforces correct padding - see long comment for details :)
Removes quite a few landmines and makes code in Renderers shorter!

Solves a sizeable chunk of #1398, figured out some RenderContext share issues along the way

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I've added a line to CHANGELOG.md (if this is a big enough change to warrant it)

@teh-cmc teh-cmc added 🔺 re_renderer affects re_renderer itself 🧑‍💻 dev experience developer experience (excluding CI) 📉 performance Optimization, memory use, etc labels Feb 26, 2023
@teh-cmc teh-cmc self-requested a review February 26, 2023 15:05
/// But this leads to more unsafe code, harder to avoid holes in write combined memory access
/// and potentially undefined values in the padding bytes on GPU.
const CHECK: () = assert!(
std::mem::align_of::<T>() >= 256,
Copy link
Member

Choose a reason for hiding this comment

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

I'd expect an == here?

Suggested change
std::mem::align_of::<T>() >= 256,
std::mem::align_of::<T>() == 256,

Copy link
Member Author

Choose a reason for hiding this comment

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

Higher alignment is unlikely but absolutely allowed. If something is aligned to 512 we're okay with that as well (but I really hope nothing is!)

@Wumpf Wumpf merged commit aec0e87 into main Feb 27, 2023
@Wumpf Wumpf deleted the andreas/re_renderer/fast-and-easy-uniform-buffer-creation branch February 27, 2023 09:05
emilk pushed a commit that referenced this pull request Mar 2, 2023
* utility function for uniform buffer from struct
* Uniform buffers are now compile time forced to be 256 bytes aligned
* CpuWriteGpuReadBelt no longer takes mutable buffer pool
* Renderers and frame_global_command_encoder are now behind locks
---------

Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI) 📉 performance Optimization, memory use, etc 🔺 re_renderer affects re_renderer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants