bump shader tools version and use distribution install by default (#6164)

Update shader tools to new version. Notably, this release contains spirv-cross with fixed issue https://github.com/KhronosGroup/SPIRV-Cross/issues/2129.

# Important Notes
Spirv-cross has no versioning that we could use to specify requirements for using system-wide installed versions. Instead, we have to download the prebuilt distribution by default, so we can rely on known good versions. The usage of binaries in PATH can still be enabled with a build flag, but it is discouraged due to severity of the bug and no easy way of detecting it. If the project is built with buggy shader tools version, the application will run, but it will be visually slightly broken in unexpected ways.

build/build/src/project/wasm.rs

@@ -154,6 +154,7 @@ pub struct BuildInput {
     pub log_level:             LogLevel,
     pub uncollapsed_log_level: LogLevel,
     pub wasm_size_limit:       Option<byte_unit::Byte>,
+    pub system_shader_tools:   bool,
 }
 
 impl BuildInput {
@@ -219,8 +220,6 @@ impl IsTarget for Wasm {
             // We want to be able to pass --profile this way.
             WasmPack.require_present_that(VersionReq::parse(">=0.10.1")?).await?;
 
-            ShaderTools.install_if_missing(&cache).await?;
-
             let BuildInput {
                 crate_path,
                 wasm_opt_options,
@@ -231,8 +230,19 @@ impl IsTarget for Wasm {
                 log_level,
                 uncollapsed_log_level,
                 wasm_size_limit: _wasm_size_limit,
+                system_shader_tools,
             } = &inner;
 
+            // NOTE: We cannot trust locally installed version of shader tools to be correct.
+            // Those binaries have no reliable versioning, and existing common distributions (e.g.
+            // Vulkan SDK) contain old builds with bugs that impact our shaders. By default, we have
+            // to force usage of our own distribution built on our CI.
+            if *system_shader_tools {
+                ShaderTools.install_if_missing(&cache).await?;
+            } else {
+                ShaderTools.install(&cache).await?;
+            }
+
             cache::goodie::binaryen::Binaryen { version: BINARYEN_VERSION_TO_INSTALL }
                 .install_if_missing(&cache)
                 .await?;
@@ -334,6 +344,7 @@ impl IsWatchable for Wasm {
                 log_level,
                 uncollapsed_log_level,
                 wasm_size_limit,
+                system_shader_tools: _,
             } = inner;
 
 

build/ci_utils/src/cache/goodie.rs

@@ -37,7 +37,7 @@ pub trait Goodie: Debug + Clone + Send + Sync + 'static {
 pub trait GoodieExt: Goodie {
     /// Check if this goodie is active. If not, download it and activate it.
     ///
-    /// If the goodie was already ective, returns Ok(None). If it was not active, returns
+    /// If the goodie was already active, returns Ok(None). If it was not active, returns
     /// Ok(Some(path)), where path is the path to the downloaded goodie within the cache.
     /// Usually it should not be necessary to use the returned path, as the goodie should have been
     /// activated and the global state modified accordingly.
@@ -49,13 +49,24 @@ pub trait GoodieExt: Goodie {
                 trace!("Skipping activation of {this:?} because it already present.",);
                 Result::Ok(None)
             } else {
-                let package = this.get(&cache).await?;
-                this.activate(&package)?;
+                let package = this.install(&cache).await?;
                 Result::Ok(Some(package))
             }
         }
         .boxed()
     }
+
+    // Download it and activate this goodie. Does not check if it is already active.
+    fn install(&self, cache: &Cache) -> BoxFuture<'static, Result<PathBuf>> {
+        let this = self.clone();
+        let cache = cache.clone();
+        async move {
+            let package = this.get(&cache).await?;
+            this.activate(&package)?;
+            Result::Ok(package)
+        }
+        .boxed()
+    }
 }
 
 impl<T: Goodie> GoodieExt for T {}

build/ci_utils/src/goodies/shader_tools.rs

@@ -29,7 +29,7 @@ use crate::programs::spirv_cross::SpirvCross;
 pub const SHADER_TOOLS_REPO: RepoRef = RepoRef { owner: "enso-org", name: "shader-tools" };
 
 /// Version of the shader tools package that we download.
-pub const VERSION: Version = Version::new(0, 1, 0);
+pub const VERSION: Version = Version::new(0, 2, 0);
 
 
 // =========================

build/cli/src/arg/wasm.rs

@@ -81,6 +81,12 @@ pub struct BuildInput {
         default_value_if("skip-wasm-opt", None, Some("0")),
     )]
     pub wasm_size_limit: Option<byte_unit::Byte>,
+
+    /// Allow usage of system installation of `shaderc`, `spirv-opt` and `spirv-cross`. When
+    /// present, the binaries from PATH will be preferred over downloading prebuilt distribution.
+    /// Caution: old versions of those tools might introduce subtle bugs in optimized shaders.
+    #[clap(long, enso_env())]
+    pub system_shader_tools: bool,
 }
 
 #[derive(Args, Clone, Debug, PartialEq, Eq)]

build/cli/src/lib.rs

@@ -616,6 +616,7 @@ impl Resolvable for Wasm {
             wasm_uncollapsed_log_level,
             wasm_size_limit,
             skip_wasm_opt,
+            system_shader_tools,
         } = from;
         ok_ready_boxed(wasm::BuildInput {
             crate_path,
@@ -627,6 +628,7 @@ impl Resolvable for Wasm {
             log_level: wasm_log_level,
             uncollapsed_log_level: wasm_uncollapsed_log_level,
             wasm_size_limit: wasm_size_limit.filter(|size_limit| size_limit.get_bytes() > 0),
+            system_shader_tools,
         })
     }
 }

lib/rust/ensogl/core/src/display/shape/primitive/system.rs

@@ -455,21 +455,8 @@ impl ShapeSystemModel {
                 vec2 padded_size = input_size + padding2;
                 vec2 uv_scale = padded_size / input_size;
                 vec2 uv_offset = padding / input_size;
+                input_uv = input_uv * uv_scale - uv_offset;
 
-                // Note: SPIRV-Cross issue https://github.com/KhronosGroup/SPIRV-Cross/issues/2129
-                // To avoid incorrect code generation for `Fma` instruction in SPIRV-Cross, the
-                // `input_uv` modification is intentionally split into two separate expressions.
-                // When this is written in single line as follows:
-                // ```
-                // input_uv = input_uv * uv_scale - uv_offset;
-                // ```
-                // That expression is incorrectly mis-optimized into:
-                // ```
-                // input_uv *= input_uv - uv_offset;
-                // ```
-                input_uv *= uv_scale;
-                input_uv -= uv_offset;
-                
                 // Compute the vertex position with shape padding, apply alignment to the vertex
                 // position, but not to the local SDF coordinates. Shape definitions should always
                 // have their origin point in the center of the shape.