Add support for sampled images #320
Conversation
Combined image samplers are allowed as resources and require a generic parameter indicating the underlying image type.
| // The spirv type of it will be generated by querying the type of the first generic. | ||
| if let TyKind::Adt(_, substs) = *ty.ty.kind() { | ||
| // TODO: enforce that the generic param is an image type? | ||
| let image_ty = substs.type_at(0); |
There was a problem hiding this comment.
Nit: I think this might ICE if it's not a generic type (as in doesn't have a type parameter 0), but considering #[spirv(sampled_image)] is only intended to be used in spirv-std, that's probably okay.
| @@ -440,6 +444,10 @@ impl fmt::Debug for SpirvTypePrinter<'_, '_> { | |||
| .field("access_qualifier", &access_qualifier) | |||
| .finish(), | |||
| SpirvType::Sampler => f.debug_struct("Sampler").field("id", &self.id).finish(), | |||
| SpirvType::SampledImage { image_type } => f | |||
| .debug_struct("SampledImage") | |||
| .field("image_type", &self.cx.debug_type(image_type)) | |||
There was a problem hiding this comment.
Nit: this should have .field("id", &self.id) too (I've found including the id of self helps a lot when debugging type shenanigans)
| } else { | ||
| cx.tcx | ||
| .sess | ||
| .err("#[spirv(sampled_image)] generic type must be an image"); |
There was a problem hiding this comment.
Nit: I think this might actually be a bug!()? I don't think it's possible to get a non-ADT here (see callsite of trans_image, where it's already matching on TyKind::Adt - I guess you could grab out the substs there and pass them into trans_image, if you'd like)
yeah, haha, there's so much handwaviness right now that's very similar... we probably want to check that somehow eventually and produce a nice error (right now it'll fail |
|
thanks for the quick and super helpful review! |
Implements #204 (comment)
Combined image samplers are allowed as resources and require a generic parameter indicating the underlying image type.
Therefore, no constructor ala
let sampled_image = image.combine(sampler);forImage2dright now.The way the internal spirv type of the underlying image is created feels a bit handwavy to be honest - ideally I would somehow directly check for all constraints (exactly an image type, possible some trait?).