Textures! #276
Textures! #276
Conversation
| return None; | ||
| } | ||
| }; | ||
| if args.len() != 6 && args.len() != 7 { |
There was a problem hiding this comment.
I think we can set a lot of defaults values. Having to set depth to 0 for 90% of textures seems a bit annoying.
There was a problem hiding this comment.
I don't imagine this attribute being used out of spirv-std (hence me leaving it undocumented), so user convenience and defaulting and whatnot isn't necessary.
There was a problem hiding this comment.
Could we specify some of these with const generics instead of attributes?
|
@khyperia don't forget to add it to the guide :) (or poke the community team to write documentation for you 🙊) |
| if ty.size != Size::from_bytes(4) { | ||
| cx.tcx.sess.err("#[spirv(image)] type must have size 4"); | ||
| return None; | ||
| } |
There was a problem hiding this comment.
Just leaving this as a note, my preference is encoding it as &Image where
extern {
#[spirv(image)]
type Image;
}But I have no idea if this is workable, so we should leave it for future (if ever) experimentation.
With a hardcoded size, one fun way to stop rustc from letting you nest the layout in anything else (with any more data), is to make it a newtype of [u8; (1 << 47) - 1]. That's because you can't have a size of 247 (or larger), on a 64-bit target (and there's a similar limitation on 32-bit targets but it's closer to 232).
There was a problem hiding this comment.
The other way to think about this is we could represent them as small as 1-byte structs, but no less, so that the offset bijectively maps to the field, and generally to avoid any ZST special-casing.
The only reason to use pointer-size is the analogy with WASM, which has e.g. "function pointers" (and more generally externref), that are abstract values which can be referred to by an usize index into a global "table", if need be.
(Is that a plausible concern/direction with SPIR-V? Could we actually have a global array of e.g. Images?)
We can also avoid any Abi special-casing by using [u8; 1], IIRC, since that should create Abi::Aggregate.
(Either way we should assert the .abi is what we expect, in case it does change in the future)
eddyb
left a comment
eddyb
left a comment
There was a problem hiding this comment.
I guess I can't "approve" it since it was already merged, but LGTM!
|
Why was it merged..? It should have waited for your review. dumb bot. |
3 participants
Partially does #204 (just the compiler parts and the minimal library part to make sure the compiler bits work - library bits can be done in a follow-up)
I have a local change to wgpu runner based off https://github.com/gfx-rs/wgpu-rs/blob/e59ea495a66ce9606c3a2bbfc2efe8c0c05d413c/examples/cube/main.rs that makes sure textures work, but, considering it changes the shader significantly (adds a texture/sampler parameter and hardcode-replaces the sky shader), I'm not sure what I should do with it.