Skip to content
This repository was archived by the owner on Jun 18, 2021. It is now read-only.

Update wgpu without peek-poke#396

Merged
bors[bot] merged 2 commits intogfx-rs:masterfrom
kvark:peek-poke
Jun 20, 2020
Merged

Update wgpu without peek-poke#396
bors[bot] merged 2 commits intogfx-rs:masterfrom
kvark:peek-poke

Conversation

@kvark
Copy link
Copy Markdown
Member

@kvark kvark commented Jun 20, 2020

Depends on gfx-rs/wgpu#739
Everything works, however I think it's worth changing the render pass color/depth/stencil descriptors to be more rusty, e.g.

struct ColorAttachmentDescriptor {
  attachment: &'a TextureView,
  resolve_target: Option<&'a TextureView>,
  clear_color: Option<Color>,
  store_result: bool,
}

enum DepthStencilChannel<T> {
  ReadOnly,
  Mutable { clear_value: Option<T>, store_result: bool },
}
struct DepthStencilAttachmentDescriptor {
}

This would also involve wgpu-type/wgpu-core changes, but can be done as a follow-up (don't want to block on them).

There is a value in doing it in this PR, so that end users don't get multiple disruptions, but we need to find a proper shape first.

bors bot added a commit to gfx-rs/wgpu that referenced this pull request Jun 20, 2020
739: Remove Peek-Poke r=cwfitzgerald a=kvark

**Connections**
Related to #738
Related to djg/peek-poke#10

**Description**
As of #726 , the buffers have a minimum binding size that has to include the shader struct size. It, therefore, can't be zero.
We can remove the hacks we had previously and switch fully to the idiomatic `Option<NonZeroU64>`.

Peek-poke doesn't `NonZeroU64` and friends, so this made me reconsider the user of it entirely. Today, render bundles as well as the Player already represent command streams using a much rustier method. I tried to move everything to this method now, and I think this is going to work much better, and safer.

**Testing**
wgpu-rs works - gfx-rs/wgpu-rs#396

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Copy link
Copy Markdown
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.

Very nice! Looks good as is, will review again when any additional changes are made.

@kvark
Copy link
Copy Markdown
Member Author

kvark commented Jun 20, 2020

I'm thinking of going with this API:

/// Operation to perform to the output attachment at the start of a renderpass.
pub enum LoadOp<V> {
    /// Clear with a specified value.
    Clear(V),
    /// Load from memory.
    Load,
}

/// Pair of load and store operations for an attachment aspect.
pub type Operations<V> = (LoadOp<V>, StoreOp);

/// Describes a color attachment to a [`RenderPass`].
pub struct RenderPassColorAttachmentDescriptor<'a> {
    pub attachment: &'a TextureView,
    pub resolve_target: Option<&'a TextureView>,
    pub ops: Operations<Color>,
}
/// Describes a depth/stencil attachment to a [`RenderPass`].
pub struct RenderPassDepthStencilAttachmentDescriptor<'a> {
    pub attachment: &'a TextureView,
    pub depth_ops: Option<Operations<f32>>,
    pub stencil_ops: Option<Operations<u32>>,
}

@cwfitzgerald
Copy link
Copy Markdown
Member

I like it!

@kvark kvark marked this pull request as ready for review June 20, 2020 22:57
Copy link
Copy Markdown
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.

LGTM!

@kvark
Copy link
Copy Markdown
Member Author

kvark commented Jun 20, 2020

thanks for reviewing quickly!
bors r=cwfitzgerald

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Jun 20, 2020

@bors bors bot merged commit 68461b1 into gfx-rs:master Jun 20, 2020
@kvark kvark deleted the peek-poke branch June 21, 2020 03:12
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.

2 participants