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

Fighting back the bovine forces#501

Merged
bors[bot] merged 1 commit intogfx-rs:masterfrom
Kimundi:retreat_to_greener_pastures
Aug 12, 2020
Merged

Fighting back the bovine forces#501
bors[bot] merged 1 commit intogfx-rs:masterfrom
Kimundi:retreat_to_greener_pastures

Conversation

@Kimundi
Copy link
Copy Markdown
Contributor

@Kimundi Kimundi commented Aug 10, 2020

"Good work boys, we'll get 'em next time!"

Copy link
Copy Markdown

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

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

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 0 warnings, 2 errors.

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

Copy link
Copy Markdown

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

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

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 0 warnings, 2 errors.

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

@Kimundi Kimundi force-pushed the retreat_to_greener_pastures branch 2 times, most recently from 827588d to 785a0fe Compare August 11, 2020 06:30
@Kimundi Kimundi marked this pull request as ready for review August 11, 2020 13:30
@cwfitzgerald cwfitzgerald self-assigned this Aug 12, 2020
@cwfitzgerald cwfitzgerald self-requested a review August 12, 2020 02:18
@cwfitzgerald cwfitzgerald removed their assignment Aug 12, 2020
Copy link
Copy Markdown
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! We should be ready to land this. After all, it's only 250 LOC here, and we'll be able to shove a bunch off wgpu-types with this.
Noted a few small things. Also, please squash this!

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.

This is amazing! The api really cleaned up!

@Kimundi Kimundi force-pushed the retreat_to_greener_pastures branch from 9837adc to a336901 Compare August 12, 2020 08:20
- BufferDescriptor
- CommandEncoderDescriptor
- RenderBundleDescriptor
- TextureDescriptor
- TextureViewDescriptor
- PipelineLayoutDescriptor
- SamplerDescriptor
- BindGroupDescriptor
- ProgrammableStageDescriptor
- RenderPassDescriptor
- RenderPipelineDescriptor
- BindGroupLayoutDescriptor
- VertexStateDescriptor
- VertexBufferDescriptor
- RenderBundleEncoderDescriptor
- ComputePipelineDescriptor

Also change anisotropy_clamp to use NonZeroU8
@Kimundi Kimundi force-pushed the retreat_to_greener_pastures branch from a336901 to e9cf243 Compare August 12, 2020 08:24
Copy link
Copy Markdown
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.

bors r+

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Aug 12, 2020

@bors bors bot merged commit d3edb95 into gfx-rs:master Aug 12, 2020
bors bot added a commit to gfx-rs/wgpu that referenced this pull request Aug 13, 2020
873: Grand cleanup and refactor of the descriptors in the API r=cwfitzgerald,grovesNL a=kvark

**Connections**
Cleanup follows gfx-rs/wgpu-rs#501
Fixes #871 
Fixes #848

**Description**
There is a lot of small and big things crumbled in here.

The major one is where descriptors live, and how they are parametrized. Logic is the following:
  - if something is useful by wgpu-rs's API leave it in `wgpu-types`
  - if *in addition* it's useful to `wgpu-native`(i.e. has `repr(C)`),  we possibly parametrize it (currently, only label is).
  - otherwise, the type is moved to `wgpu-core` and stripped of generics
  - remove all the builders

Some medium-sized things:
  - moves the subscriber out (#871)
  - moves the `RenderCommand` into the `draw` module (which is meant to contain shared things)
  - makes `TextureViewDescriptor` fields optional, but receives it directly now (#848)
  - moves stencil stuff out into `StencilStateDescriptor`, so that we can derive `Default` for it
  - stop accepting raw strings for labels in the `Device` API: neither the clients, or gfx-rs need that shape, and it can't be safe

Some smaller things:
  - using NonZeroU8 for anisotropy
  - using NonZeroU32 for descriptor count
  - putting sampler addressing modes into an array
  - add labels to command buffers and pipeline layouts
  - improves errors for exceeding binding limits

**Testing**
Tested on gfx-rs/wgpu-rs#503


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

3 participants