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

Add a vulkan workaround for large buffers. #2796

Merged
merged 2 commits into from
Jun 22, 2022

Conversation

nical
Copy link
Contributor

@nical nical commented Jun 21, 2022

Description

Some drivers run into fairly risky integer overflows with buffer sizes and buffer range parameters that don't fit in a 32 bits integer. Mesa/lavapipe is one of them.

This PR adds a driver workaround flag which, when set, limits buffer allocation sizes to what fits an i32.

For some context about mesa, it basically boils down to internal commands passing buffer ranges and sizes via pipe_box, see https://github.com/mesa3d/mesa/blob/81d6ae31d6f18d6fd2894a8b6dfe4323eea797f9/src/gallium/include/pipe/p_state.h#L529

I am not sure how to be more precise about enabling the workaround on linux. I suspect that it does not affect the nvidia proprietary driver, Iet me know if I can improve this.

Also I have no doubt that plenty of other drivers make the same mistake. If you want to be on the safe side, I don't think it would be unreasonable to set this workaround on all android devices (these tend to have more limited available memory anyway).

Testing

None.

@nical
Copy link
Contributor Author

nical commented Jun 21, 2022

Added an entry in the wiki.

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved
wgpu-hal/src/vulkan/device.rs Outdated Show resolved Hide resolved
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

One nit, otherwise LGTM

wgpu-hal/src/dx11/adapter.rs Outdated Show resolved Hide resolved
Some drivers run into issues when buffer sizes and ranges are larger than what fits signed 32 bit integer. Adapt the maximum buffer size accordingly.
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Thanks!

@cwfitzgerald cwfitzgerald merged commit 5dcd19c into gfx-rs:master Jun 22, 2022
@nical nical deleted the i32-buffer-limit branch June 22, 2022 16:57
atadier pushed a commit to atadier/wgpu that referenced this pull request Jun 23, 2022
* Add Limit::max_buffer_size.

* Prevent very large buffer with some drivers.

Some drivers run into issues when buffer sizes and ranges are larger than what fits signed 32 bit integer. Adapt the maximum buffer size accordingly.
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