kvark
commented
3 years ago Looks great @Wumpf , thank you for adding this!
I share the concern about validate_creation_desc_support. The problem is - how do we consistently draw the line between what we validate and what not? A loooot of things can be validated. In some places the cost is nothing, in another the cost can be visible. Right now the balance is - validate the minimum, i.e. just the feature set on device creation, that's pretty much it! Everything else is just "oops, unexpected stuff came", it's not really validation. Adding the check for conservative and depth clamping, while leaving all the other 100 pipeline states, seems like a wrong balance to me.
So that "check somewhere" should be in wgpu, which is safe. Would you mind removing the validate_creation_desc_support check from the PR?
Wumpf
bors[bot]
Sorta fixes #3637
Only provides the very basic option of enabling/disabling overestimation rasterization. This is almost everything DX12 has to offer anyways except the optional
SV_InnerCoveragesupport, but leaves out even more Vulkan goodies (e.g. underestimation, various device properties, configurable extra-over-estimation, conservative raster of lines...). Availability of the coverage variable is the most important missing thing but could later be exposed as an additional feature flag (debatable, since this is not how Vulkan does it, so more of a device property?)Probably more controversial about this PR:
validate_creation_desc_supportI didn't want to repeat feature check code over all devices so I created a function on the desc that validates against features (and made unallowed use of DEPTH_CLAMP an error since this was close by..). Imho this is the only sane way forward for stuff like this since more often than not there needs to be a check somewhere but the philosophy of having this on this level and in that particular place might be questionable.Tested this with wgpu - I already have wgpu and wgpu-rs branches ready with a sample that can swap back and forth between enabled and disabled conservative rasterization. Going to look like this:
PR checklist:
makesucceeds (on *nix)make reftestssucceeds -- failed in alloc, seems unrelatedthread 'main' panicked at 'removal index (is 0) should be < len (is 0)', library/alloc/src/vec.rs:1066:13