Skip to content
This repository was archived by the owner on Jan 29, 2025. It is now read-only.

[spv] emit proper member layout decorations#360

Merged
kvark merged 1 commit intogfx-rs:masterfrom
kvark:spv-offset
Jan 22, 2021
Merged

[spv] emit proper member layout decorations#360
kvark merged 1 commit intogfx-rs:masterfrom
kvark:spv-offset

Conversation

@kvark
Copy link
Copy Markdown
Member

@kvark kvark commented Jan 21, 2021

Fixes #350
Fixes #336

@kvark kvark requested a review from Timo-DK January 21, 2021 21:29
Copy link
Copy Markdown
Collaborator

@Timo-DK Timo-DK left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Maybe I am not seeing it, but what exactly is the difference in functionality between proc/sizer.rs and the new proc/layouter.rs?

Err(_) => {
let mut param = Parameters::default();
// very useful to have this by default
param.spv_capabilities.insert(spirv::Capability::Shader);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very useful indeed! Maybe something for later to add for all examples by default when a param.ron is not found?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, I was thinking about this as well, just having a default one in a file. Good for a follow-up!

@kvark
Copy link
Copy Markdown
Member Author

kvark commented Jan 22, 2021

but what exactly is the difference in functionality between proc/sizer.rs and the new proc/layouter.rs?

The difference is that the old sizer was only computing the sizes, and the new layouter computes both the sizes and the alignments, analogous to https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html

The alignment is necessary, because it affects the layout by introducing pads. For example, a f32 followed by vec4<f32> would need to have a gap, but the old routine wasn't computing this gap (and therefore also mistakenly computing the size of structures).

@kvark kvark merged commit da51f29 into gfx-rs:master Jan 22, 2021
@kvark kvark deleted the spv-offset branch January 22, 2021 13:54
@kvark kvark mentioned this pull request Jan 25, 2021
bors bot added a commit to gfx-rs/wgpu-rs that referenced this pull request Jan 30, 2021
679: WGSL shader conversion for the examples r=grovesNL a=kvark

Blocked on:
  - ~~gfx-rs/naga#335
  - ~~gfx-rs/naga#336
  - ~~gfx-rs/wgpu#1095
  - ~~gfx-rs/naga#360
  - gfx-rs/gfx#3622
  - ~~GL backend doing the program link check earlier~~

Converted:
- [x] boids
- [x] cube
- [x] mipmap
- [x] msaa-line
- [x] shadow
- [x] skybox
- [ ] texture-array
- [ ] water

Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
kvark added a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
679: WGSL shader conversion for the examples r=grovesNL a=kvark

Blocked on:
  - ~~gfx-rs/naga#335
  - ~~gfx-rs/naga#336
  - ~~gfx-rs#1095
  - ~~gfx-rs/naga#360
  - gfx-rs/gfx#3622
  - ~~GL backend doing the program link check earlier~~

Converted:
- [x] boids
- [x] cube
- [x] mipmap
- [x] msaa-line
- [x] shadow
- [x] skybox
- [ ] texture-array
- [ ] water

Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MatrixStride is not implemented Generated SPIR-V structures are missing the offset decorations

2 participants