Skip to content

Migrate to encase from crevice for uniform and storage buffer data formatting #4272

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.

Sign up for GitHub

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

Jump to bottom
Closed
superdump opened this issue Mar 20, 2022 · 7 comments
Closed

Migrate to encase from crevice for uniform and storage buffer data formatting #4272

superdump opened this issue Mar 20, 2022 · 7 comments
Labels
A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@superdump
Copy link
Contributor

superdump commented Mar 20, 2022 •
edited
Loading

Q: Should we migrate from using vendored crevice to using encase for formatting of data for uniform and storage buffers?

Noteworthy benefits of encase:

  • supports runtime-sized Vec in the Rust struct types
    • it has validation to ensure a Vec is only used where allowed (i.e. at the end of a storage buffer type)
    • it has a convenient way of writing the runtime-sized array length into the binding buffer
  • supports configurable dynamic offset alignments for both uniform and storage buffers
    • crevice only supports configuring alignment for dynamic offsets for std140 for uniform buffers
    • crevice DynamicUniformOffset enforces alignment of max(256, T::ALIGNMENT), so at least 256 in all cases
    • the alignment requirement of 256 for dynamic offsets for uniform buffers is often prohibitive for packing as much data as possible into uniform buffers for WebGL2 where uniform buffer binding sizes are limited to 16kB
    • the required minimum alignments vary by platform, graphics API, and GPU vendor
  • the author has been active on our Discord, receptive to feedback/input, responsive in fixing issues we've noted so far, and has created a branch using encase in bevy here: https://github.com/teoxoy/bevy/tree/encase
@superdump superdump added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Dependencies A change to the crates that Bevy depends on C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Mar 20, 2022
@superdump superdump moved this to Candidate in @cart's attention Mar 20, 2022
@superdump
Copy link
Contributor Author

superdump commented Mar 20, 2022 •
edited
Loading

As a note, I didn't look through the internals of encase yet. The API docs look nice, and I like the structure of the APIs too in that there is one type derivation (WgslType that may be renamed to ShaderType) and then depending on whether you use a UniformBuffer or a StorageBuffer the alignment and padding is handled as appropriate. Also, you can configure if you're going to use dynamic offsets into the buffer, and as mentioned in the noteworthy benefits, you can configure the dynamic offset alignment according to what the platform/graphics API/GPU support/require.

@mockersf
Copy link
Member

We would need a way to have the WgslType derive macro be usable without having a direct dependency on encase.
This was done in bevy_crevice with this: https://github.com/bevyengine/bevy/blob/main/crates/bevy_crevice/bevy-crevice-derive/src/lib.rs#L37

@superdump
Copy link
Contributor Author

And I don't think we should switch before 0.7 unless we're somehow very confident. This decision is rather to unblock some changes I have to use storage buffers for point lights and some changes needed to the helper impls for more efficient handling of binding data for animations.

@teoxoy
Copy link
Contributor

teoxoy commented Mar 21, 2022

We would need a way to have the WgslType derive macro be usable without having a direct dependency on encase.

I tried to find a way to get the derive to work when reexported but without luck.

I only found serde's derives can be reexported like here but that extra attribute has to be on every struct with the derive. I'd rather have a direct dependency than do that.

@mockersf
Copy link
Member

I tried to find a way to get the derive to work when reexported but without luck.

To get it to work without an extra attribute, encase would need to know the path where Bevy will reexport the macro. It can be done behind a "bevy" feature flag without needing to depend on Bevy. Where it becomes painful is if Bevy want to change the reexport path

@teoxoy
Copy link
Contributor

teoxoy commented Mar 21, 2022 •
edited
Loading

I don't think that will be the best approach since feature flags are supposed to be additive. If the user would like to have encase as a direct dependency (or they have some other crate depending on encase) while also having bevy in their dependencies, the derive imported from encase will stop working.

Is there a big advantage in wanting to reexport rather than direct dependency?

