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

Full MSAA handling #235

Merged
merged 1 commit into from
Jun 23, 2019
Merged

Full MSAA handling #235

merged 1 commit into from
Jun 23, 2019

Conversation

rukai
Copy link
Contributor

@rukai rukai commented Jun 22, 2019

These changes fix the msaa-line wgpu example, along with a PR to wgpu-native gfx-rs/wgpu-rs#28

Concerns:

  • webgpu does not expose a way for users to query limits, how are they supposed to choose a suitable sample_count?
  • I think attachment_unused should be moved into gfx-hal. Where abouts?
  • Is a sample mask of :u64 = !0 suitable?

Cargo.lock Outdated Show resolved Hide resolved
wgpu-native/src/command/mod.rs Outdated Show resolved Hide resolved
wgpu-native/src/command/mod.rs Outdated Show resolved Hide resolved
wgpu-native/src/command/mod.rs Outdated Show resolved Hide resolved
wgpu-native/src/command/mod.rs Show resolved Hide resolved
wgpu-native/src/device.rs Outdated Show resolved Hide resolved
@kvark
Copy link
Member

kvark commented Jun 23, 2019

@msiglreith it does sound like ATTACHMENT_UNUSED (or some sort of an Option) is needed in gfx-hal, for this case of resolve attachments specifically.

let color_attachments =
unsafe { slice::from_raw_parts(desc.color_attachments, desc.color_attachments_length) };
let depth_stencil_attachment = unsafe { desc.depth_stencil_attachment.as_ref() };

let sample_count = color_attachments.get(0).map(|at| view_guard[at.attachment].samples).unwrap_or(1);
assert!(sample_count & samples_count_limit != 0, "Attachment sample_count must be supported by physical device limits");
Copy link
Member

Choose a reason for hiding this comment

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

why are we using & here?

Copy link
Contributor Author

@rukai rukai Jun 23, 2019

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

oh wow, this is confusing. Filed gfx-rs/gfx#2859

wgpu-native/src/command/mod.rs Outdated Show resolved Hide resolved
wgpu-native/src/command/mod.rs Outdated Show resolved Hide resolved
wgpu-native/src/command/mod.rs Outdated Show resolved Hide resolved
wgpu-native/src/command/mod.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!
bors r+

let color_attachments =
unsafe { slice::from_raw_parts(desc.color_attachments, desc.color_attachments_length) };
let depth_stencil_attachment = unsafe { desc.depth_stencil_attachment.as_ref() };

let sample_count = color_attachments.get(0).map(|at| view_guard[at.attachment].samples).unwrap_or(1);
assert!(sample_count & samples_count_limit != 0, "Attachment sample_count must be supported by physical device limits");
Copy link
Member

Choose a reason for hiding this comment

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

oh wow, this is confusing. Filed gfx-rs/gfx#2859

bors bot added a commit that referenced this pull request Jun 23, 2019
235: Full MSAA handling r=kvark a=rukai

These changes fix the msaa-line wgpu example, along with a PR to wgpu-native gfx-rs/wgpu-rs#28

Concerns:
*   webgpu does not expose a way for users to query limits, how are they supposed to choose a suitable sample_count?
*   I think `attachment_unused` should be moved into gfx-hal. Where abouts?
*   Is a sample mask of `:u64 = !0` suitable?

Co-authored-by: Rukai <rubickent@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jun 23, 2019

Build succeeded

@bors bors bot merged commit 194943c into gfx-rs:master Jun 23, 2019
@msiglreith
Copy link
Contributor

@msiglreith it does sound like ATTACHMENT_UNUSED (or some sort of an Option) is needed in gfx-hal, for this case of resolve attachments specifically.

yes, a constant might be the easiest way to get started?

bors bot added a commit to gfx-rs/wgpu-rs that referenced this pull request Jun 24, 2019
28: msaa-line example fixes r=kvark a=rukai

This PR fixes the msaa-line example in addition to the fixes in gfx-rs/wgpu#235

If gfx-rs/gfx#2853 is merged first we can remove the crates.io patch.

Co-authored-by: Rukai <rubickent@gmail.com>
kvark pushed a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
28: msaa-line example fixes r=kvark a=rukai

This PR fixes the msaa-line example in addition to the fixes in gfx-rs#235

If gfx-rs/gfx#2853 is merged first we can remove the crates.io patch.

Co-authored-by: Rukai <rubickent@gmail.com>
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