Sampler descriptor #195
Sampler descriptor #195
Conversation
The sampler builder will be removed as the first builder being converted to a descriptor. Use CommandBufferBuilder instead as it is not expected to change soon.
| namespace backend { | ||
|
|
||
| {% for type in by_category["enum"] %} | ||
| bool IsValid{{type.name.CamelCase()}}(nxt::{{as_cppType(type.name)}} value) { |
There was a problem hiding this comment.
Couldn't this be a "<" since our enums should be contiguous? Maybe with some extra asserts.
There was a problem hiding this comment.
I was hoping that the switch would be future proof for when we start to have sparse enums for extensions. Compilers do a good job of optimizing this pattern when it is dense, see https://godbolt.org/g/HjWVZH
WDYT?
examples/HelloTriangle.cpp
Outdated
| sampler = device.CreateSamplerBuilder() | ||
| .SetFilterMode(nxt::FilterMode::Linear, nxt::FilterMode::Linear, nxt::FilterMode::Linear) | ||
| .GetResult(); | ||
| { |
There was a problem hiding this comment.
Maybe not useful for this struct - but should we generate "fill with default" functions for struct initialization?
I think even constructors, like nxt::SamplerDescriptor makeSamplerDescriptor(), would work.
There was a problem hiding this comment.
I thought about adding default to the generator but was worried that if WebGPU chooses different defaults and we update to follow them, we would have tests and samples break. Adding helper functions to make the default descriptor in NXTHelpers sounds good though. I'll look into it.
There was a problem hiding this comment.
Ok. I would also be fine with leaving it out for now.
|
The only other changes could have been making end2end tests able to run on the wire, but it looks like it will be a different PR. What do you think of the overall direction for the structure of the backend/ code? |
|
Oh and I'll pushed for CI and wasn't looking for a review just yet (didn't do my own re-review). I'll look into having travis per-branch in my repo instead. |
No problem with the changes in this commit. They seem pretty minor.
np, I can always review subsequent diffs if you don't squash. |
| @@ -57,7 +57,7 @@ namespace backend { namespace d3d12 { | |||
| RenderPassDescriptor::GetSubpassOMSetRenderTargetArgs() { | |||
| OMSetRenderTargetArgs args = {}; | |||
|
|
|||
| size_t rtvIndex = 0; | |||
| unsigned int rtvIndex = 0; | |||
There was a problem hiding this comment.
This unrelated warning fix snuck in, but if that's ok I'll leave it in this commit.
There was a problem hiding this comment.
"Leave the campground cleaner than you found it" :)
|
PTAL @SenorBlanco, also @kainino0x PTAL at the latest commit. |
|
Thank you, landing. |
No description provided.