@superdump superdump moved this from Candidate to Respond (With Priority) in @cart's attention Mar 21, 2022
@teoxoy teoxoy mentioned this issue Mar 26, 2022
Closed
@bzm3r bzm3r mentioned this issue May 6, 2022
Closed
bors bot pushed a commit that referenced this issue May 18, 2022
@teoxoy @cart
Migrate to encase from crevice (#4339)
7cb4d3c 
# Objective

- Unify buffer APIs
- Also see #4272

## Solution

- Replace vendored `crevice` with `encase`

---

## Changelog

Changed `StorageBuffer`
Added `DynamicStorageBuffer`
Replaced `UniformVec` with `UniformBuffer`
Replaced `DynamicUniformVec` with `DynamicUniformBuffer`

## Migration Guide

### `StorageBuffer`

removed `set_body()`, `values()`, `values_mut()`, `clear()`, `push()`, `append()`
added `set()`, `get()`, `get_mut()`

### `UniformVec` -> `UniformBuffer`

renamed `uniform_buffer()` to `buffer()`
removed `len()`, `is_empty()`, `capacity()`, `push()`, `reserve()`, `clear()`, `values()`
added `set()`, `get()`

### `DynamicUniformVec` -> `DynamicUniformBuffer`

renamed `uniform_buffer()` to `buffer()`
removed `capacity()`, `reserve()`


Co-authored-by: Carter Anderson <[email protected]>
@superdump
Copy link
Contributor Author

This was addressed by #4339 which is now merged. Closing.

Repository owner moved this from In Review to Done in Rendering May 31, 2022
bors bot pushed a commit that referenced this issue Jun 21, 2022
Documenting BufferVec. (#4673)
498f41e 
# Objective

Documents the `BufferVec` render resource.

`BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](#4272)) in the future. For now, since it is still used by some simple 
example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful
to provide some simple documentation on what `BufferVec` does.  

## Solution

I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. 

I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). 

---

## Changelog

- Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. 


Co-authored-by: Brian Merchant <[email protected]>
bors bot pushed a commit that referenced this issue Jun 28, 2022
Documenting BufferVec. (#4673)
e60f614 
# Objective

Documents the `BufferVec` render resource.

`BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](#4272)) in the future. For now, since it is still used by some simple 
example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful
to provide some simple documentation on what `BufferVec` does.  

## Solution

I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. 

I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). 

---

## Changelog

- Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. 


Co-authored-by: Brian Merchant <[email protected]>
james7132 pushed a commit to james7132/bevy that referenced this issue Jul 2, 2022
@james7132
Documenting BufferVec. (bevyengine#4673)
255e306 
# Objective

Documents the `BufferVec` render resource.

`BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple 
example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful
to provide some simple documentation on what `BufferVec` does.  

## Solution

I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. 

I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). 

---

## Changelog

- Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. 


Co-authored-by: Brian Merchant <[email protected]>
inodentry pushed a commit to IyesGames/bevy that referenced this issue Aug 8, 2022
@inodentry
Documenting BufferVec. (bevyengine#4673)
bdcf403 
# Objective

Documents the `BufferVec` render resource.

`BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple 
example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful
to provide some simple documentation on what `BufferVec` does.  

## Solution

I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. 

I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). 

---

## Changelog

- Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. 


Co-authored-by: Brian Merchant <[email protected]>
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
@james7132
Documenting BufferVec. (bevyengine#4673)
32a32d0 
# Objective

Documents the `BufferVec` render resource.

`BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple 
example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful
to provide some simple documentation on what `BufferVec` does.  

## Solution

I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. 

I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). 

---

## Changelog

- Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. 


Co-authored-by: Brian Merchant <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
@ItsDoot
Documenting BufferVec. (bevyengine#4673)
6f2e3e6 
# Objective

Documents the `BufferVec` render resource.

`BufferVec` is a fairly low level object, that will likely be managed by a higher level API (e.g. through [`encase`](bevyengine#4272)) in the future. For now, since it is still used by some simple 
example crates (e.g. [bevy-vertex-pulling](https://github.com/superdump/bevy-vertex-pulling)), it will be helpful
to provide some simple documentation on what `BufferVec` does.  

## Solution

I looked through Discord discussion on `BufferVec`, and found [a comment](https://discord.com/channels/691052431525675048/953222550568173580/956596218857918464 ) by @superdump to be particularly helpful, in the general discussion around `encase`. 

I have taken care to clarify where the data is stored (host-side), when the device-side buffer is created (through calls to `reserve`), and when data writes from host to device are scheduled (using `write_buffer` calls). 

---

## Changelog

- Added doc string for `BufferVec` and two of its methods: `reserve` and `write_buffer`. 


Co-authored-by: Brian Merchant <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
@teoxoy @cart @ItsDoot
Migrate to encase from crevice (bevyengine#4339)
2f789c8 
# Objective

- Unify buffer APIs
- Also see bevyengine#4272

## Solution

- Replace vendored `crevice` with `encase`

---

## Changelog

Changed `StorageBuffer`
Added `DynamicStorageBuffer`
Replaced `UniformVec` with `UniformBuffer`
Replaced `DynamicUniformVec` with `DynamicUniformBuffer`

## Migration Guide

### `StorageBuffer`

removed `set_body()`, `values()`, `values_mut()`, `clear()`, `push()`, `append()`
added `set()`, `get()`, `get_mut()`

### `UniformVec` -> `UniformBuffer`

renamed `uniform_buffer()` to `buffer()`
removed `len()`, `is_empty()`, `capacity()`, `push()`, `reserve()`, `clear()`, `values()`
added `set()`, `get()`

### `DynamicUniformVec` -> `DynamicUniformBuffer`

renamed `uniform_buffer()` to `buffer()`
removed `capacity()`, `reserve()`


Co-authored-by: Carter Anderson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
Rendering
Status: Done
Milestone
No milestone
Development

No branches or pull requests
3 participants
@superdump @mockersf @teoxoy