[msl] address review comments, rename SubOptions to PipelineOptions, add more inlined states

bin/convert.rs

@@ -202,10 +202,14 @@ fn main() {
         #[cfg(feature = "msl-out")]
         "metal" => {
             use naga::back::msl;
-            let sub_options = msl::SubOptions::default();
-            let (msl, _) =
-                msl::write_string(&module, info.as_ref().unwrap(), &params.msl, &sub_options)
-                    .unwrap_pretty();
+            let pipeline_options = msl::PipelineOptions::default();
+            let (msl, _) = msl::write_string(
+                &module,
+                info.as_ref().unwrap(),
+                &params.msl,
+                &pipeline_options,
+            )
+            .unwrap_pretty();
             fs::write(output_path, msl).unwrap();
         }
         #[cfg(feature = "spv-out")]

src/back/msl/mod.rs

@@ -31,7 +31,7 @@ use crate::{
 use std::fmt::{Error as FmtError, Write};
 
 mod keywords;
-mod sampler;
+pub mod sampler;
 mod writer;
 
 pub use writer::Writer;
@@ -138,15 +138,15 @@ impl Default for Options {
 // A subset of options that are meant to be changed per pipeline.
 #[derive(Debug, Clone)]
 #[cfg_attr(feature = "deserialize", derive(serde::Deserialize))]
-pub struct SubOptions {
+pub struct PipelineOptions {
     /// Allow `BuiltIn::PointSize` in the vertex shader.
     /// Metal doesn't like this for non-point primitive topologies.
     pub allow_point_size: bool,
 }
 
-impl Default for SubOptions {
+impl Default for PipelineOptions {
     fn default() -> Self {
-        SubOptions {
+        PipelineOptions {
             allow_point_size: true,
         }
     }
@@ -281,10 +281,10 @@ pub fn write_string(
     module: &crate::Module,
     info: &ModuleInfo,
     options: &Options,
-    sub_options: &SubOptions,
+    pipeline_options: &PipelineOptions,
 ) -> Result<(String, TranslationInfo), Error> {
     let mut w = writer::Writer::new(String::new());
-    let info = w.write(module, info, options, sub_options)?;
+    let info = w.write(module, info, options, pipeline_options)?;
     Ok((w.finish(), info))
 }
 

src/back/msl/sampler.rs

@@ -1,5 +1,6 @@
 #[cfg(feature = "deserialize")]
 use serde::Deserialize;
+use std::{num::NonZeroU32, ops::Range};
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 #[cfg_attr(feature = "deserialize", derive(Deserialize))]
@@ -14,6 +15,15 @@ impl Default for Coord {
     }
 }
 
+impl Coord {
+    pub fn as_str(&self) -> &'static str {
+        match *self {
+            Self::Normalized => "normalized",
+            Self::Pixel => "pixel",
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq)]
 #[cfg_attr(feature = "deserialize", derive(Deserialize))]
 pub enum Address {
@@ -131,5 +141,7 @@ pub struct InlineSampler {
     pub mag_filter: Filter,
     pub min_filter: Filter,
     pub mip_filter: Option<Filter>,
+    pub lod_clamp: Option<Range<f32>>,
+    pub max_anisotropy: Option<NonZeroU32>,
     pub compare_func: CompareFunc,
 }

src/back/msl/writer.rs

@@ -1,5 +1,6 @@
 use super::{
-    keywords::RESERVED, sampler as sm, Error, LocationMode, Options, SubOptions, TranslationInfo,
+    keywords::RESERVED, sampler as sm, Error, LocationMode, Options, PipelineOptions,
+    TranslationInfo,
 };
 use crate::{
     arena::Handle,
@@ -152,7 +153,7 @@ struct ExpressionContext<'a> {
     origin: FunctionOrigin,
     info: &'a FunctionInfo,
     module: &'a crate::Module,
-    sub_options: &'a SubOptions,
+    pipeline_options: &'a PipelineOptions,
 }
 
 impl<'a> ExpressionContext<'a> {
@@ -775,7 +776,7 @@ impl<W: Write> Writer<W> {
                         write!(self.out, "{}return {} {{", level, struct_name)?;
                         let mut is_first = true;
                         for (index, member) in members.iter().enumerate() {
-                            if !context.sub_options.allow_point_size
+                            if !context.pipeline_options.allow_point_size
                                 && member.binding
                                     == Some(crate::Binding::BuiltIn(crate::BuiltIn::PointSize))
                             {
@@ -1078,7 +1079,7 @@ impl<W: Write> Writer<W> {
         module: &crate::Module,
         info: &ModuleInfo,
         options: &Options,
-        sub_options: &SubOptions,
+        pipeline_options: &PipelineOptions,
     ) -> Result<TranslationInfo, Error> {
         self.names.clear();
         self.namer.reset(module, RESERVED, &mut self.names);
@@ -1090,7 +1091,7 @@ impl<W: Write> Writer<W> {
         self.write_scalar_constants(module)?;
         self.write_type_defs(module)?;
         self.write_composite_constants(module)?;
-        self.write_functions(module, info, options, sub_options)
+        self.write_functions(module, info, options, pipeline_options)
     }
 
     fn write_type_defs(&mut self, module: &crate::Module) -> Result<(), Error> {
@@ -1366,6 +1367,17 @@ impl<W: Write> Writer<W> {
                 sampler.border_color.as_str(),
             )?;
         }
+        //TODO: I'm not able to feed this in a way that MSL likes:
+        //>error: use of undeclared identifier 'lod_clamp'
+        //>error: no member named 'max_anisotropy' in namespace 'metal'
+        if false {
+            if let Some(ref lod) = sampler.lod_clamp {
+                writeln!(self.out, "{}lod_clamp({},{}),", level, lod.start, lod.end,)?;
+            }
+            if let Some(aniso) = sampler.max_anisotropy {
+                writeln!(self.out, "{}max_anisotropy({}),", level, aniso.get(),)?;
+            }
+        }
         if sampler.compare_func != sm::CompareFunc::Never {
             writeln!(
                 self.out,
@@ -1375,7 +1387,13 @@ impl<W: Write> Writer<W> {
                 sampler.compare_func.as_str(),
             )?;
         }
-        writeln!(self.out, "{}{}::coord::normalized", NAMESPACE, level)?;
+        writeln!(
+            self.out,
+            "{}{}::coord::{}",
+            level,
+            NAMESPACE,
+            sampler.coord.as_str()
+        )?;
         Ok(())
     }
 
@@ -1385,7 +1403,7 @@ impl<W: Write> Writer<W> {
         module: &crate::Module,
         mod_info: &ModuleInfo,
         options: &Options,
-        sub_options: &SubOptions,
+        pipeline_options: &PipelineOptions,
     ) -> Result<TranslationInfo, Error> {
         let mut pass_through_globals = Vec::new();
         for (fun_handle, fun) in module.functions.iter() {
@@ -1447,7 +1465,7 @@ impl<W: Write> Writer<W> {
                     origin: FunctionOrigin::Handle(fun_handle),
                     info: fun_info,
                     module,
-                    sub_options,
+                    pipeline_options,
                 },
                 mod_info,
                 result_struct: None,
@@ -1576,7 +1594,7 @@ impl<W: Write> Writer<W> {
                     for (name, ty, binding) in result_members {
                         let type_name = &self.names[&NameKey::Type(ty)];
                         let binding = binding.ok_or(Error::Validation)?;
-                        if !sub_options.allow_point_size
+                        if !pipeline_options.allow_point_size
                             && *binding == crate::Binding::BuiltIn(crate::BuiltIn::PointSize)
                         {
                             continue;
@@ -1760,7 +1778,7 @@ impl<W: Write> Writer<W> {
                     origin: FunctionOrigin::EntryPoint(ep_index as _),
                     info: fun_info,
                     module,
-                    sub_options,
+                    pipeline_options,
                 },
                 mod_info,
                 result_struct: Some(&stage_out_name),

tests/in/skybox.param.ron

@@ -21,6 +21,8 @@
 				mip_filter: None,
 				border_color: TransparentBlack,
 				compare_func: Never,
+				lod_clamp: Some((start: 0.5, end: 10.0)),
+				max_anisotropy: Some(8),
 			),
 		],
 		spirv_cross_compatibility: false,

tests/out/skybox.msl.snap

@@ -66,7 +66,7 @@ fragment fs_mainOutput fs_main(
         metal::r_address::clamp_to_edge,
         metal::mag_filter::linear,
         metal::min_filter::linear,
-metal        ::coord::normalized
+        metal::coord::normalized
     );
     const VertexOutput in = { position, varyings1.uv };
     metal::float4 _expr5 = r_texture.sample(r_sampler, in.uv);

tests/snapshots.rs

@@ -147,11 +147,11 @@ fn check_output_msl(
         &default_options
     };
 
-    let sub_options = msl::SubOptions {
+    let pipeline_options = msl::PipelineOptions {
         allow_point_size: true,
     };
 
-    let (msl, _) = msl::write_string(module, info, options, &sub_options).unwrap();
+    let (msl, _) = msl::write_string(module, info, options, &pipeline_options).unwrap();
 
     with_snapshot_settings(|| {
         insta::assert_snapshot!(format!("{}.msl", name), msl);