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

[encoding] Do not allow zero-length buffer sizes #365

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

armansito
Copy link
Collaborator

It is possible for a BufferSize of 0 bytes to trigger validation warnings as such buffers still need to get bound to their pipeline. The standard vello engine (src/engine.rs) handles this when a BufProxy is requested by setting the requested size to at least 16.

This is now handled by the encoding crate by selecting a buffer size that can fit at least one element.

It is possible for a BufferSize of 0 bytes to trigger validation
warnings as such buffers still need to get bound to their pipeline. The
standard vello engine (src/engine.rs) handles this when a BufProxy is
requested by setting the requested size to at least 16.

This is now handled by the encoding crate by selecting a buffer size
that can fit at least one element.
Copy link
Collaborator

@dfrg dfrg left a comment

Choose a reason for hiding this comment

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

This has been a source of many problems so thanks for making it actually correct!

//
// Note: not using `Ord::max` here because it doesn't support const eval yet (except
// in nightly)
len: if len > 0 { len } else { 1 },
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe len.max(1)?

Copy link
Member

Choose a reason for hiding this comment

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

Is that not the comment directly above?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That’ll teach me to read comments. Please ignore :)

@armansito armansito merged commit 530cde9 into linebender:main Sep 21, 2023
4 checks passed
@armansito armansito deleted the pr-non-zero-buffer-sizes branch September 21, 2023 17:06
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.

3 participants