Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Texture support #204

Closed
khyperia opened this issue Nov 3, 2020 · 12 comments
Closed

Texture support #204

khyperia opened this issue Nov 3, 2020 · 12 comments
Assignees
Labels
c: rustc_codegen_spirv Issues specific to the rustc_codegen_spirv crate. t: enhancement A new feature or improvement to an existing one.

Comments

@khyperia
Copy link
Contributor

khyperia commented Nov 3, 2020

Implement texture support in the compiler. Exposing all of this is a lot, and needs a decent amount of UX thought, so posting this issue so people can chime in with ideas in addition to tracking the implementation.

Here's an overview of how textures work in the SPIR-V spec:

  • OpTypeImage: an opaque data blob that contains some form of image. The type contains:
    • the underlying sampled type
    • dimensions
    • if it's a depth image (as in depth testing), can be yes/no/unknown
    • "arrayed"
    • whether it's multisampled
    • whether it can be used without a sampler (I'm guessing capabilities are required here?), values are "unknown, with sampler, without sampler"
    • image format, can be unknown
    • if it's read, write, or readwrite.
  • OpTypeSampler/OpConstantSampler: does not contain image data, merely information on how to access an image. Data is in the OpConstantSampler, not the type (the type has no parameters at all).
    • None/ClampToEdge/Clamp/Repeat/RepeatMirrored
    • if it's normalized
    • sampler filter mode: Nearest/Linear
  • OpTypeSampledImage/OpSampledImage: combines an image and OpConstantSampler into one meta-object.
    • The result of OpSampledImage has absolutely wild restrictions: it can only be referenced in the same BB that it's declared in, can't be used in OpSelect/OpPhi, if the image is in a composite then all indices getting the image must be dynamically-uniform, etc.

Then, lots of image read/write instructions (not sure what Dref is used for, "depth-comparison reference value"):

and edit, to be clear, nearly all of these (except for ImageRead/ImageWrite) take the combined TypeSampledImage. There is no avoiding using the combined version. The combined type has insane restrictions that pretty much guarantees that it will not be exposed to users as a struct (e.g. must be used in the same basic block it is declared in), but we cannot get around using it, it is how texture reads work in SPIR-V.

  • OpImageFetch - directly on OpTypeImage with sampled=1, not OpTypeSampledImage, integer indices
  • OpImageRead - directly on OpTypeImage with sampled=0 or 2, integer indices
  • OpImageWrite - directly on OpTypeImage with sampled=0 or 2, integer indices
  • OpImageGather - fetches one color from four pixels I think? weirdly, takes OpTypeSampledImage, despite name. float indices
  • OpImageDrefGather - fetches depth from four pixels I think? weirdly, takes OpTypeSampledImage, despite name. float indices
  • OpImageSampleDrefExplicitLod - idk, "Sample an image doing depth-comparison using an explicit level of detail."
  • OpImageSampleDrefImplicitLod - idk, like explicit, but implicit derivative for LOD
  • OpImageSampleFootprintNV - spec literally just says "TBD"
  • OpImageSampleExplicitLod - LOD sample, float indices, except in kernel mode which allows integer
  • OpImageSampleImplicitLod - like explicit, but implicit derivative for LOD
  • OpImageSampleProjDrefExplicitLod - idk, "Sample an image with a project coordinate, doing depth-comparison, using an explicit level of detail."
  • OpImageSampleProjDrefImplicitLod - idk, like explicit, but implicit derivative for LOD
  • OpImageSampleProjExplicitLod - Sample image but divide coordinate by w
  • OpImageSampleProjImplicitLod - like explicit, but implicit derivative for LOD
  • OpImageSparseDrefGather - assuming all these sparse ones are equivalent to their non-sparse names. Needs SparseResidency capability.
  • OpImageSparseFetch
  • OpImageSparseGather
  • OpImageSparseRead
  • OpImageSparseSampleDrefExplicitLod
  • OpImageSparseSampleDrefImplicitLod
  • OpImageSparseSampleExplicitLod
  • OpImageSparseSampleImplicitLod
  • OpImageSparseSampleProjDrefExplicitLod
  • OpImageSparseSampleProjDrefImplicitLod
  • OpImageSparseSampleProjExplicitLod
  • OpImageSparseSampleProjImplicitLod

And some misc instructions:

  • OpImageTexelPointer - like OpAccessChain, but for pixels. "Use of such a pointer is limited to atomic operations."
  • OpImageQueryFormat - kernel capability (??)
  • OpImageQueryLevels - kernel, ImageQuery capability
  • OpImageQueryLod - ImageQuery capability (??). (GLSL's textureQueryLod ?)
  • OpImageQueryOrder - kernel capability (??)
  • OpImageQuerySamples - kernel, ImageQuery capability
  • OpImageQuerySize - kernel, ImageQuery capability. (GLSL's textureSize)
  • OpImageQuerySizeLod - kernel, ImageQuery capability. (GLSL's textureSize variants)
  • OpImageSparseTexelsResident - "Result is false if any of the texels were in uncommitted texture memory, and true otherwise."

but like, the ImageQuery capability implicitly declares the Shader capability, so the queries that require both the ImageQuery and the Kernel capability are, uuuh, a program needs to be both a Shader and a Kernel? what.

@khyperia khyperia added t: enhancement A new feature or improvement to an existing one. c: rustc_codegen_spirv Issues specific to the rustc_codegen_spirv crate. labels Nov 3, 2020
@khyperia khyperia self-assigned this Nov 3, 2020
@khyperia
Copy link
Contributor Author

khyperia commented Nov 3, 2020

First pass (not the final complete one) on what the API should look like:

For OpTypeImage declarations

  • Sampled Type is a generic type parameter (possibly constrained with trait that only allows allowed types)
  • Dimensionality is in the type name (1D, 2D, 3D, Cube, Rect, Buffer, SubpassData). Only 2d for now.
  • "whether or not this image is a depth image" is always set to false (or "no indication"? idk)
  • arrayed is always set to false (I think this is for platforms that don't support 3d textures? not sure)
  • multisampled is always set to false
  • whether or not it's used with a sampler is another type name (I believe glsl is sampler2d vs image2d, not sure what names we want to go with). Only have sampled images for now.

OpConstantSampler: not sure if we should go with separate methods on texture objects, passing in a sampler constant value, or what. What's weird is that OpConstantSampler requires a LiteralSampler capability, however, there doesn't seem to be any other way in the spec to create a OpTypeSampler object. More research is needed on the second part to design this bit.

struct Texture2d<T> { opaque }
impl<T> Texture2d<T> {
    fn fetch(&self, coord: Vec2<u32>) -> T { OpImageFetch }
    fn sample(&self, coord: Vec2<f32>) -> T { OpImageSampleImplicitLod }
    fn sample_lod(&self, coord: Vec2<f32>, lod: f32) -> T { OpImageSampleExplicitLod }
    fn sample_grad(&self, coord: Vec2<f32>, dx: f32, dy: f32) -> T { OpImageSampleExplicitLod }
    // no gather, Dref, projection, or sparse for now.
}

@Jasper-Bekkers
Copy link
Contributor

Jasper-Bekkers commented Nov 3, 2020

Usually samplers get bound just like textures and buffers to a slot. OpConstantSampler is seperate where it's declared inline in the shader and that is indeed a separate capability (also referred to as static samplers in other apis).

So you need one to do a texture sample.

struct Texture2d<T> { opaque }
struct Sampler { opaque }
impl<T> Texture2d<T> {
    fn fetch(&self, coord: Vec2<u32>) -> T { OpImageFetch }
    fn sample(&self, sampler: &Sampler, coord: Vec2<f32>) -> T { OpImageSampleImplicitLod }
    fn sample_lod(&self, sampler: &Sampler, coord: Vec2<f32>, lod: f32) -> T { OpImageSampleExplicitLod }
    fn sample_grad(&self, sampler: &Sampler, coord: Vec2<f32>, dx: f32, dy: f32) -> T { OpImageSampleExplicitLod }
    // no gather, Dref, projection, or sparse for now.
}
/// Sampler could be passed by value if it's Copy/Clone, the idea is at least that you should be able to use a sampler with multiple textures

If we do sparse textures I think a SparseTexture2d type that's seperate would make a lot of sense. Same for combined texture/samplers (OpTypeSampledImage), they're needed for some mobile platforms but they're very much not the modern way of doing things anymore so it can be delayed if needed.

Gather4 ops can wait I think, same for the depth-testing samplers because they're just used for shadow mapping (and could be straight forward to add later).

@khyperia
Copy link
Contributor Author

khyperia commented Nov 3, 2020

Usually samplers get bound just like textures and buffers to a slot

Can you point out where that is in the spec? I'm not seeing anything. Like, what address space do the OpVariables use, for example?

[OpTypeSampledImage:] they're needed for some mobile platforms but they're very much not the modern way of doing things anymore so it can be delayed if needed

What? They're used in every single one of these sample instructions except the ones that directly go into the texture without a sampler, you can't avoid using them.

Same for [exposing] combined texture/samplers (OpTypeSampledImage)

I... what? not following you here, why would we want to expose this instead of it being an implementation detail of these sample methods? (especially considering the absolutely wild spir-v validation rules around it)

@fu5ha
Copy link
Member

fu5ha commented Nov 3, 2020

I have no idea how it actually gets translated to spir-v, but most of the time when writing Vulkan and Vulkan GLSL, as @Jasper-Bekkers said, you're using separate images and samplers rather than a combined ImageSampler object.

Specifically, this means that CPU-side you'd be creating separate descriptors with type VK_DESCRIPTOR_TYPE_SAMPLER, and VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE rather than a single VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, and then in GLSL it would look like, for example (snipped from a PBR shader I wrote here:

layout(set = 0, binding = 0) uniform sampler tex_sampler;

layout(set = 0, binding = 1) uniform textureCube spec_cube_map;
layout(set = 0, binding = 2) uniform textureCube irradiance_cube_map;
layout(set = 0, binding = 3) uniform texture2D spec_brdf_map;
layout(set = 2, binding = 0) uniform texture2D albedo_map;
layout(set = 2, binding = 1) uniform texture2D normal_map;
layout(set = 2, binding = 2) uniform texture2D metallic_roughness_map;
layout(set = 2, binding = 3) uniform texture2D ao_map;
layout(set = 2, binding = 4) uniform texture2D emissive_map;

// ... this then gets used like so, re-using the sampler to sample each different image even of different types. 
// I think `sampler2D()` constructor is combining the sampler and texture into SPIR-V's version of
// combined image sampler in the shader maybe?

    vec3 albedo = texture(sampler2D(albedo_map, tex_sampler), f_uv).rgb;
    vec3 normal = texture(sampler2D(normal_map, tex_sampler), f_uv).rgb;
    vec2 metallic_roughness = texture(sampler2D(metallic_roughness_map, tex_sampler), f_uv).bg;
    float metallic = metallic_roughness.x;
    float roughness = metallic_roughness.y;
    float ao = texture(sampler2D(ao_map, tex_sampler), f_uv).r;
    vec3 emissive = texture(sampler2D(emissive_map, tex_sampler), f_uv).rgb;

// ... later, here notice `samplerCube()` but with the same sampler

    vec3 ambient_irradiance = texture(samplerCube(irradiance_cube_map, tex_sampler), N).rgb;
    vec3 ambient_spec = textureLod(samplerCube(spec_cube_map, tex_sampler), R, roughness * MAX_SPEC_LOD).rgb;
    vec2 env_brdf = texture(sampler2D(spec_brdf_map, tex_sampler), vec2(NdotV, roughness)).rg;

@fu5ha
Copy link
Member

fu5ha commented Nov 3, 2020

Oh, also a thing of note is that I just went and checked and Ark is actually currently using only the combined image samplers everywhere, rather than the separate version I talked about above x) it wouldn't be very hard to change that though

@khyperia khyperia mentioned this issue Nov 12, 2020
@fu5ha fu5ha mentioned this issue Nov 12, 2020
4 tasks
@khyperia khyperia mentioned this issue Nov 26, 2020
@khyperia
Copy link
Contributor Author

khyperia commented Nov 26, 2020

Design question I thought of when doing #276: do we want to support textures whose sampled_type is not f32? Spec:

Sampled Type is the type of the components that result from sampling or reading from this image type. Must be a scalar numerical type (i.e. int or float) or OpTypeVoid.

So to be clear, it's not like we can have a Texture2d<Vec2> and Texture2d<Vec4> (which was my original thought on how this would work), all texture types only have their scalar types associated with them (which is sampled_type), then all sampling instructions always return a 4-component vector of that scalar type ("Result Type must be a vector of four components of floating-point type or integer type").

So, if we were to support sampled_type that is f64, or any integer type, we would need vector types for f64 and integers. Glam currently doesn't support those. So, do we want to put in that work eventually? Probably not now, probably not soon, but should it be on our roadmap to do eventually?

@XAMPPRocky
Copy link
Member

So, if we were to support sampled_type that is f64, or any integer type, we would need vector types for f64 and integers. Glam currently doesn't support those. So, do we want to put in that work eventually? Probably not now, probably not soon, but should it be on our roadmap to do eventually?

There's a already an issue on glam for it, so we could just track that for now. bitshifter/glam-rs#70

@msiglreith
Copy link
Contributor

I would like to add combined image sampler (TypeSampledImage), would this conflict with someones work?
As mentioned in the top post, dynamic creation comes with a few constraints but passing it as resource should be fine.

Proposed API:

#[spirv(sampled_image)]
pub struct SampledImage<I> { .. }

impl SampledImage<Image2d> {
    pub fn sample(&self, coord: Vec3A) -> Vec4 { .. }
}

Usage:

#[spirv(binding = 0)] image: UniformConstant<SampledImage<Image2d>>,
...
let image = image.load();
let sample = image.sample(coord);

@khyperia
Copy link
Contributor Author

khyperia commented Dec 4, 2020

I believe nobody else is planning on implementing TypeSampledImage. I'd be okay with either the API of struct SampledImage<T> where T is an image type, or struct Sampler2d, whatever's easier (the latter is how glsl exposes it, the former feels a lil more rusty). Feel free to do it, thanks so much! Feel free to ping me on the rust-gpu discord if you'd like implementation help.

@expenses
Copy link
Contributor

expenses commented Jan 1, 2021

Would it be readonable to have an enum in spirv-std where one variant contains an 'OpTypeImage' and a 'OpTypeSampler' and the other variant contains a 'OpTypeSampledImage'? That way it could be used for sampling without anyone having to worry about how it was constructed.

@expenses
Copy link
Contributor

expenses commented Jan 3, 2021

arrayed is always set to false (I think this is for platforms that don't support 3d textures? not sure)

This is for texture2DArray is glsl, which are similar to 3d textures but faster because you're not interpolating between the texture levels.

multisampled is always set to false

Except in the case of texture2DMS and texture2DMSArrays in glsl which I could barely find any information about. I believe that they're used when you want MSAA in a deferred renderer, for example in richter here: https://github.com/cormac-obrien/richter/blob/5251b4cc24d159d12c518d6dbdd9f80c1f4c0948/shaders/deferred.frag#L42. If you replace 'texture2DMS' with 'texture2D' then the antialiasing gets removed.

@khyperia
Copy link
Contributor Author

Let's close this and track individual features as the need for them comes up - needs seem to have stabilized, and a solid foundation with quite a bit of support is already in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: rustc_codegen_spirv Issues specific to the rustc_codegen_spirv crate. t: enhancement A new feature or improvement to an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants