Add API to enable ext/capabilities, and remove default capabilities#630
Add API to enable ext/capabilities, and remove default capabilities#630
Conversation
| .iter() | ||
| .any(|inst| { | ||
| inst.class.opcode == Op::Extension | ||
| && inst.operands[0].unwrap_literal_string() == extension | ||
| }) |
There was a problem hiding this comment.
This can be O(1) as a set of interned strings, maybe open an issue about simple algorithmic inefficiencies like this and leave // FIXME(#1234) comments? At least that's what I would should do.
| // target_features is a HashSet, not a Vec, so we need to sort to have deterministic | ||
| // compilation - otherwise, the order of capabilities in binaries depends on the iteration | ||
| // order of the hashset. Sort by the string, since that's easy. | ||
| feature_names.sort(); |
There was a problem hiding this comment.
It's a FxHashSet, right? Its order should already be deterministic. But yes this is probably a good idea just to keep the output clean, if nothing else.
There was a problem hiding this comment.
It should be deterministic! In practice, somehow, it isn't - I hit a different order when upgrading rust-toolchain, not sure if the actual cause of the different order was the different rustc binary, or if the different order was more common/subtle than that. (Random guess, do symbols like, use their pointer as a hash key, instead of the string contents? So if the symbols are allocated in a different order, the FxHashSet<Symbol> order is different?)
| build_shader( | ||
| "../../shaders/mouse-shader", | ||
| false, | ||
| &[Capability::Int8, Capability::Int16, Capability::Int64], |
There was a problem hiding this comment.
Maybe open an issue for this? I think it's just one of those num-traits things like .abs()?
| RustGPUShader::Compute => ("compute-shader", &[]), | ||
| RustGPUShader::Mouse => ( | ||
| "mouse-shader", | ||
| &[Capability::Int8, Capability::Int16, Capability::Int64], |
There was a problem hiding this comment.
Oof, the duplication here isn't great. There's no easy way to share this information, is there?
There was a problem hiding this comment.
My only thought was to make a common file and include!() it from both (or #[path = "..."] mod blah; or whatever), but I'm not sure if that makes it better or worse.
|
I'm going to leave #633 as just an issue instead of a comment in code |
I had to add a decent amount of debugging infrastructure to figure out wtf was going wrong when the default capabilities weren't enabled, and I figured I should clean up that infra and leave it in, so there's a decent amount of error message changes in this PR.
Also included is adding
glamto system crates, because otherwise it barfs on all usages of e.g.f64in glam ifFloat64isn't enabled, and I don't want to force glam to write#[cfg(any(not(target_arch = "spirv"), target_feature = "Float64"))]everywhere (and Int8, etc.) - it'd be a maintenance nightmare, easily broken.Also, compiletest gets all of Int8, Int16, Int64, Float64 automatically enabled for both the "library" crates (everything except for the compiletest'ed crate), as well as the final crate. I ranted about this in discord for as to why, I'm just going to copy that in instead of rewriting it: