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

Use a StagingBelt for regular buffer uploads #92

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hecrj
Copy link
Contributor

@hecrj hecrj commented Mar 30, 2024

Queue::write_buffer allocates very regularly, specially on Metal.

See: iced-rs/iced#2357

A StagingBelt gives us more control and predictability.

font_system: &mut FontSystem,
atlas: &mut TextAtlas,
screen_resolution: Resolution,
text_areas: impl IntoIterator<Item = TextArea<'a>>,
cache: &mut SwashCache,
mut metadata_to_depth: impl FnMut(usize) -> f32,
) -> Result<(), PrepareError> {
self.staging_belt.recall();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, for lowest memory usage recall should be called right after the user submits the Queue. But this approach removes the burden of a StagingBelt from the user and keeps it hidden.

Worst case scenario, we allocate some additional chunks. Overhead should be constant.

Comment on lines +376 to +302
self.staging_belt.finish();
self.staging_belt = StagingBelt::new(buffer_size);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can see, we only need to worry about resizing the belt when the vertex buffer grows, since index buffer uploads will never be bigger than the vertex ones.

6 u32 indices (6 * 4 bytes) are generated per each 4 GlyphToRender vertices (4 * 18 bytes).

`Queue::write_buffer` allocates very regularly, specially on Metal.

See: iced-rs/iced#2357

A `StagingBelt` gives us more control and predictability.
@grovesNL
Copy link
Owner

grovesNL commented May 27, 2024

I just rebased this onto main but would like to compare against write_buffer_with to check how much of the improvement you're noticing is the staging belt vs. the allocation. I'll make a separate PR with write_buffer_with.

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

Successfully merging this pull request may close these issues.

2 participants