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

Incorrect translation of spirv with float64 (dvec2) #1272

Closed
dhardy opened this issue Aug 21, 2021 · 2 comments
Closed

Incorrect translation of spirv with float64 (dvec2) #1272

dhardy opened this issue Aug 21, 2021 · 2 comments
Labels
area: back-end Outputs of shader conversion help wanted Extra attention is needed kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output

Comments

@dhardy
Copy link

dhardy commented Aug 21, 2021

KAS's mandlebrot example does not render with 64-bit shaders (--features shader64) since upgrading to wgpu v0.10 (leaving a black background, which is not the clear colour). It runs correctly with 32-bit shaders (almost identical code aside from the fragment shader).

An API trace is attached: trace.tar.gz

Using wgpu::Features::SPIRV_SHADER_PASSTHROUGH the example works correctly, therefore I deduce the error lies with Naga rather than (other parts of) WGPU. The following patch achieves this:

diff --git a/examples/mandlebrot/mandlebrot.rs b/examples/mandlebrot/mandlebrot.rs
index bf1b202b..03e23cbb 100644
--- a/examples/mandlebrot/mandlebrot.rs
+++ b/examples/mandlebrot/mandlebrot.rs
@@ -45,10 +45,12 @@ impl Shaders {
         let vertex = device.create_shader_module(&include_spirv!("shader.vert.spv"));
         // Note: we don't use wgpu::include_spirv since it forces validation,
         // which cannot currently deal with double precision floats (dvec2).
-        let fragment = device.create_shader_module(&wgpu::ShaderModuleDescriptor {
-            label: Some("fragment shader"),
-            source: wgpu::util::make_spirv(FRAG_SHADER),
-        });
+        let fragment = unsafe {
+            device.create_shader_module_spirv(&wgpu::ShaderModuleDescriptorSpirV {
+                label: Some("fragment shader"),
+                source: wgpu::util::make_spirv_raw(FRAG_SHADER),
+            })
+        };
 
         Shaders { vertex, fragment }
     }
@@ -94,7 +96,9 @@ impl CustomPipeBuilder for PipeBuilder {
     fn device_descriptor() -> wgpu::DeviceDescriptor<'static> {
         wgpu::DeviceDescriptor {
             label: None,
-            features: wgpu::Features::PUSH_CONSTANTS | SHADER_FLOAT64,
+            features: wgpu::Features::PUSH_CONSTANTS
+                | wgpu::Features::SPIRV_SHADER_PASSTHROUGH
+                | SHADER_FLOAT64,
             limits: wgpu::Limits {
                 max_push_constant_size: size_of::<PushConstants>().cast(),
                 ..Default::default()

Tested on: Fedora 34 x86_64 RX570

@kvark kvark added area: front-end Input formats for conversion help wanted Extra attention is needed kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output labels Aug 23, 2021
@LPGhatguy
Copy link

I think I've run into this. It is not front-end specific; Naga also has trouble with the GLSL frontend and WGSL backend. It always emits f64 instead of f32. I found this while working on Crevice.

This seems to happen with all f64-based data types, including dvec4 and dmat4.

GLSL input:

#version 450

struct FourF64 {
        double x;
        double y;
        double z;
        double w;
};

layout(std140, set = 0, binding = 0) readonly buffer INPUT {
    FourF64 in_data;
};

layout(std140, set = 0, binding = 1) buffer OUTPUT {
    FourF64 out_data;
};

void main() {
    out_data = in_data;
}

WGSL output:

struct FourF64_ {
    x: f32;
    y: f32;
    z: f32;
    w: f32;
};

[[block]]
struct INPUT {
    in_data: FourF64_;
};

[[block]]
struct OUTPUT {
    out_data: FourF64_;
};

[[group(0), binding(0)]]
var<storage> global: INPUT;
[[group(0), binding(1)]]
var<storage, read_write> global1: OUTPUT;

fn main1() {
    let e4: FourF64_ = global.in_data;
    global1.out_data = e4;
    return;
}

[[stage(compute), workgroup_size(1, 1, 1)]]
fn main() {
    main1();
    return;
}

@teoxoy
Copy link
Member

teoxoy commented Apr 30, 2022

I tried the latest version of the example and noticed it's working. I narrowed it down to naga v0.7.2 which gets it going.

It looks like it has been fixed by #1515.

@teoxoy teoxoy closed this as completed Apr 30, 2022
@teoxoy teoxoy added area: back-end Outputs of shader conversion and removed area: front-end Input formats for conversion labels Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: back-end Outputs of shader conversion help wanted Extra attention is needed kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output
Projects
None yet
Development

No branches or pull requests

4 participants