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

Mesh shaders: implement for Vulkan backend #3253

Merged
merged 1 commit into from
May 10, 2020
Merged

Conversation

MatusT
Copy link
Contributor

@MatusT MatusT commented May 9, 2020

This also includes an example.

To discuss\resolve:

  • mesh_fn added to RawDevice. Is it ok? Will It stay ok being extended with many more extensions?
  • Limits: Unfortunately, Khronos made a slight in mistake in 1.0 by not assigning pNext member to some structs that should have had one. This includes VkPhysicalProperties. Without Vulkan 1.1(or KHR extension) where this was remedied, we are unable to query for limits of the mesh shader extension that are inside VkPhysicalDeviceMeshShaderPropertiesNV struct.

PR checklist:

  • make succeeds (on *nix)
  • make reftests succeeds
  • tested examples with the following backends:
  • rustfmt run on changed code

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thank you!
I didn't read the example code too much, I assume it mostly follows the other examples.
Expressed my questions/concerns, there aren't many :)

examples/mesh-shading/data/index.html Outdated Show resolved Hide resolved
src/backend/vulkan/src/command.rs Outdated Show resolved Hide resolved
src/backend/vulkan/src/lib.rs Show resolved Hide resolved
src/backend/vulkan/src/lib.rs Outdated Show resolved Hide resolved
src/backend/vulkan/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thank you! Looks like we are almost there!

src/backend/vulkan/src/command.rs Outdated Show resolved Hide resolved
src/backend/vulkan/src/command.rs Outdated Show resolved Hide resolved
src/backend/vulkan/src/command.rs Outdated Show resolved Hide resolved
src/backend/vulkan/src/lib.rs Outdated Show resolved Hide resolved
@@ -1081,6 +1098,9 @@ impl adapter::PhysicalDevice<Backend> for PhysicalDevice {
if features.inherited_queries != 0 {
bits |= Features::INHERITED_QUERIES;
}
if self.supports_extension(*MESH_SHADER) {
bits |= Features::MESH_SHADER;
Copy link
Member

Choose a reason for hiding this comment

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

what about TASK_SHADER bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Can't be separated without VkPhysicalFeatures2. Most probably inconsequential as DirectX 12 requires both shaders to be supported despite Task being optional in the pipeline as well as in the Vulkan extension itself.

src/backend/vulkan/src/lib.rs Outdated Show resolved Hide resolved
src/backend/vulkan/src/lib.rs Show resolved Hide resolved
some fixes

fix some review notes
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Superb!
bors r+

@bors
Copy link
Contributor

bors bot commented May 10, 2020

Build succeeded:

@bors bors bot merged commit 2ee015e into gfx-rs:master May 10, 2020
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