Skip to content

Commit

Permalink
doc: document remaining imports and audit usage of 'ctx lifetime
Browse files Browse the repository at this point in the history
  • Loading branch information
chyyran committed Sep 3, 2024
1 parent 02dcb9a commit 47b9749
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 32 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion spirv-cross2/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "spirv-cross2"
version = "0.1.3"
version = "0.1.4"
edition = "2021"

license = "MIT OR Apache-2.0"
Expand Down
6 changes: 3 additions & 3 deletions spirv-cross2/src/compile/hlsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl From<HlslShaderModel> for u32 {
}

/// HLSL specific APIs.
impl<'a> Compiler<'a, Hlsl> {
impl Compiler<'_, Hlsl> {
/// Add a resource binding to the HLSL compilation.
///
/// By matching `stage`, `desc_set` and `binding` for a SPIR-V resource,
Expand All @@ -202,7 +202,7 @@ impl<'a> Compiler<'a, Hlsl> {
///
/// If resource bindings are provided, [`CompiledArtifact<Hlsl>::is_resource_used`] will return true if
/// the set/binding combination was used by the HLSL code.
pub fn add_resource_binding<'str>(&mut self, binding: &ResourceBinding) -> error::Result<()> {
pub fn add_resource_binding(&mut self, binding: &ResourceBinding) -> error::Result<()> {
unsafe {
sys::spvc_compiler_hlsl_add_resource_binding(self.ptr.as_ptr(), binding).ok(&*self)
}
Expand Down Expand Up @@ -337,7 +337,7 @@ impl<'a> Compiler<'a, Hlsl> {
}
}

impl<'a> CompiledArtifact<'a, Hlsl> {
impl CompiledArtifact<'_, Hlsl> {
/// Returns whether the set/binding combination provided in [`Compiler<Hlsl>::add_resource_binding`]
/// was used.
pub fn is_resource_used(&self, model: spirv::ExecutionModel, set: u32, binding: u32) -> bool {
Expand Down
6 changes: 3 additions & 3 deletions spirv-cross2/src/compile/msl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ pub struct BufferRequirements {
}

/// MSL specific APIs.
impl<'a> Compiler<'a, Msl> {
impl Compiler<'_, Msl> {
/// Get whether the vertex shader requires rasterization to be disabled.
pub fn is_rasterization_disabled(&self) -> bool {
unsafe { sys::spvc_compiler_msl_is_rasterization_disabled(self.ptr.as_ptr()) }
Expand Down Expand Up @@ -679,7 +679,7 @@ impl<'a> Compiler<'a, Msl> {
}

/// This function marks a resource as using a dynamic offset
/// (``VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC` or `VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC`).
/// (`VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC` or `VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC`).
///
/// `desc_set` and `binding` are the SPIR-V descriptor set and binding of a buffer resource
/// in this shader.
Expand Down Expand Up @@ -912,7 +912,7 @@ pub enum AutomaticResourceBindingTier {
Secondary,
}

impl<'a> CompiledArtifact<'a, Msl> {
impl CompiledArtifact<'_, Msl> {
/// Returns whether the set/binding combination provided in [`Compiler<Msl>::add_resource_binding`]
/// was used.
pub fn is_resource_used(&self, model: spirv::ExecutionModel, set: u32, binding: u32) -> bool {
Expand Down
6 changes: 4 additions & 2 deletions spirv-cross2/src/reflect/buffers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ use spirv_cross_sys::{SpvId, VariableId};
pub use spirv_cross_sys::BufferRange;

/// Reflection of buffers (UBO, SSBOs, and PushConstant blocks).
impl<'a, T> Compiler<'a, T> {
impl<'ctx, T> Compiler<'ctx, T> {
/// Returns a list of which members of a struct are potentially in use by a
/// SPIR-V shader. The granularity of this analysis is per-member of a struct.
/// This can be used for Buffer (UBO), BufferBlock/StorageBuffer (SSBO) and PushConstant blocks.
pub fn active_buffer_ranges(
&self,
handle: impl Into<Handle<VariableId>>,
) -> error::Result<&'a [BufferRange]> {
) -> error::Result<&'ctx [BufferRange]> {
let handle = handle.into();
let handle = self.yield_id(handle)?;

Expand All @@ -31,6 +31,8 @@ impl<'a, T> Compiler<'a, T> {
)
.ok(self)?;

// SAFETY: 'ctx is sound here
// https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.cpp#L2575
Ok(std::slice::from_raw_parts(ranges, size))
}
}
Expand Down
7 changes: 5 additions & 2 deletions spirv-cross2/src/reflect/combined_image_samplers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub struct CombinedImageSampler {
pub sampler_id: Handle<VariableId>,
}

impl<'a, T> Compiler<'a, T> {
impl<'ctx, T> Compiler<'ctx, T> {
/// Analyzes all OpImageFetch (texelFetch) opcodes and checks if there are instances where
/// said instruction is used without a combined image sampler.
/// GLSL targets do not support the use of texelFetch without a sampler.
Expand Down Expand Up @@ -117,10 +117,13 @@ impl<'a, T> Compiler<'a, T> {
}

/// Gets a remapping for the combined image samplers.
pub fn combined_image_samplers(&self) -> error::Result<CombinedImageSamplerIter> {
pub fn combined_image_samplers(&self) -> error::Result<CombinedImageSamplerIter<'ctx>> {
unsafe {
let mut samplers = std::ptr::null();
let mut size = 0;

// SAFETY: 'ctx is sound here.
// https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.cpp#L2497
sys::spvc_compiler_get_combined_image_samplers(
self.ptr.as_ptr(),
&mut samplers,
Expand Down
6 changes: 4 additions & 2 deletions spirv-cross2/src/reflect/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl Iterator for SpecializationConstantIter<'_> {
}

/// Reflection of specialization constants.
impl<'a, T> Compiler<'a, T> {
impl<'ctx, T> Compiler<'ctx, T> {
// check bounds of the constant, otherwise you can write to arbitrary memory.
unsafe fn bounds_check_constant(
handle: spvc_constant,
Expand Down Expand Up @@ -181,7 +181,7 @@ impl<'a, T> Compiler<'a, T> {
}

/// Query declared specialization constants.
pub fn specialization_constants(&self) -> error::Result<SpecializationConstantIter<'a>> {
pub fn specialization_constants(&self) -> error::Result<SpecializationConstantIter<'ctx>> {
unsafe {
let mut constants = std::ptr::null();
let mut size = 0;
Expand All @@ -192,6 +192,8 @@ impl<'a, T> Compiler<'a, T> {
)
.ok(self)?;

// SAFETY: 'ctx is sound here.
// https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.cpp#L2522
let slice = slice::from_raw_parts(constants, size);
Ok(SpecializationConstantIter(self.phantom(), slice.iter()))
}
Expand Down
13 changes: 9 additions & 4 deletions spirv-cross2/src/reflect/decorations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,16 @@ fn decoration_is_string(decoration: Decoration) -> bool {
}
}

impl<'a, T> Compiler<'a, T> {
impl<'ctx, T> Compiler<'ctx, T> {
/// Gets the value for decorations which take arguments.
pub fn decoration<I: Id>(
&self,
id: Handle<I>,
decoration: Decoration,
) -> error::Result<Option<DecorationValue>> {
// SAFETY: 'ctx is not sound to return here!
// https://github.com/KhronosGroup/SPIRV-Cross/blob/6a1fb66eef1bdca14acf7d0a51a3f883499d79f0/spirv_cross_c.cpp#L2154

// SAFETY: id is yielded by the instance so it's safe to use.
let id = SpvId(self.yield_id(id)?.id());
unsafe {
Expand Down Expand Up @@ -234,7 +237,7 @@ impl<'a, T> Compiler<'a, T> {
/// Gets the value for member decorations which take arguments.
pub fn member_decoration<I: Id>(
&self,
member: &StructMember<'a>,
member: &StructMember<'ctx>,
decoration: Decoration,
) -> error::Result<Option<DecorationValue>> {
self.member_decoration_by_handle(member.struct_type, member.index as u32, decoration)
Expand Down Expand Up @@ -333,7 +336,7 @@ impl<'a, T> Compiler<'a, T> {
/// Set the value of a decoration for a struct member.
pub fn set_member_decoration<'value>(
&mut self,
member: &StructMember<'a>,
member: &StructMember<'ctx>,
decoration: Decoration,
value: Option<impl Into<DecorationValue<'value>>>,
) -> error::Result<()> {
Expand Down Expand Up @@ -539,7 +542,7 @@ impl<'a, T> Compiler<'a, T> {
pub fn buffer_block_decorations(
&self,
variable: impl Into<Handle<VariableId>>,
) -> error::Result<Option<&'a [Decoration]>> {
) -> error::Result<Option<&'ctx [Decoration]>> {
let variable = variable.into();
let id = self.yield_id(variable)?;

Expand All @@ -554,6 +557,8 @@ impl<'a, T> Compiler<'a, T> {
)
.ok(self)?;

// SAFETY: 'ctx is sound here.
// https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.cpp#L2790
let slice = slice::from_raw_parts(buffer, size);
if slice.is_empty() {
Ok(None)
Expand Down
4 changes: 2 additions & 2 deletions spirv-cross2/src/reflect/entry_points.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl<'ctx, T> Compiler<'ctx, T> {
/// To disambiguate, we must pass along with the entry point names the execution model.
pub fn entry_points(&self) -> error::Result<EntryPointIter<'ctx>> {
unsafe {
// SAFETY: 'ctx is OK to return here
// SAFETY: 'ctx is sound here
// https://github.com/KhronosGroup/SPIRV-Cross/blob/6a1fb66eef1bdca14acf7d0a51a3f883499d79f0/spirv_cross_c.cpp#L2170
let mut entry_points = std::ptr::null();
let mut size = 0;
Expand All @@ -168,7 +168,7 @@ impl<'ctx, T> Compiler<'ctx, T> {
name: impl Into<ContextStr<'str>>,
model: spirv::ExecutionModel,
) -> error::Result<Option<ContextStr<'ctx>>> {
// SAFETY: 'ctx is OK to return here
// SAFETY: 'ctx is sound here
// https://github.com/KhronosGroup/SPIRV-Cross/blob/6a1fb66eef1bdca14acf7d0a51a3f883499d79f0/spirv_cross_c.cpp#L2217
let name = name.into();

Expand Down
6 changes: 4 additions & 2 deletions spirv-cross2/src/reflect/execution_modes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl ExecutionModeArguments {
}
}

impl<'a, T> Compiler<'a, T> {
impl<'ctx, T> Compiler<'ctx, T> {
/// Set or unset execution modes and arguments.
///
/// If arguments is `None`, unsets the execution mode. To set an execution mode that does not
Expand All @@ -68,14 +68,16 @@ impl<'a, T> Compiler<'a, T> {
}

/// Query `OpExecutionMode`.
pub fn execution_modes(&self) -> error::Result<&'a [spirv::ExecutionMode]> {
pub fn execution_modes(&self) -> error::Result<&'ctx [spirv::ExecutionMode]> {
unsafe {
let mut size = 0;
let mut modes = std::ptr::null();

sys::spvc_compiler_get_execution_modes(self.ptr.as_ptr(), &mut modes, &mut size)
.ok(self)?;

// SAFETY: 'ctx is sound here.
// https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.cpp#L2250
Ok(slice::from_raw_parts(modes, size))
}
}
Expand Down
3 changes: 3 additions & 0 deletions spirv-cross2/src/reflect/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ impl<'ctx, T> Compiler<'ctx, T> {
unsafe {
let name =
sys::spvc_compiler_get_remapped_declared_block_name(self.ptr.as_ptr(), handle);

// SAFETY: 'ctx is sound here
// https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.cpp#L2773
let name = ContextStr::from_ptr(name, self.ctx.clone());
if name.is_empty() {
Ok(None)
Expand Down
33 changes: 23 additions & 10 deletions spirv-cross2/src/reflect/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ use std::marker::PhantomData;
use std::ptr::NonNull;
use std::slice;

pub use spirv_cross_sys::{BuiltinResourceType, ResourceType};
/// The type of built-in resources to query.
pub use spirv_cross_sys::BuiltinResourceType;

/// The type of resource to query.
pub use spirv_cross_sys::ResourceType;

/// A handle to shader resources.
pub struct ShaderResources<'a>(NonNull<spvc_resources_s>, PhantomCompiler<'a>);
pub struct ShaderResources<'ctx>(NonNull<spvc_resources_s>, PhantomCompiler<'ctx>);

impl ContextRooted for &ShaderResources<'_> {
#[inline(always)]
Expand Down Expand Up @@ -102,7 +106,7 @@ impl<'a> InterfaceVariableSet<'a> {
}

// reflection
impl<'a, T> Compiler<'a, T> {
impl<'ctx, T> Compiler<'ctx, T> {
/// Returns a set of all global variables which are statically accessed
/// by the control flow graph from the current entry point.
/// Only variables which change the interface for a shader are returned, that is,
Expand All @@ -114,9 +118,13 @@ impl<'a, T> Compiler<'a, T> {
///
/// The return object is opaque to Rust, but its contents inspected by using [`InterfaceVariableSet::to_handles`].
/// There is no way to modify the contents or use your own `InterfaceVariableSet`.
pub fn active_interface_variables(&self) -> error::Result<InterfaceVariableSet<'a>> {
pub fn active_interface_variables(&self) -> error::Result<InterfaceVariableSet<'ctx>> {
unsafe {
let mut set = std::ptr::null();

// SAFETY: 'ctx is sound here
// https://github.com/KhronosGroup/SPIRV-Cross/blob/6a1fb66eef1bdca14acf7d0a51a3f883499d79f0/spirv_cross_c.cpp#L1888

sys::spvc_compiler_get_active_interface_variables(self.ptr.as_ptr(), &mut set)
.ok(self)?;

Expand Down Expand Up @@ -441,11 +449,13 @@ impl Clone for AllResources<'_> {
}
}

impl<'a> ShaderResources<'a> {
impl<'ctx> ShaderResources<'ctx> {
/// Get an iterator for all resources of the given type.
pub fn resources_for_type(&self, ty: ResourceType) -> error::Result<ResourceIter<'a>> {
// SAFETY: 'a is OK to return here:
pub fn resources_for_type(&self, ty: ResourceType) -> error::Result<ResourceIter<'ctx>> {
// SAFETY: 'ctx is sound here,
// https://github.com/KhronosGroup/SPIRV-Cross/blob/6a1fb66eef1bdca14acf7d0a51a3f883499d79f0/spirv_cross_c.cpp#L1802
// Furthermore, once allocated, the lifetime of spvc_resources_s is tied to that of the context.
// so all child resources inherit the lifetime.
let mut count = 0;
let mut out = std::ptr::null();
unsafe {
Expand All @@ -467,12 +477,14 @@ impl<'a> ShaderResources<'a> {
pub fn builtin_resources_for_type(
&self,
ty: BuiltinResourceType,
) -> error::Result<BuiltinResourceIter<'a>> {
) -> error::Result<BuiltinResourceIter<'ctx>> {
let mut count = 0;
let mut out = std::ptr::null();

// SAFETY: 'a is OK to return here:
// SAFETY: 'ctx is sound here,
// https://github.com/KhronosGroup/SPIRV-Cross/blob/6a1fb66eef1bdca14acf7d0a51a3f883499d79f0/spirv_cross_c.cpp#L1826
// Furthermore, once allocated, the lifetime of spvc_resources_s is tied to that of the context.
// so all child resources inherit the lifetime.
unsafe {
spirv_cross_sys::spvc_resources_get_builtin_resource_list_for_type(
self.0.as_ptr(),
Expand All @@ -490,7 +502,8 @@ impl<'a> ShaderResources<'a> {

/// Get all resources declared in the shader.
#[rustfmt::skip]
pub fn all_resources(&self) -> error::Result<AllResources<'a>> {
pub fn all_resources(&self) -> error::Result<AllResources<'ctx>> {
// SAFETY: 'ctx is sound by transitive property of resources_for_type
Ok(AllResources {
uniform_buffers: self.resources_for_type(ResourceType::UniformBuffer)?.collect(),
storage_buffers: self.resources_for_type(ResourceType::StorageBuffer)?.collect(),
Expand Down
3 changes: 3 additions & 0 deletions spirv-cross2/src/reflect/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,9 @@ pub enum TypeInner<'a> {

/// Reflection of SPIR-V types.
impl<T> Compiler<'_, T> {
// None of the names here belong to the context, they belong to the compiler.
// so 'ctx is unsound to return.

fn process_struct(&self, struct_ty_id: TypeId) -> error::Result<StructType> {
unsafe {
let ty = sys::spvc_compiler_get_type_handle(self.ptr.as_ptr(), struct_ty_id);
Expand Down
13 changes: 13 additions & 0 deletions spirv-cross2/src/spirv/mod.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,28 @@
/// [BuiltIn](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_builtin)
pub use spirv_cross_sys::SpvBuiltIn as BuiltIn;
/// [Capability](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_capability)
pub use spirv_cross_sys::SpvCapability as Capability;
/// [Decoration](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_decoration)
pub use spirv_cross_sys::SpvDecoration as Decoration;
/// [Dim](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_dim)
pub use spirv_cross_sys::SpvDim as Dim;
/// [Execution Mode](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_execution_mode)
pub use spirv_cross_sys::SpvExecutionMode as ExecutionMode;
/// [Execution Model](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_execution_model) (i.e. shader stage)
pub use spirv_cross_sys::SpvExecutionModel as ExecutionModel;
/// [FP Denorm Mode](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_fp_denorm_mode)
pub use spirv_cross_sys::SpvFPDenormMode as FPDenormMode;
/// [FP Fast Math Mode](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_fp_fast_math_mode)
pub use spirv_cross_sys::SpvFPFastMathModeMask as FPFastMathModeMask;
/// [FP Fast Math Mode](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_fp_fast_math_mode)
pub use spirv_cross_sys::SpvFPFastMathModeShift as FPFastMathModeShift;
/// [FP Operation Mode](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_fp_operation_mode)
pub use spirv_cross_sys::SpvFPOperationMode as FPOperationMode;
/// [FP Rounding Mode](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_fp_rounding_mode)
pub use spirv_cross_sys::SpvFPRoundingMode as FPRoundingMode;
/// [Image Format](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_image_format)
pub use spirv_cross_sys::SpvImageFormat as ImageFormat;
/// [Storage Class](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_storage_class)
pub use spirv_cross_sys::SpvStorageClass as StorageClass;

// These are unused.
Expand Down

0 comments on commit 47b9749

Please sign in to comment.